Update TextField styling for consistency with other components (focus styling) #2302

beaesguerra · 2024-08-23T14:16:01Z

Summary:

Update TextField state styling so it is consistent with other components like TextArea, SingleSelect, MultiSelect (especially the focus styling)
The styling changes use CSS pseudo-classes now so that we can have visual regression tests for the UI states using Storybook's pseudo-states add on.
These styles align with the Figma TextField component
Removed unneeded focused state since we use pseudo classes now
Added variant stories for Chromatic tests

Note: These styles are based on how TextArea was styled!

Issue: WB-1746

Test plan:

Confirm the focus styling for TextField when (?path=/docs/packages-form-textfield--docs):

it is focused
it is focused on a dark background with the light prop set to true
it is in an error state and is focused
it is in an error state on a dark background with the light prop set to true

Confirm other states (error, disabled, default)
Review the new variant stories (?path=/docs/packages-form-textfield-all-variants--docs)

…es pseudo-classes so we can have visual regression tests using the pseudostates storybook add on

…tests to test this

changeset-bot · 2024-08-23T14:16:04Z

🦋 Changeset detected

Latest commit: d051a64

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

This PR includes changesets to release 4 packages

Name	Type
@khanacademy/wonder-blocks-form	Patch
@khanacademy/wonder-blocks-search-field	Patch
@khanacademy/wonder-blocks-dropdown	Patch
@khanacademy/wonder-blocks-birthday-picker	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

github-actions · 2024-08-23T14:17:56Z

Size Change: +19 B (+0.02%)

Total Size: 96.1 kB

Filename	Size	Change
`packages/wonder-blocks-form/dist/es/index.js`	6.13 kB	+19 B (+0.31%)

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3.78 kB
`packages/wonder-blocks-banner/dist/es/index.js`	1.53 kB
`packages/wonder-blocks-birthday-picker/dist/es/index.js`	1.61 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.45 kB
`packages/wonder-blocks-data/dist/es/index.js`	6.24 kB
`packages/wonder-blocks-dropdown/dist/es/index.js`	14.1 kB
`packages/wonder-blocks-grid/dist/es/index.js`	1.36 kB
`packages/wonder-blocks-i18n/dist/es/index.js`	4.76 kB
`packages/wonder-blocks-icon-button/dist/es/index.js`	2.99 kB
`packages/wonder-blocks-icon/dist/es/index.js`	828 B
`packages/wonder-blocks-labeled-field/dist/es/index.js`	72 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.36 kB
`packages/wonder-blocks-pill/dist/es/index.js`	1.64 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.3 kB
`packages/wonder-blocks-switch/dist/es/index.js`	1.94 kB
`packages/wonder-blocks-testing-core/dist/es/index.js`	3.64 kB
`packages/wonder-blocks-testing/dist/es/index.js`	1.13 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.1 kB
`packages/wonder-blocks-toolbar/dist/es/index.js`	855 B
`packages/wonder-blocks-tooltip/dist/es/index.js`	7 kB
`packages/wonder-blocks-typography/dist/es/index.js`	1.23 kB

_{compressed-size-action}

codecov · 2024-08-23T14:18:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.74%. Comparing base (bc00eb4) to head (d051a64).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2302      +/-   ##
==========================================
+ Coverage   94.15%   94.74%   +0.58%     
==========================================
  Files         254      254              
  Lines       30479    30517      +38     
  Branches     2481     2454      -27     
==========================================
+ Hits        28697    28912     +215     
+ Misses       1777     1591     -186     
- Partials        5       14       +9

Files	Coverage Δ
...es/wonder-blocks-form/src/components/text-area.tsx	`100.00% <100.00%> (ø)`
...s/wonder-blocks-form/src/components/text-field.tsx	`100.00% <100.00%> (+4.76%)`	⬆️

... and 39 files with indirect coverage changes

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 bc00eb4...d051a64. Read the comment docs.

github-actions · 2024-08-23T14:22:21Z

A new build was pushed to Chromatic! 🚀

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

Chromatic results:

Metric	Total
Captured snapshots	151
Tests with visual changes	0
Total stories	482
Inherited (not captured) snapshots [TurboSnap]	213
Tests on the build	364

beaesguerra · 2024-08-23T14:37:58Z

packages/wonder-blocks-form/src/components/__tests__/labeled-text-field.test.tsx

@@ -382,28 +381,6 @@ describe("LabeledTextField", () => {
        expect(input).toBeInTheDocument();
    });

-    it("light prop is passed to textfield", async () => {


We can remove the styling related unit tests since we'll have Chromatic tests to verify the styles!

beaesguerra · 2024-08-23T14:40:40Z

packages/wonder-blocks-form/src/components/text-field.tsx

+            color: color.offBlack64,
+        },
+        cursor: "not-allowed",
+        ":focus-visible": {


Note: The TextField isn't focusable yet when disabled! I created a separate ticket for that work: WB-1751

khan-actions-bot · 2024-08-23T14:43:21Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/famous-walls-scream.md, __docs__/wonder-blocks-form/text-field-variants.stories.tsx, packages/wonder-blocks-form/src/components/text-area.tsx, packages/wonder-blocks-form/src/components/text-field.tsx, packages/wonder-blocks-form/src/components/__tests__/labeled-text-field.test.tsx

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

github-actions · 2024-08-23T14:47:29Z

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (6bd7841) 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="PR2302"

Packages can also be installed manually by running:

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

jandrade

This looks awesome! thanks for the improvements 🎉

jandrade · 2024-08-23T18:42:40Z

packages/wonder-blocks-form/src/components/text-field.tsx

+        }
+    };
+
+    getStyles = (): (React.CSSProperties | Falsy)[] => {


nit: You could also use StyleType from wonder-blocks-core.

Suggested change

getStyles = (): (React.CSSProperties | Falsy)[] => {

getStyles = (): StyleType => {

This is what I needed! Thank you for pointing this out 😄

I also updated this in TextArea since that's where I copied it from!

jandrade · 2024-08-23T18:44:18Z

packages/wonder-blocks-form/src/components/text-field.tsx

+        const defaultStyles = [
+            styles.default,
+            !disabled && styles.defaultFocus,
+            disabled && styles.disabled,
+            !!error && styles.error,
+        ];
+        const lightStyles = [
+            styles.light,
+            !disabled && styles.lightFocus,
+            disabled && styles.lightDisabled,
+            !!error && styles.lightError,
+        ];


praise: Nice cleanup!

jandrade · 2024-08-23T18:48:50Z

packages/wonder-blocks-form/src/components/text-field.tsx

+            outline: `1px solid ${color.blue}`,
+            outlineOffset: 0, // Explicitly set outline offset to 0 because Safari sets a default offset
+            borderColor: color.blue,
+            boxShadow: `0px 0px 0px 2px ${color.blue}, 0px 0px 0px 3px ${color.white}`,


question: Probably out of scope for this PR... do you think this could use outline instead of boxShadow? I don't remember the exact reason why we took this approach.

I think we could! I based the styling on what I implemented in TextArea, and that was based on styling I found in other components (I remember it being tricky - it's using outline, border, and box shadow!). I think we could rework the styles to simplify things. I created WB-1753 for this!

It would also be nice if we could share common state styles between components so it's consistent (kind of like what we do with Typography styles). What do you think?

…s (focus styling) (#2302)" This reverts commit 982f680.

…ropped outline) (#2311) ## Summary: Work was initially done in #2302 to update the TextField state styling so it was consistent with other components like SingleSelect, MultiSelect, and TextArea. The new styling had some issues in webapp where the focus outline would get cropped off if an ancestor element had `overflow: hidden`. The new styles were reverted in #2309 to unblock other WB changes at the time. This PR re-applies those styles and addresses the focus outline issue in both the TextField and TextArea components. The cropped focus outline issue is resolved by making sure the outlines are within the component. This makes it so the outline is never cropped off if a parent or ancestor element has `overflow: hidden`. The changes that address the outline issue are (see 51d9e4f): - using a negative offset for outlines so that they appear within the component (instead of outside) - simplifying outline styling by removing the box-shadow styling. Only border + outline are used for styling now Other alternatives considered: - Keeping the outline outside and adding additional padding + negative margin so there's enough space for the outline - I think adding spacing around our components isn't ideal because spacing is often added to the components and would override the built in spacing - Adding the outline styles to a pseudo element (`:after`) - doesn't work with input elements - Continue using box-shadow - box-shadow styling also gets cropped out. Could use `inset` option though. I think using outline simplifies things though! ## Test Plan - Confirm that the states for TextField and TextArea are as expected. Note: I added `overflow: hidden` to the component variant stories to simulate the issue (see before & after screenshots) - TextField `?path=/docs/packages-form-textfield-all-variants--docs` - TextArea `?path=/docs/packages-form-textarea-all-variants--docs` | Before | After | | --- | --- | | <img width="1840" alt="Screenshot 2024-08-29 at 8 58 51 AM" src="https://github.com/user-attachments/assets/a9e30b3b-39f2-4807-be14-0554b862542e"> | <img width="1840" alt="Screenshot 2024-08-29 at 8 59 06 AM" src="https://github.com/user-attachments/assets/0d812895-5217-496a-ba72-8c312a246168"> | | <img width="1840" alt="Screenshot 2024-08-29 at 8 59 37 AM" src="https://github.com/user-attachments/assets/0f761996-ddaf-44be-80d7-ac14b28a89a4"> | <img width="1840" alt="Screenshot 2024-08-29 at 8 59 14 AM" src="https://github.com/user-attachments/assets/83070398-b67a-47e3-9ba4-0886dd097956"> | - Confirmed in webapp that focus outline is not cropped off in TextArea and TextField components Issue: WB-1746 Author: beaesguerra Reviewers: jandrade Required Reviewers: Approved By: jandrade Checks: ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish npm snapshot (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 (ubuntu-latest, 20.x, 2/2), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Lint (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ⏭️ Publish npm snapshot, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), 🚫 Chromatic - Build 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), ⏭️ Publish npm snapshot, ✅ gerald, ⌛ undefined Pull Request URL: #2311

beaesguerra added 3 commits August 23, 2024 07:31

Update TextField state styling based on TextArea styles. This also us…

c867b93

…es pseudo-classes so we can have visual regression tests using the pseudostates storybook add on

Remove unneeded focused state since we use pseudo classes now

cd0edfa

remove test that looks for specific styles. we can rely on Chromatic …

499ba69

…tests to test this

beaesguerra self-assigned this Aug 23, 2024

Add changeset

7f6b68f

beaesguerra changed the title ~~Update TextField styling~~ Update TextField styling for consistency with other components (focus styling) Aug 23, 2024

linting

ed5b6db

beaesguerra commented Aug 23, 2024

View reviewed changes

beaesguerra marked this pull request as ready for review August 23, 2024 14:43

khan-actions-bot requested a review from a team August 23, 2024 14:43

jandrade approved these changes Aug 23, 2024

View reviewed changes

Use StyleType type instead

d051a64

beaesguerra force-pushed the text-field-states branch from 88f51cb to d051a64 Compare August 23, 2024 20:53

beaesguerra merged commit 982f680 into main Aug 26, 2024
16 checks passed

beaesguerra deleted the text-field-states branch August 26, 2024 21:28

beaesguerra added a commit that referenced this pull request Aug 28, 2024

Revert "Update TextField styling for consistency with other component…

470b6b9

…s (focus styling) (#2302)" This reverts commit 982f680.

beaesguerra added a commit that referenced this pull request Aug 28, 2024

Revert "Update TextField styling for consistency with other component…

a7c9219

…s (focus styling) (#2302)" This reverts commit 982f680.

beaesguerra added a commit that referenced this pull request Aug 28, 2024

Revert "Update TextField styling for consistency with other component…

93ea0b4

…s (focus styling) (#2302)" This reverts commit 982f680.

beaesguerra mentioned this pull request Sep 10, 2024

Updated TextField & TextArea styling (for consistency and addresses cropped outline) #2311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update TextField styling for consistency with other components (focus styling) #2302

Update TextField styling for consistency with other components (focus styling) #2302

beaesguerra commented Aug 23, 2024 •

edited

Loading

changeset-bot bot commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

beaesguerra Aug 23, 2024

beaesguerra Aug 23, 2024

khan-actions-bot commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

jandrade left a comment

jandrade Aug 23, 2024

beaesguerra Aug 23, 2024

beaesguerra Aug 23, 2024

jandrade Aug 23, 2024

jandrade Aug 23, 2024

beaesguerra Aug 23, 2024

	getStyles = (): (React.CSSProperties \| Falsy)[] => {
	getStyles = (): StyleType => {

Update TextField styling for consistency with other components (focus styling) #2302

Update TextField styling for consistency with other components (focus styling) #2302

Conversation

beaesguerra commented Aug 23, 2024 • edited Loading

Summary:

Test plan:

changeset-bot bot commented Aug 23, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Aug 23, 2024 • edited Loading

codecov bot commented Aug 23, 2024 • edited Loading

Codecov Report

github-actions bot commented Aug 23, 2024 • edited Loading

A new build was pushed to Chromatic! 🚀

Chromatic results:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khan-actions-bot commented Aug 23, 2024 • edited Loading

Gerald

github-actions bot commented Aug 23, 2024 • edited Loading

npm Snapshot: Published

jandrade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beaesguerra commented Aug 23, 2024 •

edited

Loading

changeset-bot bot commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

khan-actions-bot commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading