feat(textfield, textarea): migrate to spectrum 2 #2856

rise-erpelding · 2024-06-21T22:04:15Z

Description (updated as of 2/25/25!)

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

[NEW 2/25] min-width for textfield is now using the multiplier token as a multiplier (multiplier times component height), resulting in min-width having a unit value (rather than a unitless value).
[NEW 2/25] focus hover borders for default and invalid textfield inputs match the spec
Medium sizing is now the default
The textfield component was checked against the tokens design spec and any discrepancies were adjusted
Textarea tokens also updated to match spec
- Note that there are some tokens that currently have different values than what is seen in Figma, this is (for the moment) intentional - see notes in Figma
New tokens added to match design spec
Quiet variant removed

Fixes

[NEW 2/25] Fixes disabled readonly text color where it was not displaying as disabled color
[NEW 2/25] Character count uses --spectrum-textfield-font-<prop> custom properties instead of its own custom properties
[NEW 2/25] ::after focus indicator styles removed
Makes text field keyboard focusable where it was not before
Improves the disabled state in WHCM to look different from default state
Reorders and reorganizes custom properties

Storybook Updates

[NEW 2/25] Adds a variety of new tests and removes redundant test cases where possible
[NEW 2/25] Updates existing tests to use more realistic example copy (in line with this guidance) and text that matches guidelines
[NEW 2/25] Adds new control for hovered state, and adds description to required control
[NEW 2/25] Adds Placeholder story to highlight that we shouldn't use the placeholder
[NEW 2/25] Adds Required stories to textfield and textarea, supporting the ability (at least in docs) to set required attribute with asterisk or with "(required)" hint text in label
[NEW 2/25] Uses S2 icons and sets checkmark size in template (also set in CSS)
Removes the quiet variant from Storybook docs and tests

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

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

Screenshots

Before S2 migration:

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

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
Updated S2 Icons have been added
~~Design review/approval~~
Code review/approval
✨ This pull request is ready to merge. ✨

changeset-bot · 2024-06-21T22:04:19Z

🦋 Changeset detected

Latest commit: dcc9e1a

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

This PR includes changesets to release 1 package

Name	Type
@spectrum-css/textfield	Major

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

github-actions · 2024-06-21T22:07:06Z

🚀 Deployed on https://pr-2856--spectrum-css.netlify.app

github-actions · 2024-06-21T22:10:08Z

File metrics

Summary

Total size: 1.37 MB^*

Package	Size	Minified	Gzipped
textfield	31.83 KB	30.42 KB	3.60 KB

textfield

Filename	Head	Minified	Gzipped	Compared to base
index.css	31.83 KB	30.42 KB	3.60 KB	🟢 ⬇ 0.64 KB
metadata.json	17.01 KB	-	-	🔴 ⬆ 0.06 KB

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

components/textfield/index.css

components/textfield/stories/template.js

rise-erpelding · 2024-07-09T20:24:47Z

components/textfield/index.css

@@ -410,7 +410,8 @@ governing permissions and limitations under the License.
 	}

 	/* keyboard focus */
