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

feat(NumberField): add belowMin and aboveMax error keys to NumberField #2853

Merged
merged 5 commits into from
Jul 23, 2024

Conversation

ragafus
Copy link
Contributor

@ragafus ragafus commented Jul 12, 2024

Summary

Adds two new error keys, belowMin and aboveMax, to improve NumberField DX.

Description

NumberField provides a min and max props which are used as property on input field. Unfortunately, users can introduce numbers above the max prop and below the min props. ATM, the developer using NumberField has to handle all the logic regarding min and max errors.
With this PR we provide default error messages for belowMin and aboveMax error keys, so, the developer using NumberField has to just generate these new keys when validating their forms.

@ragafus ragafus self-assigned this Jul 12, 2024
Copy link

vercel bot commented Jul 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui-kit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2024 3:31pm

Copy link

changeset-bot bot commented Jul 12, 2024

🦋 Changeset detected

Latest commit: 5ebceb0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 96 packages
Name Type
@commercetools-uikit/number-field Minor
@commercetools-uikit/i18n Minor
@commercetools-uikit/fields Minor
@commercetools-frontend/ui-kit Minor
@commercetools-uikit/design-system Minor
@commercetools-uikit/calendar-time-utils Minor
@commercetools-uikit/calendar-utils Minor
@commercetools-uikit/hooks Minor
@commercetools-uikit/localized-utils Minor
@commercetools-uikit/utils Minor
@commercetools-uikit/accessible-hidden Minor
@commercetools-uikit/avatar Minor
@commercetools-uikit/card Minor
@commercetools-uikit/collapsible-motion Minor
@commercetools-uikit/collapsible-panel Minor
@commercetools-uikit/collapsible Minor
@commercetools-uikit/constraints Minor
@commercetools-uikit/data-table-manager Minor
@commercetools-uikit/data-table Minor
@commercetools-uikit/field-errors Minor
@commercetools-uikit/field-label Minor
@commercetools-uikit/field-warnings Minor
@commercetools-uikit/grid Minor
@commercetools-uikit/icons Minor
@commercetools-uikit/label Minor
@commercetools-uikit/link Minor
@commercetools-uikit/loading-spinner Minor
@commercetools-uikit/messages Minor
@commercetools-uikit/notifications Minor
@commercetools-uikit/pagination Minor
@commercetools-uikit/primary-action-dropdown Minor
@commercetools-uikit/progress-bar Minor
@commercetools-uikit/stamp Minor
@commercetools-uikit/tag Minor
@commercetools-uikit/text Minor
@commercetools-uikit/tooltip Minor
@commercetools-uikit/view-switcher Minor
@commercetools-uikit/accessible-button Minor
@commercetools-uikit/flat-button Minor
@commercetools-uikit/icon-button Minor
@commercetools-uikit/link-button Minor
@commercetools-uikit/primary-button Minor
@commercetools-uikit/secondary-button Minor
@commercetools-uikit/secondary-icon-button Minor
@commercetools-uikit/dropdown-menu Minor
@commercetools-uikit/async-creatable-select-field Minor
@commercetools-uikit/async-select-field Minor
@commercetools-uikit/creatable-select-field Minor
@commercetools-uikit/date-field Minor
@commercetools-uikit/date-range-field Minor
@commercetools-uikit/date-time-field Minor
@commercetools-uikit/localized-multiline-text-field Minor
@commercetools-uikit/localized-text-field Minor
@commercetools-uikit/money-field Minor
@commercetools-uikit/multiline-text-field Minor
@commercetools-uikit/password-field Minor
@commercetools-uikit/radio-field Minor
@commercetools-uikit/search-select-field Minor
@commercetools-uikit/select-field Minor
@commercetools-uikit/text-field Minor
@commercetools-uikit/time-field Minor
@commercetools-uikit/async-creatable-select-input Minor
@commercetools-uikit/async-select-input Minor
@commercetools-uikit/checkbox-input Minor
@commercetools-uikit/creatable-select-input Minor
@commercetools-uikit/date-input Minor
@commercetools-uikit/date-range-input Minor
@commercetools-uikit/date-time-input Minor
@commercetools-uikit/input-utils Minor
@commercetools-uikit/localized-money-input Minor
@commercetools-uikit/localized-multiline-text-input Minor
@commercetools-uikit/localized-rich-text-input Minor
@commercetools-uikit/localized-text-input Minor
@commercetools-uikit/money-input Minor
@commercetools-uikit/multiline-text-input Minor
@commercetools-uikit/number-input Minor
@commercetools-uikit/password-input Minor
@commercetools-uikit/radio-input Minor
@commercetools-uikit/rich-text-input Minor
@commercetools-uikit/rich-text-utils Minor
@commercetools-uikit/search-select-input Minor
@commercetools-uikit/search-text-input Minor
@commercetools-uikit/select-input Minor
@commercetools-uikit/select-utils Minor
@commercetools-uikit/selectable-search-input Minor
@commercetools-uikit/text-input Minor
@commercetools-uikit/time-input Minor
@commercetools-uikit/toggle-input Minor
@commercetools-uikit/spacings-inline Minor
@commercetools-uikit/spacings-inset-squish Minor
@commercetools-uikit/spacings-inset Minor
@commercetools-uikit/spacings-stack Minor
@commercetools-uikit/buttons Minor
@commercetools-uikit/inputs Minor
@commercetools-uikit/spacings Minor
visual-testing-app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ragafus ragafus force-pushed the rg-number-field-min-max-errors branch from 14d6550 to d629d53 Compare July 12, 2024 13:28
@ragafus ragafus force-pushed the rg-number-field-min-max-errors branch from d629d53 to 361f9ba Compare July 12, 2024 13:35
@ragafus ragafus force-pushed the rg-number-field-min-max-errors branch from 361f9ba to a8ec564 Compare July 12, 2024 13:48
@ragafus ragafus marked this pull request as ready for review July 12, 2024 13:50
@ragafus ragafus requested a review from a team July 12, 2024 13:50
@ragafus ragafus changed the title feat: add belowMin and aboveMax error keys to NumberField feat(NumberField): add belowMin and aboveMax error keys to NumberField Jul 12, 2024
Copy link

