elad2412 / the-new-css-reset

The New Simple and Lighter CSS Reset
https://elad2412.github.io/the-new-css-reset/
MIT License
2.26k stars 229 forks source link

Focus states & keyboard accessibility #25

Closed lewisl9029 closed 2 years ago

lewisl9029 commented 2 years ago

First of all, thank you so much for building this! I've been trying it out in a few projects and it's honestly been a breath of fresh air to not have to constantly fight with default user agent styles to get things looking exactly the way I want.

That said, I do have some concerns around accessibility of the reset in its default configuration, specifically because it removes the default browser focus outlines. This is the reset working as intended, but as it grows in popularity I fear that people who use it might not realize this and will build apps with really poor accessibility for keyboard users by not providing any custom focus styles on top of the reset.

Some potential ideas to work around this:

  1. We can make an exception for the default focus styles and not override them. This way usages will continue to have a reasonable default level of accessibility for keyboard users.
  2. We can add a note in the readme, ideally very high up, mentioning that the reset removes focus styles, the consequences around that, and strongly recommend users to provide their own.

I would personally vastly prefer 1 as a safer default, but 2 is probably better than nothing if we want to err on the purist side and override as much styling as possible. Definitely open to other ideas as well, and happy to send a PR once we settle on the approach!

SeanMcP commented 2 years ago

I vote for option 1, as well.

elad2412 commented 2 years ago

I know this is unpopular, but the main idea is to remove all the styles, and then the user/developer will add focus styles.

Think that this is the reset project, and on top of it, you can add any new level of styles.

If you check checkboxes and radio buttons, you will see that you can't see them after resetting them. This is because the main idea is to reset, and after that, web developers will add their styles.

SeanMcP commented 2 years ago

This article is a little dated now, but Eric Meyer reflects on – in his words – the "blunder" of removing visible focus styles from his original CSS reset.

That said, this isn't an exact parallel and @elad2412's position isn't unreasonable. But I fear that developers are much more likely to verify that a checkbox is visible than that the focus styles are appropriate.

lewisl9029 commented 2 years ago

@elad2412 would you be open to option 2, i.e. add something prominent to the readme to recommend providing focus styles?

While I completely agree with @SeanMcP that it's probably not going to be enough, and a vast majority of users that adopt this reset will end up ignoring focus states completely, we'll at least be in a slightly better situation than the status quo where it's not mentioned at all.

elad2412 commented 2 years ago

Hi @SeanMcP, This CSS reset's primary goal is to take the CSS reset to the next level of resetting the basic styles which aren't under shadow DOM. I understand your opinion, but this misses this CSS reset's primary goal.

elad2412 commented 2 years ago

Hi @lewisl9029, I added the "Accessibility Recommendation" section on the READ ME.

Issue closed.

zaydek commented 2 years ago

Just wanted to add that this seems to bring back the browser focus styles. There may be some missing use cases but this seems to work:

:focus {
   outline: revert;
}

Putting that below the-new-css-reset seems to do the trick.