-
Notifications
You must be signed in to change notification settings - Fork 4.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
Improve accessibility story for custom focus styles #2460
Conversation
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.
Great improvement! I did a simple recreation in CodePen with shadow-outline
and compared the existing outline-none
with the new implementation.
As described, the new implementation has no impact on existing focus styles, but looks great in High Contrast Mode with a dedicated focus indicator. I think the free upgrade by sticking with the same class name is the best approach.
Excited to see this go live!
I actually use outline-none because I don't want an outline at all. I think enforcing it to output an outline for accessibility etc is a bit far if that's the case it should be a configure option like you said but to me the term "none" is no outline etc even if you add outline-0 but I would use that kind of terminology to say something is none then actually has a 2px outline output. That's said I do think outline should be configurable. |
@uncodable Can you share an example of a situation where using a transparent outline instead of removing the outline would cause an issue in your projects? Definitely don't want to merge this if we are going to regret not just doing something very explicit, but if there's no good reason not to do it this way then I really like the benefits this offers. |
@adamwathan Sure, take into account I understand the claims for accessibility about this PR, but I feel like the user should be responsible for that to a certain degree. If I use I feel like the I personally wouldn't merge I hope this feedback helps, It wasn't to shut this PR down or anything like that just thought I'd address my concerns with this as to me if this got merged it would also be a breaking change in terms of how our UI works. Keep up the good work <3 |
Hey thanks for providing more detail! I still can't quite tell where using a transparent outline would actually cause an issue in your case though — what would the actual difference between |
@uncodable Can you elaborate on how this would impact native-oriented projects? With the use cases you mentioned, are you using I'm not sure I follow the "target users" aspect of this, unless products are intentionally excluding users who rely on assistive technology. I think there are many different ways to handle the implementation/class naming, but I'm not sure about letting users remove outlines simply because they don't like them/see a need for them, when users actually depend on them to make a product usable. The hope is to support high contrast mode users with no impact to non-high contrast mode design or layout. |
I can think of one use case for Perhaps the best solution to this is just to add a |
f3893d1
to
55dcc53
Compare
b2a2605
to
6b13f4a
Compare
Made a couple updates:
The configurable outline stuff accepts a single value or a tuple, where the second item is the offset:
I considered adding individual utilities for Another benefit of making the outline stuff configurable is that if someone really hates that If someone hates the available black and white dotted outlines, they can override those too. |
66503a3
to
4cde919
Compare
@adamwathan I wondering if outline styling will make a comeback and box-shadow go back to doing what it's supposed to do as |
@skattabrain 100% agree, hoping that's the case! |
Windows has a high contrast mode which disables all styling it considers superficial, and renders the UI in ultra high contrast for users with low vision. This presents a problem when you try to use box shadows for custom focus styles alongside
focus:outline-none
, because for WHC users, the box shadow is not rendered, and sinceoutline-none
has historically just setoutline: 0
, WHC users see no outline either, so they have no way to tell that the element is focused.This PR changes
outline-none
to use the following implementation:For non-WHC users, the rendered result is the same because the outline is transparent and therefore invisible. But WHC users will now see a 2px solid outline, since WHC ignores most color preferences, including setting the outline to transparent.
I considered adding this as a new
outline-transparent
utility but that name is annoyingly long, and I think the responsible thing to do is "auto-upgrade" all existing usage ofoutline-none
to help Tailwind users make their existing sites more accessible without making any changes. I'm not going to classify this as a breaking change since there should be no perceived difference in behavior.Also considered the name
outline-hidden
, but again it feels more responsible to change the existing behavior (since 99% of people are usingoutline-none
to hide focus styles) instead of forcing people to change their HTML.I considered also adding a new
outline-0
utility for people who really want to completely disable the outline for one reason or another, but have decided not to as I think it would be too easy to use by mistake instead ofoutline-none
.I'm not totally opposed to making
outline
a configurable property if anyone has a good argument for it but honestly I have never once in my life used it for anything but hiding focus styles.Appreciate any thoughts or input on this one 👍