WICG / focus-visible

Polyfill for `:focus-visible`
https://wicg.github.io/focus-visible/demo/
Other
1.6k stars 124 forks source link

When to match :focus-ring:? #33

Closed alice closed 7 years ago

alice commented 7 years ago

Currently we model the behaviour of <button>, which (in Chrome) shows a focus ring when it receives focus after a keyboard event, or when a key is pressed while it is focused.

Another option would be to show a focus ring only when focus is received from a keyboard event. This would be a break from existing behaviour, but may be input into browsers' eventual decision of what behaviour to model.

kloots commented 7 years ago

@alice as I am currently working on putting this polyfill to work in the Slack codebase my feedback is: not every focus event following a keydown event should result in a focus ring being drawn. As such, the behavior as currently implemented results in the focus ring showing up unexpectedly/or at times when it is not desired.

An example from Slack: a user presses a keyboard shortcut (Cmd + .) to show or hide the right-hand sidebar. We move focus to the sidebar when it is shown, so users have the option to navigate it with the keyboard. However, that focus ring is only useful should the user actually begin navigating via the keyboard. And not everyone opens the sidebar with the intention or need to navigate its content via the keyboard. As such, I end up having to add more CSS to suppress the focus ring style for this instance/state.

The library would be much more useful, reliable and easier to work with if I could simply rely on the focus style provided by the focus-ring polyfill to only appear when a control receives focus as a result of the user navigating to it via the default methods provided by the browser: Tab & Shift + Tab. That way I could simply have:

// Suppress focus by default
:focus {
  outline: none;
}

// Show focus for text inputs
input[type=text],
textarea {
  outline: 3px solid blue;
}

// Show focus when navigating via the keyboard
.focus-ring {
  outline: 3px solid blue;
}

This is much more with the grain of default browser behavior and solves the real problem I have: users and designers only want to see focus when either navigating via Tab OR if you are typing in a text box.

alice commented 7 years ago

As a counter-argument, if both keyboard shortcuts and pointer-based options are available to access a piece of UI, I think using the keyboard shortcut would be a good signal that the user is intending to continue using the keyboard to interact with that UI.

In fact, predominantly keyboard users are almost certainly more likely to use keyboard shortcuts which result in moving focus, aren't they?

kloots commented 7 years ago

I can tell you with certainty from using this library with Slack that assumption is not correct. :) In fact, I wouldn't have raised this issue if it was correct. :) At this point I'm dealing with way more "why am I seeing this focus ring" bug reports than "I was expecting to see a focus ring and didn't" reports.

alice commented 7 years ago

I don't think it follows that because you are getting more bug reports about unexpected focus rings, predominantly keyboard users are not more likely to be using keyboard shortcuts.

If we focus (heh) only on the predominantly keyboard users for a moment, I would imagine that they would be heavier users of keyboard shortcuts which move focus than users who rely less heavily on the keyboard. And if we show focus only when focus is moved via tab/shift-tab, this would mean that focus is lost even for users who only use the keyboard when they move focus via a keyboard shortcut.

However, predominantly pointer-based users are very likely to dominate any given user population in terms of numbers, so it would make sense to see more reports of issues which affect them.

kloots commented 7 years ago

Sorry, I clearly failed to explain myself. Very sorry for that. :) Yes, predominantly keyboard users are more likely to be using keyboard shortcuts. My point was not every user who uses a keyboard shortcut is a predominantly keyboard user. Many use keyboard shortcuts for some actions, but are otherwise predominantly pointer-based folks and hence don't rely on the keyboard for navigation between focusable elements within the app. It is for these users that the current behavior is producing more unwanted focus rings.

From my perspective I think we should keep in mind pointer-based users do dominate any given user population so I think the most elegant solution is one that ensures the focus-ring is there when you need it, but otherwise hidden.

If you're unsure, disagree or feel like the discoveries we've made while implementing this polyfill within Slack don't align to the goals of this project I totally understand. If that's the case, I can simply fork this project and make the changes to support the heuristics we need.

alice commented 7 years ago

Agreed, the premise of this selector is that the focus ring should be there when you need it, and not otherwise.

The bit we're disagreeing on is how much of a signal using a keyboard shortcut to move focus is, compared to using tab/shift tab, as to whether the user needs it.

Obviously the last thing we want is to end up back in a situation where designers are insisting on focus rings being hidden because they are appearing unexpectedly when predominantly pointer-based users use keyboard shortcuts to speed up their workflow (which is obviously even more likely in a heavily typing-based app like Slack).

