feat(textfield, textarea): migrate to spectrum 2#2856
feat(textfield, textarea): migrate to spectrum 2#2856rise-erpelding merged 26 commits intospectrum-twofrom
Conversation
🦋 Changeset detectedLatest commit: dcc9e1a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
🚀 Deployed on https://pr-2856--spectrum-css.netlify.app |
File metricsSummaryTotal size: 1.37 MB*
textfield
* An ASCII character in UTF-8 is 8 bits or 1 byte. |
6b9897f to
a275b4b
Compare
a275b4b to
74ee789
Compare
6787366 to
0ae0bcb
Compare
ef18f8a to
644cb46
Compare
a19f685 to
69230b0
Compare
components/textfield/index.css
Outdated
| /* keyboard focus */ | ||
| .is-keyboardFocused & { | ||
| .is-keyboardFocused &, | ||
| &:focus-visible { |
There was a problem hiding this comment.
The way textfield was before, it didn't have that nice focus outline when you keyboard focus, and that didn't seem quite right.
|
|
||
| .is-disabled &, | ||
| .is-disabled:hover & { | ||
| color: var(--highcontrast-textfield-text-color-disabled, var(--mod-textfield-text-color-disabled, var(--spectrum-textfield-text-color-disabled))); |
There was a problem hiding this comment.
Ensures we get a disabled color for placeholder text if both disabled and read-only are on
There was a problem hiding this comment.
Phew, that was a big one!! I found some places where you're pointing to the correct token, but it seems like the value might be off (which you already mentioned). Maybe once another round of tokens gets released, some of those will take care of themselves.
I did find a weird custom property that I would want to see if you were getting too. Some --system prefixed properties are overriding the correct --spectrum properties, but I don't know where those are coming from.
Other than that, I found a couple of small things! Excellent work!
| &, | ||
| .spectrum-Textfield-input::placeholder { | ||
| --highcontrast-textfield-text-color-valid: CanvasText; | ||
| --highcontrast-textfield-text-color-invalid: CanvasText; | ||
| --highcontrast-textfield-text-color-default: CanvasText; | ||
| --highcontrast-textfield-text-color-hover: CanvasText; | ||
| --highcontrast-textfield-text-color-focus: CanvasText; | ||
| --highcontrast-textfield-text-color-focus-hover: CanvasText; | ||
| --highcontrast-textfield-text-color-keyboard-focus: CanvasText; | ||
| --highcontrast-textfield-text-color-disabled: GrayText; | ||
| --highcontrast-textfield-text-color-readonly: CanvasText; |
There was a problem hiding this comment.
Consolidating because placeholder and input value styles should be the same
| grid-column: 1 / span 2; | ||
|
|
||
| /* prevents unexpected color changes for border in high contrast mode */ | ||
| forced-color-adjust: none; |
There was a problem hiding this comment.
Forces the input to use the styles from CSS and not the default, so that we won't get that border flash. But since WHCM is now relying on the styles set for the input, we have to set the highcontrast background color or it turns white in WHCM.
|
|
||
| /* Disabled Colors */ | ||
| --spectrum-textfield-background-color-disabled: var(--highcontrast-textfield-background-color-disabled, var(--mod-textfield-background-color-disabled, var(--spectrum-gray-25))); | ||
| --spectrum-textfield-border-color-disabled: var(--highcontrast-textfield-border-color-disabled, var(--mod-textfield-border-color-disabled, var(--spectrum-disabled-border-color))); |
There was a problem hiding this comment.
So this disabled border color set according to the specs, but I think the read-only styles are overriding it somewhere. When I toggle the disabled control, the entire textfield disappears.
There was a problem hiding this comment.
Screen.Recording.2025-03-28.at.10.16.43.AM.mov
components/textfield/index.css
Outdated
| .spectrum-Textfield.is-invalid:hover:not(.is-disabled) &, | ||
| .spectrum-Textfield.is-invalid &:hover:not(.is-disabled) { |
There was a problem hiding this comment.
Now that we have the is-hover class, do we need to add that selector here at all? Invalid + Disabled + hover looks right on the Chromatic tests, but then if I actually hover over it with a cursor, I get the border color to change.
Or maybe it just needed to change a bit: .spectrum-Textfield.is-invalid:not(.is-disabled)&:hover? I might not be in the selector...
There was a problem hiding this comment.
Screen.Recording.2025-03-31.at.2.42.28.PM.mov
There was a problem hiding this comment.
@rise-erpelding I noticed a similar thing with the invalid+disabled+otherStates. When I toggled random controls, I happened to notice the first one, and then dug some more (with the controls and the keyboard). I'm decently confident that your CSS is right, and that this is just a side effect of/issue with storybook controls, so feel free to disregard! I'm going to approve anyways, since I can't actually recreate them outside of the storybook controls 🤞
Invalid+Disabled+Focus (I think this might just be a storybook only thing- I can't actually click or tab to this)

Invalid+Disabled+KeyboardFocus (same here- I think this is just storybook-only, where the controls create something that doesn't actually trigger/happen with a mouse/keyboard)

Invalid+Disabled or Readonly+FocusHover or KeyboardFocus (same with these)


- Fixes a regression caused when generic selectors like `.is-invalid` were scoped to `.spectrum-Textfield`. - Throws in an additional removal of a readonly & disabled :hover selector that appeared to be unnecessary.

Description (updated as of 2/25/25!)
CSS-767
CSS-768
This work migrates the Textfield and Textarea components to Spectrum 2 - these components share many tokens. Earlier iterations of this PR did incorporate changes to both components, but they've since been split into two separate (but tightly coupled) components which necessitated cards for each component's migration. Thus, this work includes migration work for Textfield and for Textarea.
Migration Work
Fixes
--spectrum-textfield-font-<prop>custom properties instead of its own custom properties::afterfocus indicator styles removedStorybook Updates
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps (updated as of February 25, 2025)
Check the Storybook deployment preview for textfield or check the branch out locally:
@jawinn
mainRegression testing
Validate:
Screenshots
Before S2 migration:

After S2 migration (updated 7/22/24):

To-do list
Design review/approval