-
Notifications
You must be signed in to change notification settings - Fork 85
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
♿️(styles) improve the offscreen class #1645
♿️(styles) improve the offscreen class #1645
Conversation
position: absolute !important; | ||
width: 1px !important; | ||
height: 1px !important; | ||
padding: 0 !important; | ||
margin: -1px !important; | ||
overflow: hidden !important; | ||
clip: rect(0, 0, 0, 0) !important; | ||
white-space: nowrap !important; | ||
border: 0 !important; |
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.
From this article, I think you should add clip-rect
below clip
as this last one has been deprecated in favor of the first.
Furthermore, I think you should remove !important
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.
I'm with @jbpenrath on the !important
part. It makes things more complex to debug if we ever override something in the future.
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.
You meant clip-path
I guess. There were a few discussions on this in the bootstrap project, and not using it is intentional, even when knowing it's deprecated, i'll let you check twbs/bootstrap#25197
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.
Will remove the !important
bits ;).
edit: done.
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.
So what do you think @jbpenrath about the clip-path issue? Seems to have noticeable perfs issues because of a Chrome bug. My choice would be to leave it as is, but as you wish, let me know.
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.
I didn't know about that. Since february 2018, the problem seems to be fixed. I tried an example from my chromium based browser and there is no more painting issue. https://bugs.chromium.org/p/chromium/issues/detail?id=611257
So I think we could use it today.
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.
Good find, updated 👍
3482ca7
to
889345d
Compare
Change the implementation of the offscreen class to match the one from bootstrap that is battle-tested. The main idea is to stop using "left: -100000px" that can cause scrolling "flicker" or weird focus styles when using a screen reader. This can be annoying for sighted people that use a screen reader.
889345d
to
099e5b8
Compare
Change the implementation of the offscreen class to match the one from bootstrap that is battle-tested.
The main idea is to stop using
left: -100000px
that can cause scrolling "flicker" or weird focus styles when using a screen reader, as that can be annoying for sighted people that use a screen reader.