-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Use :focus-visible
polyfill
#30872
Use :focus-visible
polyfill
#30872
Conversation
63663ff
to
e538cb0
Compare
Bundlewatch diff : I think it worths it :) |
I'm still against adding this. It's extra dependency. |
5e378df
to
e4e203b
Compare
Indeed. However it'll allow Bootstrap to make a (big) step further on the accessibility side. Maybe using the polyfill isn't the best option as of now, but I wanted to introduce at least a POC for this concern. I mentionned an alternative already using only future-proof CSS (props to @patrickhlauke): button:focus { /* some exciting button focus styles */ }
@supports selector(:focus-visible) {
button:focus { /* undo all the above focused button styles */ }
button:focus-visible { /* and then reapply the styles here instead */ }
} but this would bloat our styles and wouldn't really work for now—except for Firefox users. We could also implement our own polyfill for this: we already have a polyfill file, and we might not need every case covered by WICG's polyfill (it supports IE11, also supports Shadow DOM, etc.). I'm not fluent enough on the JavaScript side to determine what can be done, but from both an accessibility and a CSS perspective, I think it's the safer way to go. As I mentionned previously, this is only a POC and I'd like to get as much advices as possible to see what can be done :) |
Ok it is an extra dependency but it is usefull on all websites. |
as this shows as closed rather than merged, a silly question...is this now in v5 or not? just asking because i've long been a proponent of adding this (with the polyfill, with an eye towards future proofing), and now that Chrome shipped it ... it's probably a good time to do it. https://blog.chromium.org/2020/09/giving-users-and-developers-more.html |
It's not in v5, this was the latest attempt but adding a polyfill seemed an extra effort that we couldn't afford ATM. FWIW I merged it in Boosted v5 (and it was already in v4 since 4.4.0). |
meanwhile we're adding utility classes for every possible CSS property and value... oh well |
I agree, but I think that what was the main argument is to add a dependency: since we dropped jQuery, only Popper remains. I suggested the polyfill and was the only one in favor of this ATM. Nevertheless, the PR could be restored pretty easily if you dare since as I said, works fine in Boosted currently. |
@patrickhlauke Just to be sure, are you in favor of using the WICG polyfill, or "simply" the CSS enhancement based on |
could try the |
This is a very rough first try to include and use WICG's
:focus-visible
polyfill in Bootstrap. Let me add some background.What
:focus-visible
belong to CSS Selectors level 4 specification. As per the spec:Thus it's a very helpful selector to massively enhance accessibility. I suggest you read "
:focus-visible
and backwards compatibility" by @patrickhlauke for more explanation and even more background.Why
First of all: it's an easy way to safely drop the (sometimes) ugly default
outline
s when activating things using a mouse. This PR, as-is, fixes #27163 and may help with #29875.For years, Bootstrap has been using
box-shadow
for focus states, which can lead to some confusion since there's also a$enable-shadows
variable that may cause some focus style to not render appropriately. See #26802 for an example.But what's more promising here is that it'd allow us to make focus styles stronger and more obvious.
It would allow us to increase contrasts on
:focus
, which had been requested a few times (#23329 and #29422).And I'm quite sure there's probably a few cases with our components where focus could be better. :)
How
There, I need some help from @twbs/js-review :D
I added the dependency, but this definetly require some JS love to be included in the best possible way. For now, I roughly imported the file form the
node_modules
—which can't obviously stay this way.On the CSS side:
outline
s: you can preview this in our docs navbar for example, which got the defaultoutline
until now.[data-focus-visible-added]
attribute instead of the.focus-visible
class since it's environment-proof.2px
wide solidoutline
s, and atransition
onoutline-offset
(which may vary, depending on the component). You can use keyboard to navigate through Boosted website to preview this.Sidenote
@patrickhlauke mentionned in his post in 2018 the potential use of
@supports
—but it couldn't test for selector support at that time. Well, it can now and the support for, say,@supports selector(:focus-visible)
is pretty much the same than support for:focus-visible
.So a future-proof, JS-free way to go might be using
@supports
—but it'd be much, much more verbose on the CSS side.