-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VisuallyHidden] Updating Visually Hidden CSS #4641
Conversation
size-limit report
|
34a5bb0
to
e8e1e62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding some context from Slack to this issue. This is the approach I am most familiar with:
clip-path: inset(50%);
height: 1px;
overflow: hidden;
position: absolute;
white-space: nowrap;
width: 1px;
top
and left
values should be removed. clip
is depreacted. I would love to know if you have any opinions @sophschneider. I read this great blog post recently diving deep into this problem. I also appreciated the conversation in the HTML5 boilerplate.
@alex-page read the blog and thread, they were both super interesting and helpful! I'm going to use your snippet and test it on a bunch of browsers and screen readers, I might add in or take out some styles from the threads if I run into the same problems they did (announcing order, weird focus ring, etc.) |
c9f44bf
to
2693175
Compare
525dac1
to
291b978
Compare
d37f6db
to
35ff532
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good just make sure to reenable the no-important lint rule and then 🚢
ee691ed
to
b68bc47
Compare
👋 Hi! We've run into a couple of bugs in online-store-web as a result of this change. In one instance, we were able to create a workaround:
However, the bug also occurs on the ThemesIndex (video included below), and there are too many components using Would there be any strong objections to me re-adding cc @sophschneider, @alex-page , @alexandcote
Screen.Recording.2022-02-18.at.10.59.46.mov |
hi @emma-boardman! no objections from me but it is kind of weird that |
WHY are these changes introduced?
The visually hidden styles in polaris uses
top
which unnecessarily pushes hidden content to the top. it also uses the deprecatedclip
style. This PR updates the styles to use the GovUK design system visually hidden styles discussed here. The styles have been tested extensively as seen here. I tested the styles myself on mac Voice Over.VO demo of old styles: https://videobin.shopify.io/v/1XbjO4
VO demo of new styles: https://videobin.shopify.io/v/bbj2Aj
old styles, note the
top
attribute, this can be much more drastic if the container is tall:new styles:
WHAT is this pull request doing?
updates styles for visually hidden
Playground code
🎩 checklist
README.md
with documentation changes