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

Improved focus states for components #69

Merged
merged 1 commit into from
Apr 10, 2015
Merged

Improved focus states for components #69

merged 1 commit into from
Apr 10, 2015

Conversation

jrubenoff
Copy link
Contributor

No description provided.

@ajb
Copy link
Contributor

ajb commented Apr 6, 2015

Can you explain this PR a little bit? I kinda get it, but I want to hear it from you..

@jrubenoff
Copy link
Contributor Author

The default Webkit outline for focus states is pretty ugly. If a hover state looks distinct enough from a component's default state, I've changed the focus state to match.

@ajb
Copy link
Contributor

ajb commented Apr 7, 2015

So I'm not really sure this is the best idea. Is there any instance in which the user will see focus states of these components, except when they're tabbing to them? Other concerns:

  1. I can't find another website that overrides the default focus styles on a per-component basis. (Although since I'm using GitHub.com as my reference, I will admit that the outline on the buttons below this text box looks custom...)
  2. I'm afraid that our custom-defined focus states will cause accessibility issues, because they aren't distinct enough from the non-focus states.
  3. Even if 1 & 2 are allayed, I'm worried that we'll be playing whack-a-mole with keeping track of our focus states for our components. We already have enough to do, this seems pretty extraneous if it's only something that keyboard-navigators are going to see.

@jrubenoff
Copy link
Contributor Author

this seems pretty extraneous if it's only something that keyboard-navigators are going to see

That's what designing for accessibility means!

they aren't distinct enough from the non-focus states

Hover and focus states can be identical. You're navigating with a keyboard because you cannot or don't want to use the mouse, so you'll never see a hover state.

we'll be playing whack-a-mole with keeping track of our focus states for our components

We already have custom hover and active states for most components... not sure why this is different.

@ajb
Copy link
Contributor

ajb commented Apr 7, 2015

That's what designing for accessibility means!

Right, but isn't this less accessible, since we're making the :focus states more subtle? See below...

Hover and focus states can be identical. You're navigating with a keyboard because you cannot or don't want to use the mouse, so you'll never see a hover state.

What I mean is that these new focus states have the potential to be too subtle compared to the normal state of the element.

And this all brings us back to my first bullet as well. Can you provide some kind of reference point for this being a good practice? Neither ZURB Foundation nor Bootstrap appear to style their focus states.

@jrubenoff
Copy link
Contributor Author

Here's the A11Y Project... I'm proposing solution 2.

Generally, I don't think it's a matter of good / bad practice so much as attention to detail, empathy and consistent branding. It's about paying attention to that audience... not many do.

We're fine with styling focus states on our inputs, so that fully-abled users don't have to see the focus ring. Why should other elements be any different?

screen shot 2015-04-07 at 6 19 19 pm

Defining focus states ourselves also lets us avoid weird visual bugs with the browser's default focus ring.

screen shot 2015-04-07 at 6 16 25 pm

@ajb
Copy link
Contributor

ajb commented Apr 8, 2015

Thanks for the thoughtful reply, this helps clarify a bit.

I still stand by my concern that we might be making our site less accessible by making the focus states more subtle. I want a focus state to look like what a user expects a focus state to look like.

How about this for a rule:

  • We only apply subtle style changes to focus states. This can include changing the size of the focus outline, or replacing the outline with a border of similar color, but we never remove the outline/border completely.

I think this helps us achieve "attention to detail, empathy and consistent branding", while allaying my concern above.

@jrubenoff
Copy link
Contributor Author

So, basing an argument on anecdotal experience is obnoxious, but I'm going to do it anyway. 😞

Everything I've read about accessibility advises that you need to clearly indicate focus states. It doesn't prescribe how they should be styled. This is why browser default focus states differ from each other... Internet Explorer, for instance, is much lower-contrast and harder to discern. Sometimes when you custom style, you're actually doing users a favor .

Anecdotally, I've heard many users complain that they can't tell what's currently in focus. That's always because the focus styles have been removed, or they're way too subtle. I've never heard anyone complain that a focus state isn't styled in the way they expect. They're the ones tabbing through the page, so they know they're looking for some kind of change in state.

Here's my test to ensure that a focus state isn't too subtle:

  • Visit a webpage.
  • Tab through it.
  • Can you tell which element is in focus at any given time while tabbing through the page?

That's it.

I conducted that test before opening this PR.

@ajb
Copy link
Contributor

ajb commented Apr 10, 2015

Alright, I'm OK merging this. I'm still not convinced that this is super-necessary, but it sounds like you've thought about it so I'm alright to just let it go. Sorry for making you go through all of that just to get these changes merged!

ajb added a commit that referenced this pull request Apr 10, 2015
Improved focus states for components
@ajb ajb merged commit dd9922a into master Apr 10, 2015
@ajb ajb deleted the focus_states branch April 10, 2015 01:33
@jrubenoff jrubenoff restored the focus_states branch April 13, 2015 22:49
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.

2 participants