However, we need to balance that against the weirdness of the experience for a predominantly keyboard users who would have their focus disappear whenever they used a keyboard shortcut (which they are likely to do pretty frequently).

So, at this point, I'm still not 100% clear on what the best solution is.

(edit) Oh also, it would be nice if, as you suggested in your original pull, there were some way for apps to somewhat customize this behaviour - but I have even less idea how we might achieve that.

kloots commented 7 years ago

Having implemented keyboard shortcuts in Slack, Twitter and Yahoo! Mail — those always require JavaScript to explicitly manage calling focus() in response to a keypress as well as the strategic placement of a class used to style focus. In adopting this polyfill for Slack I'm not evening relying on the "focus-ring" class to support keyboard shortcuts, as often the class I need to style the focus state needs to be applied on some other parent element of the focused element in order to achieve the designed focus state we require. As such, as it pertains to keyboard shortcuts, this polyfill is not adding any value for us, but rather adding headaches. Styling focus for keyboard shortcuts isn't a use case we have. Styling focus for basic browser keyboard navigation is. The same was true at Twitter. So, that's why I think the current heuristic for when the focus-ring class is applied going too far.

My thoughts are this polyfill should be more conservative as to how it styles focus and ensure it does so in a way that is with the grain of what the browser provides for moving focus out of the box: via Tab and Shift + Tab.

kloots commented 7 years ago

Have any teams at Google implemented this polyfill in their web apps? Or any other large web apps used this? If my opinion is in the minority, I'm happy to simply fork and move on.

bkardell commented 7 years ago

The chat input itself has a ring, right? Then you press a key command and it disappears because the sidebar opens. What you're saying I think is that the act of suddenly displaying that is enough of a visual cue to let users understand the state of focus and what will happen if they press tab, shift tab, etc? Just trying to make sure I understand because the current behavior seems reasonable to me...

alice commented 7 years ago

@bkardell That's a good point. I don't think this is a general rule for focus following a keypress, though; I could imagine a scenario where, say, a keyboard shortcut exists to move focus away from an active text field which would otherwise be a tab trap, in which case there wouldn't be any new UI.

@kloots Thanks for the further examples - very useful to know that you typically style focus manually in cases where focus is moved via shortcut instead of by tab/shift-tab.

I'm coming around to the idea that we should probably do as @kloots suggests and have the polyfill match :focus-ring only on focus from tab/shift-tab, and offer it as a suggestion to user agents.

Reasoning:

bkardell commented 7 years ago

I know I said this before, but I still think we may need to figure out and allow some attributes or at least properties for folks making things beyond what are generally already interactive elements to say where they 'fit' in this algorithm.

On Jun 3, 2017 6:08 PM, "Alice" notifications@github.com wrote:

@bkardell https://github.com/bkardell That's a good point. I don't think this is a general rule for focus following a keypress, though; I could imagine a scenario where, say, a keyboard shortcut exists to move focus away from an active text field which would otherwise be a tab trap, in which case there wouldn't be any new UI.

@kloots https://github.com/kloots Thanks for the further examples - very useful to know that you typically style focus manually in cases where focus is moved via shortcut instead of by tab/shift-tab.

I'm coming around to the idea that we should probably do as @kloots https://github.com/kloots suggests and have the polyfill match :focus-ring only on focus from tab/shift-tab, and offer it as a suggestion to user agents.

Reasoning:

  • A generic keypress is not a strong enough signal that a user intends to keep using the keyboard, and this selector intends to avoid showing focus rings when it would be confusing to the user.
  • If a keyboard shortcut is likely to move focus, it is probably either showing some new UI, or it is specifically designed to assist keyboard users, in which case a manual focus style is likely to be applied.
  • User agents may add a setting which would allow keyboard-only users to signal that they always wish to see a focus ring.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/WICG/focus-ring/issues/33#issuecomment-306004359, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1IZUNUh-lBtS-T_UoWfXSR2uFMKwnaks5sAdltgaJpZM4Ntul7 .

alice commented 7 years ago

@bkardell

we may need to figure out and allow some attributes or at least properties for folks making things beyond what are generally already interactive elements to say where they 'fit' in this algorithm.

Agreed, but I think that is a separate (but related) issue to this.

kloots commented 7 years ago

@alice thanks for your patience and thoughtful consideration on this issue. Really appreciate it! I've opened a PR for your review.

robdodson commented 7 years ago

Thanks for the thorough discussion everyone. Sorry that I'm coming to this late, but I had a few questions about the proposed solution.

To recap:

Right now when I hit cmd + . in Slack it opens the drawer and focuses a heading with tabindex=-1. This is problematic because a mouse user is now matching :focus-ring based on the fact that they just used the keyboard. As a result, @kloots suggest we only match :focus-ring if the keys pressed were tab or shift + tab.

I'm concerned about this change based on an opposing use case:

Let's say I have a button which, when activated with spacebar or enter, opens a modal window and puts focus on a button inside the modal. Because the user clicked on the button with their keyboard instead of a mouse, currently :focus-ring will match and the button inside of the modal will get styled. I like this behavior, but it would no longer work if we make the proposed change. In order to differentiate mouse and keyboard focus I'd need to go back to writing JavaScript to detect what just happened. That feels like a step back.

Similarly, what if I'm focusing different interactive controls using the accesskey attribute? If I focus a button using an accesskey keyboard press, I'd like :focus-ring to match. That means it'd need to support more than just tab and shift+tab.

Finally, what if you were focusing an actual interactive element (i'll call it .special-button) with the cmd-. keyboard shortcut? In that case you'd probably want :focus-ring to match, right? Otherwise you might end up with CSS like this:

:focus {
  outline: none;
}

:focus-ring {
  outline: 2px solid green;
}

.special-button:focus {
  outline: 2px solid green; // ack! this is going to match if the user clicks on this control with the mouse now
}
kloots commented 7 years ago

@robdodson regarding the modal window use case you mentioned: even that is questionable in my experience. Just because an action like opening a dialog is initiated via one mode (keyboard), you're never guaranteed the user will complete it via the same mode—especially in a context switch like popping up a dialog.

Here's a good example from a bug we experienced while integrating this library at Slack: closing a dialog using the Esc key. As I'm sure you know, this is an incredibly common user action, and not one exclusive to keyboard-only users. As a result of the current implementation, lots of folks were complaining of seeing an unwanted focus ring after pressing Esc to close a dialog or popup menu. And I think they're right to complain, and I say that as someone who cares deeply about accessibility. :)

It is for these, and my various aforementioned use cases, I have found the current matching is creating more user problems than it is solving. I think a much more conservative design pattern makes sense: treat the focus outline like a mouse cursor: show it when you need it to move from A to B or if you are editing something in a text box.

We've found this pattern is the right balance for us at Slack. Consider the dialog example I mentioned earlier. After closing a dialog by pressing Esc, should a keyboard user want to resume navigating all they need to do is press Tab / Shift + Tab and the focus ring is back and they can carry on navigating. The product is no less keyboard accessible and mouse users are also happy.

There's lots and lots of gray areas—and rightly so! Balancing these two modes is complicated, and hence, I think the conservative approach makes more sense.

This of course is my opinion based on my experience making consumer web apps accessible. I can understand your hesitation in trusting the opinion of someone you don't know and have never worked with. Before you and @alice make a decision though, I would encourage you to see if you can get this integrated into one of Google's consumer web apps (or some other consumer web app). (Unless you've done so already, in which case NM.) You may discover some of the same challenges I have, different challenges, or find out: "Hey, that Kloots character was dead wrong—screw that guy!" :)

bkardell commented 7 years ago

FWIW, my org uses it for online colleges and internal apps for managing them. We've never heard any complaints, but that doesn't mean I don't appreciate your arguments.

alice commented 7 years ago

@kloots

Balancing these two modes is complicated

Agreed, this is the crux of it.

I would like to accept the PR and see how people feel about it, on the understanding that we can always roll it back later on if it breaks things.

@marcysutton @robdodson @bkardell Any objections?

alice commented 7 years ago

Also, I think something like this might work as an author mechanism for opting in to the focus ring:

:focus:not(:focus-ring) {
  outline-width: 0;
}

.focus-ring:focus {
  outline-width: initial;
}

Then authors could use the focus-ring class to force the focus ring when appropriate.

This would be just a convention, but if we agree on this convention it would probably make sense to reinstate the ability to manually apply the focus-ring class in the polyfill.

bkardell commented 7 years ago

so, I guess here are the things I am pondering... Browsers are very hesitant to spec when you should get the ring and when you shouldn't, they prefer to wait until there are competing experiments and massive amounts of data and agreement. Not all browsers even agree on all cases and so anything you do 'universally' is going to mismatch with some of what they do 'naturally'.

Given all this, I'm kind of wondering... Maybe it's crazy, but -- is it maybe worth putting something in the DOM as a clue as to how you'd like this to act - even if that is not on a per-case basis (like a doc level attribute maybe) and supporting both. Then we can look for cow paths based on lots of available input that would be fairly easy to find (since you dont need to analyze the whole app) and collect kind of simple feedback from authors (including 'gee, neither way really works for me "all the time"')? idk.

alice commented 7 years ago

@bkardell Any proposal for what the "clue" part might look like?

kloots commented 7 years ago

@bkardell my thoughts with respect to browser behavior relative to this project and what you are proposing:

  1. Tab and Shift + Tab are the only mechanism browsers provide out of the box for navigating between UI controls via the keyboard. If we only apply the focus-ring class in response to a Tab or Shift + Tab keypress, we're inline with current browser behavior.

  2. Should we instead want to match more broadly by default while also providing developers a means of customization I think an event-driven design is the right approach, as it is also inline existing mechanisms provided by the browser.

Consider, for example, focus: Focus is a state. And as such, the browser fires an event when an element is focused and provides :focus for styling state. Developers can leverage preventDefault() in listeners for the preceding events (mousedown and keydown) to prevent an element from gaining focus.

With that in mind, one idea I floated to @alice via a PR was firing an event in advance of applying the focus-ring class. The event payload contained the keyCode of the key event responsible for the target gaining focus and you could leverage that to selectively prevent that behavior via an event listener. For example:

function onFocusRing(e) {
    // Don't apply the focus-ring class unless the element received 
    // focus via a Tab key press
    if (e.keyCode != 9) {
        e.preventDefault();
    }
}

document.addEventListener('focus-ring', onFocusRing, false);

@alice didn't buy it (as I remember) on the premise that other pseudos don't fire events. But I blame myself for that—perhaps I didn't explain my idea well enough at the time. (Or maybe it is a crap idea.) But when considered in the context of other states like focus, I think this makes sense. We just consider keyboard focus a state, and that state has a corresponding event and pseudo class. If we go that direction, maybe the pseudo class would be a better name :keyboard-focus rather than :focus-ring as the latter name doesn't really convey the state/behavior we're mapping to.

bkardell commented 7 years ago

@kloots I do want to mention that our original piece on this was modality oriented, so you could talk specifically about keyboard focus, for example, but that didn't really fly for various reasons. I think most of them should be documented in css wg minutes or here in issues.

kloots commented 7 years ago

@bkardell good to know! Thanks for that additional context. I'm interested in learning the history then and the decisions that led to the current state.

marcysutton commented 7 years ago

After closing a dialog by pressing Esc, should a keyboard user want to resume navigating all they need to do is press Tab / Shift + Tab and the focus ring is back and they can carry on navigating. The product is no less keyboard accessible and mouse users are also happy.

I'm concerned about the cognitive overhead required to find your place on the screen if you have to Tab / Shift + Tab to get focus outlines to show up after a modal window opens (if I understood that correctly). It's hard to tell how many users will be affected by that, but it would be a guessing game as to whether your focus was actually sent into the modal until you hit Tab again. Not exactly following the "don't make me think" philosophy of UX.

That said, I think merging the PR and seeing how it works in real-world scenarios is an okay way forward. But I also like the idea tossed around previously for a UA hook that a user could apply to override focus behavior if necessary, especially since it's difficult to please everyone.

bkardell commented 7 years ago

@alice I mean, an attr on the document element would work/be pretty speedy/easy to look up

cycomachead commented 7 years ago

I do think there is value in exposing how focus was achieved. The ally.js library has a function that uses data-focus-source as an attribute that can you can query for in CSS. (The possible values are "script", "keyboard" and "pointer")

https://allyjs.io/api/style/focus-source.html

That's fairly easy to override and control in CSS, but a callback is pretty limitless, and I could potentially see having a use case for that.

I'm concerned about the cognitive overhead required to find your place on the screen if you have to Tab / Shift + Tab to get focus outlines to show up after a modal window opens. But it's hard to tell how many users will be affected by that.

I think, for me, one compromising idea would be if you open the modal with a keyboard then closing it with a keyboard should maintain a focus ring. (However, this starts to seem overly complex...) Needing an "extra" tab isn't a significant impediment (if it's consistent across a site, it can be learned), but it certainly can be confusing. It's never clear if you can from seeing no focus ring → tab → seeing a focus ring, if you're in the same place you were before, or if you'd be in the next logical location.

I personally don't mind seeing focus rings anyway, but I do know lots of people who expect something like the escape key to exit a modal even if they aren't primarily a keyboard user and are in some cases are a little confused by seeing an outline around a button that wasn't there before.

Sorry, I'm not sure if that helps this discussion...but I've been struggling with the same constraints lately.

alice commented 7 years ago

@cycomachead It does help, thank you!

alice commented 7 years ago

@marcysutton In terms of the cognitive overload, we've thrown around a few times the idea of having a browser setting to always show the focus ring (TBD how that plays into :focus-ring matching heuristics) - while this would require folks to find, understand and enable the setting, which is non-trivial, do you think it might still help for this case?

alice commented 7 years ago

@cycomachead We're tossing around the idea of adding some extra information to the focus event to determine the focus source. Do you think you'd want an explicit enum like in a11y.js, or would an Event object be more useful?

cycomachead commented 7 years ago

Do you think you'd want an explicit enum like in a11y.js, or would an Event object be more useful?

Hmm. This is tough. I haven't thought through every case - but I feel like an enum probably gets me 90% of the conceivable stuff, but an event object is essentially unlimited. The enum would be easy to work with, and probably more "forgiving" in the sense of possibly preventing complex cases in which focus rings are(n't) shown.

Lately, (and unsurprisingly) I've found the more complex/perfect a setup is, the more likely it becomes that I'm overlooking some case where focus-rings do still make sense - so being restricted from too much power isn't always bad.

I guess, at the end of it all, it depends on how many options an enum provides. ally.js doesn't expose differences between touch and click sources, which could be useful depending upon the app.

robdodson commented 7 years ago

I've been thinking about this a lot and I'm now very in favor of only matching :focus-ring when the user presses Tab or Shift + Tab.

@kloot's use case of opening the side drawer made me realize we're also missing some accessibility primitives. Currently we all do this hacky thing of setting an element to tabindex=-1, moving focus to it and likely suppressing the focus ring using CSS. As @alice pointed out to me in conversation, really what developers want is the ability to move the focus navigation start point.

I also like the idea of a hook for an element to say "hey please always match focus-ring on me".

necolas commented 7 years ago

I thought I'd add a couple more data points to support Todd's argument and proposal. We use a fork of this module in Twitter Lite and React Native for Web, and would occasionally and unexpectedly see the focus ring for the reasons Todd outlined. His proposed fix is also along the lines of the modifications I'm currently making to our fork so that Twitter Lite can better support multi- and mixed-modality usage.

alice commented 7 years ago

Great to have more data in support, thanks @necolas!

robdodson commented 7 years ago

@necolas if we went with this change would you be able to use this polyfill directly or are there other things you needed to tweak? Just curious if there are any other missing pieces.

necolas commented 7 years ago

I went with a different approach, which is to automatically handle the enabling/disabling of focus-ring styles without writing class names to the DOM: https://github.com/necolas/react-native-web/blob/02cfbf8/src/modules/modality/index.js#L85-L101

robdodson commented 7 years ago

oh that's really cool, I like that 😄

kloots commented 7 years ago

@alice what's your current thinking on how to proceed here? Like @necolas, I ended up having to fork focus-ring months back in order to successfully integrate it into a production web app (Slack). And I believe @robdodson is also in favor of only matching when the user is navigating via the Tab key. Where do we stand here?

robdodson commented 7 years ago

thanks for your patience @kloots :)

I just chatted with alice about this. We had a couple ideas:

First step is we'll switch over to just matching tab/shift-tab. I think we definitely want to roll in the feedback from yourself and @necolas. That's WICG in action 💪

Then as a future v2 we'd explore making it more configurable per alice's suggestion here https://github.com/WICG/focus-ring/issues/42#issuecomment-309309323

what do you think?

bkardell commented 7 years ago

FWIW I see CSSWG had some brief discussion in Paris https://logs.csswg.org/irc.w3.org/css/2017-08-02/#e842809 - unfortunately I was not present :(

kloots commented 7 years ago

@bkardell I just scanned through the discussion from Paris, but don't see anything relevant to moving forward with only matching on Tab or Shift + Tab. Unless I missed something. If I did, can you post a pull quote and link it back?

kloots commented 7 years ago

@robdodson the next steps you mentioned sound good to me.

bkardell commented 7 years ago

@kloots disregard, pasted this into the wrong issue, had a few open :( sorry.

robdodson commented 7 years ago

Closing this issue because I think next steps were outlined here: https://github.com/WICG/focus-ring/issues/33#issuecomment-320155940

Happy to reopen if folks want to keep the discussion going.