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

LabeledField: Make sure custom required message is shown #2425

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

beaesguerra
Copy link
Member

Summary:

Makes sure custom required messages passed into LabeledField or the field prop are displayed

Issue: WB-1848

Test plan:

Confirm in ?path=/story/packages-labeledfield--required that the error message shows the custom required message

… the required value. This was preventing custom required messages from rendering
…om required messages passed into `LabeledField` or the `field` prop are displayed
@beaesguerra beaesguerra self-assigned this Jan 11, 2025
Copy link

changeset-bot bot commented Jan 11, 2025

🦋 Changeset detected

Latest commit: f09dcda

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

This PR includes changesets to release 1 package
Name Type
@khanacademy/wonder-blocks-labeled-field 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

Copy link
Contributor

github-actions bot commented Jan 11, 2025

Size Change: +5 B (+0.01%)

Total Size: 98.8 kB

Filename Size Change
packages/wonder-blocks-labeled-field/dist/es/index.js 1.94 kB +5 B (+0.26%)
ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-accordion/dist/es/index.js 3.77 kB
packages/wonder-blocks-banner/dist/es/index.js 1.53 kB
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.77 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 887 B
packages/wonder-blocks-button/dist/es/index.js 4.04 kB
packages/wonder-blocks-cell/dist/es/index.js 2.01 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.06 kB
packages/wonder-blocks-core/dist/es/index.js 3.52 kB
packages/wonder-blocks-data/dist/es/index.js 6.24 kB
packages/wonder-blocks-dropdown/dist/es/index.js 19.1 kB
packages/wonder-blocks-form/dist/es/index.js 6.2 kB
packages/wonder-blocks-grid/dist/es/index.js 1.36 kB
packages/wonder-blocks-icon-button/dist/es/index.js 2.95 kB
packages/wonder-blocks-icon/dist/es/index.js 871 B
packages/wonder-blocks-layout/dist/es/index.js 1.82 kB
packages/wonder-blocks-link/dist/es/index.js 2.28 kB
packages/wonder-blocks-modal/dist/es/index.js 5.42 kB
packages/wonder-blocks-pill/dist/es/index.js 1.65 kB
packages/wonder-blocks-popover/dist/es/index.js 4.85 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.52 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.36 kB
packages/wonder-blocks-switch/dist/es/index.js 1.92 kB
packages/wonder-blocks-testing-core/dist/es/index.js 3.74 kB
packages/wonder-blocks-testing/dist/es/index.js 1.07 kB
packages/wonder-blocks-theming/dist/es/index.js 693 B
packages/wonder-blocks-timing/dist/es/index.js 1.8 kB
packages/wonder-blocks-tokens/dist/es/index.js 2.36 kB
packages/wonder-blocks-toolbar/dist/es/index.js 905 B
packages/wonder-blocks-tooltip/dist/es/index.js 6.99 kB
packages/wonder-blocks-typography/dist/es/index.js 1.23 kB

compressed-size-action

Copy link
Contributor

github-actions bot commented Jan 11, 2025

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-fnsnwmxvwb.chromatic.com/

Chromatic results:

Metric Total
Captured snapshots 0
Tests with visual changes 0
Total stories 525
Inherited (not captured) snapshots [TurboSnap] 381
Tests on the build 381

@@ -250,7 +250,7 @@ export default function LabeledField(props: Props) {
]
.filter(Boolean)
.join(" "),
required: isRequired,
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we were passing in a boolean, the default required error message was always used 😅

@beaesguerra beaesguerra marked this pull request as ready for review January 11, 2025 00:26
@khan-actions-bot khan-actions-bot requested a review from a team January 11, 2025 00:26
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to .changeset/serious-cherries-wonder.md, packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx, packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

Copy link
Contributor

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (2595ef0) and published all packages with changesets to npm.

You can install the packages in webapp by running:

./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2425"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2425

Copy link
Member

@jandrade jandrade left a comment

Choose a reason for hiding this comment

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

LGTM! nice fix 🚀

@beaesguerra beaesguerra merged commit ed8fb7b into feature/labeled-field Jan 15, 2025
22 checks passed
@beaesguerra beaesguerra deleted the wb-1848-labeled-field-required-msg branch January 15, 2025 00:12
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (3e40728) to head (f09dcda).
Report is 1 commits behind head on feature/labeled-field.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##   feature/labeled-field   #2425   +/-   ##
=============================================
=============================================

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e40728...f09dcda. Read the comment docs.

beaesguerra added a commit that referenced this pull request Jan 16, 2025
… `main` (#2429)

## Summary:
This PR includes the following commits:
- LabeledField (part 1): Initial set up for LabeledField component based on FieldHeading (#2322)
- LabeledField(part2): refactor to a function component (#2338)
- LabeledField(part3): Wire up attributes (#2339)
- LabeledField(part4): styling and error icon (#2344)
- LabeledField: integrate with field components and fixes (#2399)
- Use LabeledField in other component stories (#2400)
- Add more docs around different browser + screen reader behaviours for LabeledField (#2403)
- LabeledField: Make sure custom required message is shown (#2425)
- LabeledField: Add a story for custom jsx for props (#2426)
- LabeledField: fix import (#2427)

Issue: WB-1499

## Test plan:
- Confirm LabeledField is working as expected `?path=/docs/packages-labeledfield--docs`
- Review docs for LabeledField: `?path=/docs/packages-labeledfield--docs`

Author: beaesguerra

Reviewers: beaesguerra, jandrade

Required Reviewers:

Approved By: jandrade

Checks: ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ⏭️  Publish npm snapshot, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ⏭️  Publish npm snapshot, ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ gerald, ⌛ undefined

Pull Request URL: #2429
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