Updated TextField & TextArea styling (for consistency and addresses cropped outline) #2311
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
🦋 Changeset detectedLatest commit: 52cfc81 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
Size Change: +105 B (+0.11%) Total Size: 100 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-ucxbjlcigq.chromatic.com/ Chromatic results:
|
beaesguerra
force-pushed
the
new-text-field-styling
branch
from
f7c0868 to
a6fcf08
Compare
beaesguerra
force-pushed
the
new-text-field-styling
branch
from
a6fcf08 to
51d9e4f
Compare
beaesguerra
changed the title
|
Chromatic visual tests are available again now! Note that the visual tests includes other visual changes that has been added since it was disabled |
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 (8765f7c) 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="PR2311"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2311 |
jandrade
approved these changes
Sep 16, 2024
beaesguerra
added a commit
that referenced
this pull request
Sep 16, 2024
…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
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:
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):Other alternatives considered:
:after) - doesn't work with input elementsinsetoption though. I think using outline simplifies things though!Test Plan
overflow: hiddento the component variant stories to simulate the issue (see before & after screenshots)?path=/docs/packages-form-textfield-all-variants--docs?path=/docs/packages-form-textarea-all-variants--docsIssue: WB-1746