-	.is-keyboardFocused & {
+	.is-keyboardFocused &,
+	&:focus-visible {


The way textfield was before, it didn't have that nice focus outline when you keyboard focus, and that didn't seem quite right.

rise-erpelding · 2024-07-09T20:27:01Z

components/textfield/index.css

+
+			.is-disabled &,
+			.is-disabled:hover & {
+				color: var(--highcontrast-textfield-text-color-disabled, var(--mod-textfield-text-color-disabled, var(--spectrum-textfield-text-color-disabled)));


Ensures we get a disabled color for placeholder text if both disabled and read-only are on

marissahuysentruyt

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!

components/textfield/stories/template.js

components/textfield/index.css

components/textfield/stories/template.js

components/textfield/stories/textfield.stories.js

components/textfield/stories/template.js

rise-erpelding · 2025-03-28T13:39:22Z

components/textfield/index.css

+		&,
+		.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;


Consolidating because placeholder and input value styles should be the same

rise-erpelding · 2025-03-28T13:41:47Z

components/textfield/index.css

@@ -345,11 +345,15 @@
 	grid-row: 2;
 	grid-column: 1 / span 2;

+	/* prevents unexpected color changes for border in high contrast mode */
+	forced-color-adjust: none;


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.

marissahuysentruyt · 2025-03-28T14:17:04Z

components/textfield/index.css

@@ -76,6 +86,8 @@
 	--spectrum-textfield-text-color-readonly: var(--spectrum-neutral-content-color-default);

 	/* 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)));


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.

Screen.Recording.2025-03-28.at.10.16.43.AM.mov

marissahuysentruyt · 2025-03-31T18:47:57Z

components/textfield/index.css

+	.spectrum-Textfield.is-invalid:hover:not(.is-disabled) &,
+	.spectrum-Textfield.is-invalid &:hover:not(.is-disabled) {


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...

Screen.Recording.2025-03-31.at.2.42.28.PM.mov

@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)

components/textfield/dist/metadata.json

- 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.

marissahuysentruyt

Thanks for sticking with this one! Let's get it merged!

rise-erpelding added the wip This is a work in progress, don't judge. label Jun 21, 2024

rise-erpelding added skip_vrt Add to a PR to skip running VRT (but still pass the action) S2 Spectrum 2 labels Jun 21, 2024

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch 2 times, most recently from 6b9897f to a275b4b Compare June 24, 2024 21:29

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch from a275b4b to 74ee789 Compare July 1, 2024 23:05

rise-erpelding commented Jul 1, 2024

View reviewed changes

components/textfield/index.css Outdated Show resolved Hide resolved

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch 3 times, most recently from 6787366 to 0ae0bcb Compare July 5, 2024 18:15

rise-erpelding commented Jul 5, 2024

View reviewed changes

components/textfield/index.css Outdated Show resolved Hide resolved

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch 4 times, most recently from ef18f8a to 644cb46 Compare July 8, 2024 15:40

rise-erpelding commented Jul 9, 2024

View reviewed changes

components/textfield/stories/template.js Outdated Show resolved Hide resolved

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch 2 times, most recently from a19f685 to 69230b0 Compare July 9, 2024 20:19

rise-erpelding commented Jul 9, 2024

View reviewed changes

components/textfield/stories/template.js Outdated Show resolved Hide resolved

rise-erpelding commented Jul 9, 2024

View reviewed changes

rise-erpelding added ready-for-review and removed wip This is a work in progress, don't judge. labels Jul 9, 2024

rise-erpelding marked this pull request as ready for review July 9, 2024 20:27

rise-erpelding requested review from jawinn and marissahuysentruyt July 9, 2024 20:28

marissahuysentruyt requested changes Jul 18, 2024

View reviewed changes

rise-erpelding added 11 commits March 28, 2025 06:34

fix: remove :focus-visible selectors, see #2306

9dca22d

docs: invalid stories separated, update story formatting

cfb0d54

docs: remove isLoading from controls

c7eefb5

docs: add control descriptions

0e15d15

test: add disabled & hover

3338255

fix: scope generic selectors to .spectrum-Textfield

710cd4b

fix: add WHCM readonly border, disabled bg color

a4fafb3

fix: adjust WHCM placeholder text to match value

9976179

fix: adjust WHCM keyfocus border color to match spec

647ddd9

fix: adjust high contrast focus hover colors

16406fa

fix: prevent WHCM orange flash

5d0c8e0

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch from ca87f08 to 5d0c8e0 Compare March 28, 2025 11:34

rise-erpelding commented Mar 28, 2025

View reviewed changes

marissahuysentruyt reviewed Mar 28, 2025

View reviewed changes

fix: readonly & disabled selector

e3ba0dd

rise-erpelding requested a review from marissahuysentruyt March 31, 2025 15:58

marissahuysentruyt reviewed Mar 31, 2025

View reviewed changes

github-actions bot reviewed Apr 1, 2025

View reviewed changes

components/textfield/dist/metadata.json Outdated Show resolved Hide resolved

fix: remove invalid border colors

dcc9e1a

- 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.

rise-erpelding force-pushed the rise-erpelding/css-767-textfield-s2-migration branch from 13969cf to dcc9e1a Compare April 1, 2025 14:30

rise-erpelding requested a review from marissahuysentruyt April 1, 2025 14:39

marissahuysentruyt approved these changes Apr 1, 2025

View reviewed changes

jawinn approved these changes Apr 1, 2025

View reviewed changes

rise-erpelding merged commit 19ca328 into spectrum-two Apr 1, 2025
12 checks passed

rise-erpelding deleted the rise-erpelding/css-767-textfield-s2-migration branch April 1, 2025 20:29

github-actions bot mentioned this pull request Apr 1, 2025

chore: release (next) #3653

Merged

castastrophe mentioned this pull request Apr 15, 2025

feat(tokens,card): update to @adobe/spectrum-tokens@13.3.0 #3669

Merged

12 tasks

This was referenced May 16, 2025

chore(swatch,menu,swatchgroup): fix reverted metadata assets #3752

Merged

chore: merge latest updates from main #3777

Merged

		.spectrum-Textfield.is-invalid:hover:not(.is-disabled) &,
		.spectrum-Textfield.is-invalid &:hover:not(.is-disabled) {

feat(textfield, textarea): migrate to spectrum 2 #2856

feat(textfield, textarea): migrate to spectrum 2 #2856

Uh oh!

Conversation

rise-erpelding commented Jun 21, 2024 • edited by jawinn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (updated as of 2/25/25!)

Migration Work

Fixes

Storybook Updates

How and where has this been tested?

Validation steps (updated as of February 25, 2025)

Regression testing

Screenshots

To-do list

Uh oh!

changeset-bot bot commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

textfield

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rise-erpelding commented Jun 21, 2024 •

edited by jawinn

Loading

changeset-bot bot commented Jun 21, 2024 •

edited

Loading

github-actions bot commented Jun 21, 2024 •

edited

Loading

github-actions bot commented Jun 21, 2024 •

edited

Loading

marissahuysentruyt left a comment •

edited

Loading