Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend reusable components #11

Merged
merged 13 commits into from
Feb 18, 2021
Merged

Extend reusable components #11

merged 13 commits into from
Feb 18, 2021

Conversation

terryyylim
Copy link
Contributor

@terryyylim terryyylim commented Feb 16, 2021

Signed-off-by: Terence Lim terencelimxp@gmail.com

Add more reusable components to the library for use in other MLP systems.

Components added:

  1. AccordionForm
  2. ConfirmationModal
  3. ExpandableContainer
  4. Forms
    • ComboBox
    • DescribedFormGroup
    • DockerImageComboBox
    • EndpointComboBox
    • FieldDuration
    • InMemoryTableForm
    • LabelWithToolTip
    • Validation
    • HorizontalDescriptionList
  5. Multi-steps Form
  6. Overlay Mask
  7. Page Navigation

@terryyylim terryyylim force-pushed the update-mlp-lib-components branch 2 times, most recently from 7fed32e to 17f3e99 Compare February 16, 2021 12:07
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
@terryyylim terryyylim force-pushed the update-mlp-lib-components branch from 17f3e99 to 202a9ed Compare February 16, 2021 12:14
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
@ariefrahmansyah
Copy link
Contributor

Can you please update the PR's description with the list of added/updated components?

Copy link
Contributor

@romanwozniak romanwozniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM. Agree with @ariefrahmansyah that a quick doc about added components would be useful.

@tiopramayudi @ariefrahmansyah it just caught my attention, that CI here ignores lint errors. Also, all the other jobs should probably have needs on lint job, so the pipeline would not be executed e2e if lint job fails. wdyt?

Copy link
Contributor

@ariefrahmansyah ariefrahmansyah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bump the version on package.json files to 1.1.0:

  1. https://github.com/gojek/mlp/blob/main/ui/package.json#L3
  2. https://github.com/gojek/mlp/blob/main/ui/packages/app/package.json#L3
  3. https://github.com/gojek/mlp/blob/main/ui/packages/lib/package.json#L3

I also agree with @romanwozniak's suggestion, can you help add lint as dependencies to other jobs in this PR?

Other than that, LGTM

Signed-off-by: Terence Lim <terencelimxp@gmail.com>
Signed-off-by: Terence Lim <terencelimxp@gmail.com>
@terryyylim
Copy link
Contributor Author

Please bump the version on package.json files to 1.1.0:

  1. https://github.com/gojek/mlp/blob/main/ui/package.json#L3
  2. https://github.com/gojek/mlp/blob/main/ui/packages/app/package.json#L3
  3. https://github.com/gojek/mlp/blob/main/ui/packages/lib/package.json#L3

I also agree with @romanwozniak's suggestion, can you help add lint as dependencies to other jobs in this PR?

Other than that, LGTM

I added lint dependency to integration-test step only since the other steps already have needs: integration-test dependency.

@romanwozniak
Copy link
Contributor

I added lint dependency to integration-test step only since the other steps already have needs: integration-test dependency.

publish-gojek-mlp-ui should also be dependent on lint

Signed-off-by: Terence Lim <terencelimxp@gmail.com>
@ariefrahmansyah ariefrahmansyah merged commit 7d46703 into main Feb 18, 2021
@ariefrahmansyah ariefrahmansyah deleted the update-mlp-lib-components branch February 18, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants