[WB-1814.2] Refactor TextField and TextArea to use semantic colors #2440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
January 22, 2025 15:11
…or token to use for the focus outline
…semanticColor internally on TextField and TextArea
🦋 Changeset detectedLatest commit: b4b0762 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
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 |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (297735b) 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="PR2440"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2440 |
|
Size Change: +190 B (+0.19%) Total Size: 98 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-mllqkjfcbh.chromatic.com/ Chromatic results:
|
beaesguerra
reviewed
Jan 24, 2025
Comment on lines
+139
to
+148
| color: semanticColor.text.primary, | ||
| }, | ||
| description: { | ||
| color: color.offBlack64, | ||
| color: semanticColor.text.secondary, | ||
| }, | ||
| error: { | ||
| color: color.red, | ||
| color: semanticColor.status.critical.foreground, | ||
| }, | ||
| required: { | ||
| color: color.red, | ||
| color: semanticColor.status.critical.foreground, |
There was a problem hiding this comment.
So clean and straightforward 🤩
| "::placeholder": { | ||
| color: color.offBlack64, | ||
| color: semanticColor.text.secondary, |
There was a problem hiding this comment.
Same comment here around if we want to have a separate token for placeholder text color! (comment from other PR: #2438 (comment))
There was a problem hiding this comment.
Discussed with Design and they are good with this approach.
| border: `1px solid ${color.red}`, | ||
| color: color.offBlack, | ||
| background: semanticColor.status.critical.background, | ||
| border: `${border.width.hairline}px solid ${semanticColor.status.critical.foreground}`, |
There was a problem hiding this comment.
Would it make sense to have semanticColor.status.critical.border here and in the focus-visible state?
There was a problem hiding this comment.
As mentioned in other PRs, the goal is to keep status as it is. I'll create a ticket to also simplify the action tokens once we have something more defined a more stable visual direction for the new experience.
| @@ -4,6 +4,7 @@ const border = { | |||
| primary: color.fadedOffBlack16, | |||
| subtle: color.fadedOffBlack8, | |||
| strong: color.fadedOffBlack50, | |||
| focus: color.blue, | |||
There was a problem hiding this comment.
Same comments from TextArea for TextField since they have similar styles!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2440 +/- ##
============================
============================
Continue to review full report in Codecov by Sentry.
|
This was referenced Jan 29, 2025
jandrade
added a commit
that referenced
this pull request
Jan 31, 2025
## Summary: Continuing with the semantic colors migration, this PR covers the `Switch` component. - Refactored the `Switch` themes to use semantic colors. - Added `All Variants` story to switch. - Disabled chromatic snapshots for existing stories (now using All Variants). ### Implementation plan: 1. #2439 2. #2440 3. Switch (current PR) 4. Accordion, Banner, BirthdayPicker 5. IconButton 6. Clickable, Link 7. Modal 8. Popover, Tooltip 9. Pill, Toolbar Issue: WB-1814 ## Test plan: - Navigate to /?path=/story/packages-switch-switch-all-variants--default - Verify that the styles are correct. Author: jandrade Reviewers: jandrade, beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ 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 (ubuntu-latest, 20.x), ✅ Chromatic - Build and test 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, ⏭️ dependabot Pull Request URL: #2441
jandrade
added a commit
that referenced
this pull request
Jan 31, 2025
…c colors (#2446) ## Summary: Next step is to refactor the `Accordion`, `Banner`, and `BirthdayPicker` components to use semantic colors. ### Implementation plan: 1. #2439 2. #2440 3. #2441 4. Accordion, Banner, BirthdayPicker (current PR) 5. IconButton 6. Clickable, Link 7. Modal 8. Popover, Tooltip 9. Pill, Toolbar Issue: WB-1814 ## Test plan: Verify that the Chromatic snapshots are unchanged. Author: jandrade Reviewers: jandrade, beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ 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 (ubuntu-latest, 20.x), ✅ Chromatic - Build and test on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2446
jandrade
added a commit
that referenced
this pull request
Jan 31, 2025
## Summary: Next step is to refactor the `IconButton` component to use semantic colors. Besides the migration, this PR also includes the following changes: - Reworked the theme structure to make it closer to the semanticColor structure. - Moved all the color-related CSS properties to a separate function (`getStylesByKind`). - Added `border` to all the states to prepare the work for Polaris as these icon buttons will look similar to the Button ones. ### Implementation plan: 1. #2439 2. #2440 3. #2441 4. #2446 5. IconButton (current PR) 6. Clickable, Link 7. Modal 8. Popover, Tooltip 9. Pill, Toolbar Issue: WB-1814 ## Test plan: Verify that the Chromatic snapshots are unchanged. URL: `/?path=/story/packages-iconbutton-all-variants--default` Author: jandrade Reviewers: jandrade, marcysutton, beaesguerra Required Reviewers: Approved By: marcysutton, beaesguerra Checks: ✅ 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 (ubuntu-latest, 20.x), ✅ Chromatic - Build and test on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2449
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Form: Migrated
colorinstances to usesemanticColorinternally inTextField and TextArea.
Tokens: Added
border.focussemantic color token to use for the focus outlineIssue: WB-1814
Test plan:
Navigate to the Storybook for
TextFieldandTextAreacomponents and verifythat the colors are looking as expected.