gitstream-cm bot commented Jul 12, 2024

This PR is missing a Jira ticket reference in the title or description.
Please add a Jira ticket reference to the title or description of this PR.

Copy link

gitstream-cm bot commented Jul 12, 2024

🥷 Code experts: emmenko

emmenko has most 👩‍💻 activity in the files.
emmenko, Rhotimee have most 🧠 knowledge in the files.

See details

packages/components/fields/number-field/README.md

Activity based on git-commit:

emmenko
JUL
JUN
MAY
APR
MAR
FEB

Knowledge based on git-blame:
Rhotimee: 28%
emmenko: 21%

packages/components/fields/number-field/docs/additional-info.md

Activity based on git-commit:

emmenko
JUL
JUN
MAY
APR
MAR
FEB

Knowledge based on git-blame:
emmenko: 49%

packages/components/fields/number-field/src/number-field.spec.js

Activity based on git-commit:

emmenko
JUL
JUN
MAY
APR
MAR
FEB

Knowledge based on git-blame:
emmenko: 1%

packages/components/fields/number-field/src/number-field.story.js

Activity based on git-commit:

emmenko
JUL
JUN
MAY
APR
MAR
FEB

Knowledge based on git-blame:
emmenko: 4%

packages/components/fields/number-field/src/number-field.tsx

Activity based on git-commit:

emmenko
JUL
JUN
MAY
APR
MAR
FEB

Knowledge based on git-blame:
Rhotimee: 82%
emmenko: 6%

packages/i18n/data/core.json

Activity based on git-commit:

emmenko
JUL
JUN
MAY
APR
MAR 292 additions & 73 deletions
FEB

Knowledge based on git-blame:
emmenko: 100%

To learn more about /:\ gitStream - Visit our Docs

@@ -278,7 +280,36 @@ class NumberField extends Component<TNumberFieldProps, TNumberFieldState> {
id={sequentialErrorsId}
errors={this.props.errors}
isVisible={hasError}
renderError={this.props.renderError}
renderError={(key: string, error?: boolean) => {
const element = this.props.renderError?.(key, error);
Copy link
Contributor

Choose a reason for hiding this comment

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

@ragafus Do you have a bit of time for us (remeet) to elaborate as to why this change is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ragafus Do you have a bit of time for us (remeet) to elaborate as to why this change is needed?

Of course! I can explain my point of view as a "consumer" of NumberField component.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great! I will set up a meeting for us.
Thanks again

@ragafus ragafus requested review from misama-ct and ddouglasz July 22, 2024 15:34
@ragafus
Copy link
Contributor Author

ragafus commented Jul 22, 2024

Hey @misama-ct @ddouglasz, I've refactored the code as we agreed in the meeting. Please, let me know your thoughts.

Copy link
Contributor

@ddouglasz ddouglasz left a comment

Choose a reason for hiding this comment

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

Thank you @ragafus
Looks good to me now 🙌🏾

@ddouglasz
Copy link
Contributor

Please note that we would also need a sign off from our designer @FilPob too.

@misama-ct misama-ct requested a review from FilPob July 23, 2024 10:40
@misama-ct
Copy link
Contributor

Thanks for taking the time to do this change.

@FilPob you can provoke the error-message by setting a min and/or max value here, entering a number out of the min/max range in the input and then checking the "touched" checkbox.

@FilPob
Copy link

FilPob commented Jul 23, 2024

Thanks! Looks good from my side

@ddouglasz ddouglasz merged commit f10448a into main Jul 23, 2024
9 checks passed
@ddouglasz ddouglasz deleted the rg-number-field-min-max-errors branch July 23, 2024 12:42
@ct-changesets ct-changesets bot mentioned this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants