Revert TextField state styling #2309

beaesguerra · 2024-08-28T16:54:47Z

Summary:

The new focus styling in the TextField can sometimes be cropped off in webapp if an ancestor has overflow: hidden. This
reverts the changes (commit 982f680) to unblock updating other WB dependencies in webapp (https://github.com/Khan/webapp/pull/25010). A proper fix will be made in a follow up PR!

Ran: git revert 982f680865ad1ffc5fb6a0743fdf68c3853a65dc

Issue: WB-1746

Test plan:

Confirm TextField styling is the same as before (ie. no additional focus outline) (?path=/story/packages-form-textfield--default)

changeset-bot · 2024-08-28T16:54:51Z

🦋 Changeset detected

Latest commit: a73fd2c

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-28T16:56:56Z

Size Change: -20 B (-0.02%)

Total Size: 99.9 kB

Filename	Size	Change
`packages/wonder-blocks-form/dist/es/index.js`	6.17 kB	-20 B (-0.32%)

ℹ️ 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.44 kB
`packages/wonder-blocks-data/dist/es/index.js`	6.24 kB
`packages/wonder-blocks-dropdown/dist/es/index.js`	17.9 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.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.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}

github-actions · 2024-08-28T16:57:30Z

A new build was pushed to Chromatic! 🚀

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

Chromatic results:

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

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

khan-actions-bot · 2024-08-28T17:08:35Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/chilled-pumpkins-accept.md, 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-28T17:09:18Z

npm Snapshot: Published

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

Packages can also be installed manually by running:

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

beaesguerra · 2024-08-28T19:20:42Z

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

I tested the WB PR snapshot with webapp and it reverts the TextField styling correctly. I will also confirm in @somewhatabstract's webapp wbupdate PR znd once these changes are merged and released!

Before with `@khanacademy/wonder-blocks-form@4.9.1`	After with `@khanacademy/wonder-blocks-form@0.0.0-PR2309-20240828170905`

jandrade · 2024-08-28T19:50:31Z

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

-        ":focus-visible": {
-            outline: "none",
-            boxShadow: `0 0 0 1px ${color.white}, 0 0 0 3px ${color.offBlack32}`,
+            color: color.offBlack32,


question: Is this going to be modified in the future PR? See related comment here: https://www.chromatic.com/test?appId=5e1bf4b385e3fb0020b7073c&id=66cf79973b48a29341523038

Yes that's right! I'll put all these changes back in the future PR and then fix the outline styles so it can work in all cases. These changes will just revert the TextField styles to what they were before!

got it, sounds good, thanks for the clarification :)

jandrade

The revert looks good. It's unfortunate that we have to do this, as using :focus-visible + outline is the approach that makes more sense, but I totally understand that it's better to be cautious on this change (better safe than sorry).

beaesguerra · 2024-08-28T20:01:19Z

It's unfortunate that we have to do this, as using :focus-visible + outline is the approach that makes more sense, but I totally understand that it's better to be cautious on this change (better safe than sorry)

@jandrade I'll look into a solution so that maybe we can still do this! I was tinkering around with a negative outline offset and testing the different states. Stay tuned! 😄

This reverts commit 9ec147a.

…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 self-assigned this Aug 28, 2024

beaesguerra force-pushed the revert-text-field-styling branch from 470b6b9 to a7c9219 Compare August 28, 2024 17:00

beaesguerra added 2 commits August 28, 2024 10:06

Revert "Update TextField styling for consistency with other component…

93ea0b4

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

update changeset

a73fd2c

beaesguerra force-pushed the revert-text-field-styling branch from a7c9219 to a73fd2c Compare August 28, 2024 17:06

beaesguerra marked this pull request as ready for review August 28, 2024 17:08

khan-actions-bot requested a review from a team August 28, 2024 17:08

jandrade reviewed Aug 28, 2024

View reviewed changes

jandrade approved these changes Aug 28, 2024

View reviewed changes

beaesguerra merged commit 9ec147a into main Aug 28, 2024
43 of 45 checks passed

beaesguerra deleted the revert-text-field-styling branch August 28, 2024 20:03

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

Revert "Revert TextField state styling (#2309)"

c81b661

This reverts commit 9ec147a.

beaesguerra mentioned this pull request Sep 10, 2024

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

Merged

beaesguerra added a commit that referenced this pull request Sep 10, 2024

Revert "Revert TextField state styling (#2309)"

a792f49

This reverts commit 9ec147a.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert TextField state styling #2309

Revert TextField state styling #2309

beaesguerra commented Aug 28, 2024 •

edited

Loading

changeset-bot bot commented Aug 28, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading

khan-actions-bot commented Aug 28, 2024

github-actions bot commented Aug 28, 2024 •

edited

Loading

beaesguerra commented Aug 28, 2024

jandrade Aug 28, 2024

beaesguerra Aug 28, 2024

jandrade Aug 28, 2024

jandrade left a comment

beaesguerra commented Aug 28, 2024

Revert TextField state styling #2309

Revert TextField state styling #2309

Conversation

beaesguerra commented Aug 28, 2024 • edited Loading

Summary:

Test plan:

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

🦋 Changeset detected

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

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

A new build was pushed to Chromatic! 🚀

Chromatic results:

khan-actions-bot commented Aug 28, 2024

Gerald

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

npm Snapshot: Published

beaesguerra commented Aug 28, 2024

jandrade Aug 28, 2024

Choose a reason for hiding this comment

beaesguerra Aug 28, 2024

Choose a reason for hiding this comment

jandrade Aug 28, 2024

Choose a reason for hiding this comment

jandrade left a comment

Choose a reason for hiding this comment

beaesguerra commented Aug 28, 2024

beaesguerra commented Aug 28, 2024 •

edited

Loading

changeset-bot bot commented Aug 28, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading