Skip to content
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

Merged
merged 4 commits into from
Oct 8, 2020

Conversation

adamwathan
Copy link
Member

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 since outline-none has historically just set outline: 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:

.outline-none {
  outline: 2px solid transparent;
  outline-offset: 1px;
}

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 of outline-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 using outline-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 of outline-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 👍

Copy link

@davidluhr davidluhr left a 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!

@n10000k
Copy link

n10000k commented Sep 29, 2020

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.

@adamwathan
Copy link
Member Author

@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.

@n10000k
Copy link

n10000k commented Sep 29, 2020

@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 outline: none (or outline: 0) is an option within css I think we should keep it within the framework, I guess users will have their own use cases to why they remove it and keep it (a11y), I guess that depends on what they're building and possibly their target users also.

I use outline-none within our electron app that has certain styling to elements that I don't want to have the standard outline. (I also know someone who just said to me they're using it in a three.js ui also which they removed it from) Please take into account that today tailwind isn't just used to make websites, dashboards etc but Is also used with frameworks such as React Native, Electron etc. I know it would be ideal to have everything accessible but with tailwind becoming bigger and bigger by the day the use case for the applications varies quite a lot.

I feel like the outline utility should be reworked and options added like you said outline-0 xyz, I also feel like we are missing styles such as like dotted, dashed, solid... and the rest, which I guess you could argue could be a plugin or you just make a class for it.

I personally wouldn't merge outline-none from 0 to suddenly a 2px outline, feel like the naming on that convention doesn't work well.

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

@adamwathan
Copy link
Member Author

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 outline: none and outline: 2px solid transparent be in practice in your situation?

@davidluhr
Copy link

@uncodable Can you elaborate on how this would impact native-oriented projects? With the use cases you mentioned, are you using outline-none to reset outline styles, and then providing a custom outline or different focus indicator?

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.

@ghost
Copy link

ghost commented Oct 4, 2020

I can think of one use case for outline: 0, which is on an element with tabindex="-1". You generally want to remove all focus styles here as the element isn’t genuinely focusable in the same way a button is. The point of being able to focus at all is that is can be scrolled into view or can move screenreaders to the relevant content (a popover, say, or a heading). It would seem weird in high-contrast mode that we had a focus ring.

Perhaps the best solution to this is just to add a outline: 0 property to [tabindex="-1"] in the reset? Then you don’t need a separate utility; outline-none can Just Work as intended in this PR.

@adamwathan
Copy link
Member Author

adamwathan commented Oct 8, 2020

Made a couple updates:

  • Now the outline stuff is configurable, so there's an outline key in your config file where you can override stuff
  • Added outline-white and outline-black by default, which are 2px dotted outlines with a 2px offset. Idea is these can be used as universal focus indicators, and are the two highest contrast options possible.

The configurable outline stuff accepts a single value or a tuple, where the second item is the offset:

outline: {
  purple: '2px solid purple',
  blue: ['2px dotted blue', '2px'],
}

I considered adding individual utilities for outline-color, outline-style, outline-width, and outline-offset but after researching many sites, no one is using outline for anything basically ever, and adding all those colors is a lot of bloat for no benefit. Those can be handled in a third party plugin if someone ever wants them, but I think just handling the shorthand property + including support for defining a baked in offset will handle 99.99999999% of peoples' needs.

Another benefit of making the outline stuff configurable is that if someone really hates that outline-none now does a transparent outline by default, they can easily override it in their config.

If someone hates the available black and white dotted outlines, they can override those too.

@skattabrain
Copy link

@adamwathan I wondering if outline styling will make a comeback and box-shadow go back to doing what it's supposed to do as focus-visible use becomes widespread. Only really need focus rings when flying sans mouse.

@adamwathan
Copy link
Member Author

@skattabrain 100% agree, hoping that's the case!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants