-
Notifications
You must be signed in to change notification settings - Fork 123
When to match :focus-ring:? #33
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
Comments
@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 ( 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:
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. |
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? |
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. |
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. |
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. |
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. |
Having implemented keyboard shortcuts in Slack, Twitter and Yahoo! Mail — those always require JavaScript to explicitly manage calling 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 |
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. |
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... |
@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 Reasoning:
|
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" ***@***.***> 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
<#33 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA1IZUNUh-lBtS-T_UoWfXSR2uFMKwnaks5sAdltgaJpZM4Ntul7>
.
|
Agreed, but I think that is a separate (but related) issue to this. |
@alice thanks for your patience and thoughtful consideration on this issue. Really appreciate it! I've opened a PR for your review. |
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 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 Similarly, what if I'm focusing different interactive controls using the accesskey attribute? If I focus a button using an Finally, what if you were focusing an actual interactive element (i'll call it
|
@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 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 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!" :) |
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. |
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? |
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 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 |
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. |
@bkardell Any proposal for what the "clue" part might look like? |
@bkardell my thoughts with respect to browser behavior relative to this project and what you are proposing:
Consider, for example, focus: Focus is a state. And as such, the browser fires an event when an element is focused and provides With that in mind, one idea I floated to @alice via a PR was firing an event in advance of applying the
@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 |
@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. |
@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. |
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. |
@alice I mean, an attr on the document element would work/be pretty speedy/easy to look up |
I do think there is value in exposing how focus was achieved. The ally.js library has a function that uses 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 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. |
@cycomachead It does help, thank you! |
@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 |
@cycomachead We're tossing around the idea of adding some extra information to the |
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. |
I've been thinking about this a lot and I'm now very in favor of only matching @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". |
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. |
Great to have more data in support, thanks @necolas! |
@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. |
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 |
oh that's really cool, I like that 😄 |
@alice what's your current thinking on how to proceed here? Like @necolas, I ended up having to fork |
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 #42 (comment) what do you think? |
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 :( |
@bkardell I just scanned through the discussion from Paris, but don't see anything relevant to moving forward with only matching on |
@robdodson the next steps you mentioned sound good to me. |
@kloots disregard, pasted this into the wrong issue, had a few open :( sorry. |
* 1. Refine heuristics such that the focus-ring class is only applied to the target if the user navigates to it via Tab or Shift + Tab 2. Remove use of data-focus-ring-added 3. Remove need for use of matches polyfill 4. Remove need for setTimeout() * Remove checking Shift for Tab keypresses. Always use curly braces for ifs. * Remove curly braces for single-line conditionals * Restore use of the data-focus-ring-added attribute * Cleanup comments. Remove use of toLowerCase() when checking tagName * Don't handle focus and blur events that bubble to the document.
Closing this issue because I think next steps were outlined here: #33 (comment) Happy to reopen if folks want to keep the discussion going. |
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.
The text was updated successfully, but these errors were encountered: