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

Should :focus-visible match when returning focus or programmatically focusing? #88

Open
OliverJAsh opened this issue Dec 7, 2017 · 40 comments
Labels

Comments

@OliverJAsh
Copy link

When a user closes a modal, focus is programmatically returned to the previously focused element. For example, this is the case when using react-modal.

In this case, should :focus-ring apply to the focussed element or not?

I guess the same question applies to anytime when an element is focussed programmatically.

My guess is that, if the element that focus is returning to had :focus-ring when it was initially focussed, it should also have :focus-ring when focus is returned.

@robdodson
Copy link
Collaborator

This one comes up a lot. @marcysutton, @alice, what do y'all think?

My hunch is if the user used a keyboard to close the modal (pressing escape or focusing the close button and pressing enter/space) then when focus is returned it should match :focus-ring. However, if the user used a mouse to close the modal then when focus is returned it would not match :focus-ring.

@marcysutton
Copy link

if the user used a keyboard to close the modal (pressing escape or focusing the close button and pressing enter/space) then when focus is returned it should match :focus-ring. However, if the user used a mouse to close the modal then when focus is returned it would not match :focus-ring.

I'd agree with that. If you use the keyboard to operate UI controls, you'll need a focus state at all times. The user's input modality is the important part here, not whether it was programmatically focused–that's an implementation detail.

@OliverJAsh
Copy link
Author

Interesting, do you know if this is how the browser version of :focus-ring will behave? Or is it still under consideration?

@alice
Copy link
Member

alice commented Dec 19, 2017

I would agree with Rob and Marcy's comments.

Re browser implementations, the spec deliberately doesn't specify when the pseudo-selector should match, so it'll be up to the browsers to decide.

@OliverJAsh
Copy link
Author

Do you think this polyfill should move forward with the proposed behaviour, or does it need to wait?

@robdodson
Copy link
Collaborator

I think the way the polyfill works today (latest version, at least) is if the user closes the modal via a keyboard action, the next element to receive programmatic focus would match :focus-ring.

@OliverJAsh
Copy link
Author

I think the way the polyfill works today

@robdodson I just tried this and I don't think it does work that way currently.

Here is a demo: https://stackblitz.com/edit/wicg-focus-ring-test-programmatically-focusing

Clicking moveFocus button programmatically focuses the foo button.

When actioning moveFocus with the keyboard, I would expect foo to receive the focus-ring, but it does not.

@Justineo
Copy link
Contributor

Justineo commented Feb 4, 2018

It seems that programmatically focused elements are not receiving .focus-visible by any means currently. In fact I think such elements should always match :focus-visible because even if they are focused from a mouse event, the focused element and the one being clicked may not always the same.

And people may be using mouse and keyboard at the same time, like open a dialog by clicking a button, and press enter to confirm an alert popup. The later one has a big chance to have an “OK” button which is programmatically focused. Moreover, for a confirm popup there might be both “OK” and “cancel” buttons so :focus-visible should be essential.

In general, I think all focus events not originating from a pointer event (which indicates that the user probably know where the “focus” is upon clicks) should match :focus-visible. The difference is that for devices with “linear access” need explicit focus indication while for those with “random access” to the screen, the visual hint becomes not so important. So should we just adjust the :focus-visible logic from “handling keyboard triggered focuses” to “exclude pointer events triggered focuses” instead?

@robdodson
Copy link
Collaborator

@OliverJAsh I think i found the issue.

https://github.com/WICG/focus-visible/blob/master/src/focus-visible.js#L82-L88

That has a list of accepted keys for moving focus. I think we added that for a previous radio group issue. Let me dig into this a bit more to see if we actually want to keep that array around.

@robdodson
Copy link
Collaborator

robdodson commented Feb 5, 2018

ah yes... the reason we limit it to a known set of keys is because folks were complaining that their web app's keyboard shortcuts (like cmd-b to open a sidebar or something) were triggering keyboard modality and they didn't like that.

Having spent some time digging around in Chromium recently, I now know that Chromium doesn't check which key was used to move focus, it just checks to see if a mouse was used.

I'm actually working on implementing :focus-visible behind a flag in Chromium and so far its behavior is similar to what @Justineo describes:

In general, I think all focus events not originating from a pointer event (which indicates that the user probably know where the “focus” is upon clicks) should match :focus-visible.

I image we'll want the polyfill to match this same behavior. So I'm tempted to just remove this array of allowed keys and make it so any keyboard press triggers :focus-visible and since that will leave the hadKeyboardEvent flag on in our polyfill, it means calling focus() after a keyboard press will also apply .focus-ring.

@robdodson
Copy link
Collaborator

Actually we may need to do some additional work to fully match what @Justineo was describing. If you use a mouse to click an element that calls focus on another element, we need that other element to match .focus-visible as well. Otherwise you end up with behavior like this, which I think is just weird (i'm using spacebar to press the other button because mouse clicking the first button calls focus() on it).

click

@Justineo
Copy link
Contributor

Justineo commented Feb 6, 2018

i'm using spacebar to press the other button because mouse clicking the first button calls focus() on it

@robdodson, yes that's indeed the problem I've been suffering from. Programmatically focusing an element indicates that the focus has moved away, in this case :focus-visible will be really helpful.

And recently I found that in Firefox and Safari, mouse clicks on <button>s won't move the focus to them. Instead, after the clicks, document.activeElement remains to be <body> (see here). I'm not sure if such behavior is documented in specs, but it seems a little weird to me. But if we adopt this approach, we may never actually need :focus-visible anymore (we can always add visual styles to :focus).

@robdodson
Copy link
Collaborator

robdodson commented Feb 6, 2018

And recently I found that in Firefox and Safari, mouse clicks on s won't move the focus to them. Instead, after the clicks, document.activeElement remains to be (see here). I'm not sure if such behavior is documented in specs, but it seems a little weird to me. But if we adopt this approach, we may never actually need :focus-visible anymore (we can always add visual styles to :focus).

As you mentioned, I also think this behavior is interesting but weird and kinda wrong. If I mouse click on something then I assume it's focused and that I can also press spacebar to click it again. Either way, I don't know if we'd be able to change the way :focus matching works in Chrome without getting a flood of folks complaining that we broke their site. The hope with :focus-visible is to arrive at a consistent model that all browsers implement and then just shift the whole ecosystem over to using to it as we slowly wait for :focus to fade away :P

@Justineo
Copy link
Contributor

Justineo commented Feb 7, 2018

I prefer :focus-visible over the current behavior of Firefox/Safari (which seems to be a hack to somehow mimic the visual effect of Chrome's behavior) because the later one introduced side effects on user interaction as @robdodson just mentioned.

Let's get back to the current issue on the polyfill. The current implementation only works if the target element is focused via navigation keys like tab and arrow keys. If we hit space or enter on a <button> and then focus the target element programmatically, it won't receive .focus-visible. While for Chrome, it shows a focus ring on any programmatically focused element.

ATM I have no idea about how to detect programmatical focus and always apply .focus-visible in the polyfill with the current keyboard-event-based approach. So what I'm proposing is, can we just apply .focus-visible on any focus unless the target element is focused directly from a pointer event?

@robdodson
Copy link
Collaborator

Yeah I plan to ship a fix for this today. I was working on another PR yesterday that I wanted to land first.

@robdodson
Copy link
Collaborator

Hoping this PR fixes the issue: #118

@robdodson
Copy link
Collaborator

Also want to share a link to a comment to something somewhat related: #64 (comment)

@robdodson
Copy link
Collaborator

bah! I realized the PR only solves the issue if a keyboard was used. Gotta think a bit more how to do this for a mouse...

@Justineo
Copy link
Contributor

Justineo commented Feb 8, 2018

bah! I realized the PR only solves the issue if a keyboard was used.

Yep. That's why I proposed to only exclude focuses following a pointer event (mousedown, touchstart, ...) instead of looking for those following a keyboard event.

@Justineo
Copy link
Contributor

Justineo commented Feb 8, 2018

exclude focuses following a pointer event

And we have to check if the focused element is the same as the one received the pointer event earlier. If not, it may be programmatically focused or some native mechanisms which changes focus, like focusing an <input> after clicking a <label for="...">.

@robdodson
Copy link
Collaborator

robdodson commented Feb 8, 2018

Yep. That's why I proposed to only exclude focuses following a pointer event (mousedown, touchstart, ...) instead of looking for those following a keyboard event.

Yeah I'm not sure if that would solve this either. If, for instance, we set a flag on mouseDown that signals to the system to not apply focus-visible someone could do:

foo.addEventListener('mousedown', function() {
  bar.focus();  // we don't know to put .focus-visible on bar
}, true);

@robdodson
Copy link
Collaborator

i think i have a fix for this...

robdodson added a commit that referenced this issue Feb 8, 2018
This is a fairly big change.
I removed hadKeyboardEvent and made it so the polyfill now keeps tabs on
whether or not focus came from a pointing device.
It also keeps a reference to the clicked on element and uses that to
determine if clicking on one element calls focus() on another element.
In that scenario, we want focus-visible to be applied to the new element.

Fixes #88
robdodson added a commit that referenced this issue Feb 8, 2018
This is a fairly big change.
I removed hadKeyboardEvent and made it so the polyfill now keeps tabs on
whether or not focus came from a pointing device.
It also keeps a reference to the clicked on element and uses that to
determine if clicking on one element calls focus() on another element.
In that scenario, we want focus-visible to be applied to the new element.

Fixes #88
@robdodson
Copy link
Collaborator

robdodson commented Feb 10, 2018

After talking with Alice and Marcy about this, I think we arrived at a bit of an impasse.

To recap,

Option 1: If a user mouse clicks (or touches) something that moves focus, we still consider them in "mouse modality", and don't apply focus-visible to the new element.

Option 2: We always apply focus-visible when focus() is called, so the user can clearly tell where focus is.

Per earlier discussion in this thread, it sounded like we were in favor of option 1, but @Justineo and I discussed some situations where it might be misleading, and started to look into option 2.

The upside of option 2 is that it's very clear where focus has moved to. But the con(s) are that it isn't helpful on touch/mobile displays, and many developers already disable :focus exactly because it matches in this situation. We might end up with folks doing :focus, :focus-visible { outline: none } to get around it—we definitely don't want that.

Alice and Marcy were in favor of option 1, I think I'm more in favor of option 2, though I can see why option 1 makes sense (because ultimately what we care about is the user's input modality).

The easiest next step is to just fix the immediate problem at hand (the source of this bug) which is that only some keys trigger keyboard modality. I believe #118 will fix this issue. I've also created a branch with the option 2 behavior for folks who would prefer to use that in their app.

I've also reached out to a Chrome engineer to figure out if it will be possible for :focus-visible to ship with the option 1 behavior, or if it will need to be more like option 2 (option 2 is essentially the way :focus works today). If it sounds like doing option 1 is going to be too difficult, we may have to switch to the option 2 behavior at some point down the line.

@alice
Copy link
Member

alice commented Feb 10, 2018

Agreed: the spec is deliberately vague so that we can ship with "good enough" behaviour, and tweak over time.

In my opinion we should err on the side of not showing a focus ring if in doubt, as in the short term users can force it to show via hitting a key on the keyboard, and hopefully in the medium term we will provide hooks for developers to force it to show.

@Justineo
Copy link
Contributor

But the con(s) are that it isn't helpful on touch/mobile displays, and many developers already disable :focus exactly because it matches in this situation.

As the spec allows UAs to decide whether the focus should be “visible”, I think mobile browsers can just choose to not match :focus-visible under such situation. Our earlier discussion may be just based on desktop browsers (at least for me). On the polyfill side I'm not sure if we can apply UA detection (or based on some heuristics like screen sizes) to achieve this.

@Justineo
Copy link
Contributor

In my opinion, if keyboard navigation is unavailable, visual hint for current focus doesn't make much sense because next navigation can only be from another pointer event. Otherwise, we cannot predict next navigation is from pointer or keyboard. In this case, :focus-visible does make sense and should be matched when the focused element is not the previously interacted element from a pointer event (either being keyboard navigation or programmatically changed).

@alice
Copy link
Member

alice commented Feb 12, 2018

Otherwise, we cannot predict next navigation is from pointer or keyboard.

Agreed.

In this case, :focus-visible does make sense and should be matched when the focused element is not the previously interacted element from a pointer event (either being keyboard navigation or programmatically changed).

I disagree. I think we should only match :focus-visible in situations where we have some level of confidence that a keyboard is about to be used - i.e. if the user just used a keyboard, or is using a control which requires the use of a keyboard.

@Justineo
Copy link
Contributor

Justineo commented Feb 12, 2018

User's input modality can be mixed. eg. with one's right hand on the mouse and left hand on the keyboard. If not visual hint indicating that keyboard navigation is available, one may never be aware of this. For those keyboard navigation is just optional, not matching :focus-visible may stop them from using keyboard in the first place (while keyboard + mouse may greatly improve efficiency, especially for operating those backend management systems).

Actually I think the current behavior on Chrome works exactly as I expected: when we hit on a button to call for an confirm dialog via confirm(), the cancel button is focused with a focus ring. That's how I know I can just hit tab and space to confirm and proceed. If there's no such visual hint, I may continue to use mouse to click on the ok button instead but actually I'm able to proceed without moving my pointer.

feb-12-2018 12-25-53

@alice
Copy link
Member

alice commented Feb 12, 2018

I think dialogs are a special case here.

@Justineo
Copy link
Contributor

It might be a quite typical case for people to programmatically move focus IMO. And it's the problem the current issue want to solve after all.

@alice
Copy link
Member

alice commented Feb 12, 2018

My concern is that we are missing the cases where showing the focus ring confuses people. I know @marcysutton has fielded numerous bug reports about a "weird blue ring" appearing where people didn't understand what it was for.

@Justineo
Copy link
Contributor

Justineo commented Feb 13, 2018

Emm...I think we might need a way for developers to explicitly specify the behavior. Different users may feel differently on the same behavior, one may feel fluent on the workflow and another may feel confused. In the end it depends on what kind of product we are developing. @alice I think your earlier proposal in #42 may be able to solve the problem (with further discussion on proper property name and values according to common use cases).

@alice
Copy link
Member

alice commented Feb 13, 2018

Completely agreed, I think we should err on the side of not showing a focus ring, and allow developers to opt in.

Obviously it would be great to make it so developers never need to override the default behaviour at all, but I think we should have a goal of trying to avoid false positives and accepting false negatives which we can fine-tune more over time.

@Justineo
Copy link
Contributor

The problem here is I don't know any CSS property which can modify a pseudo selector's behavior at the moment and this may not fit in the current cascading process (correct me if I'm wrong). Not sure if it's possible/willing for browsers to ship this kind of feature. One concern is this may be problematic if we give something like:

:focus-visible {
  focus-visible: never;
}

or

:not(:focus-visible) {
  focus-visible: always;
}

@Justineo
Copy link
Contributor

@robdodson @alice

I noticed Chrome Canary recently shipped :focus-visible behind a flag. Now it looks like both Chrome's :focus-visible and Firefox's :-moz-focusring matches elements focused programatically, no matter what device triggers it. See https://jsfiddle.net/gwjsd8qc/.

Regarding the polyfill, I still think it's better to match programatical focus BY DEFAULT because it wouldn't make sense to move focus to a button on a touching device. And this is also consistent with browsers' current native implementation.

@robdodson
Copy link
Collaborator

I noticed Chrome Canary recently shipped :focus-visible behind a flag. Now it looks like both Chrome's :focus-visible and Firefox's :-moz-focusring matches elements focused programatically, no matter what device triggers it. See jsfiddle.net/gwjsd8qc.

I know, I'm the one who implemented it :)
https://chromium-review.googlesource.com/c/chromium/src/+/897002

We went with this initial behavior because it was very easy to implement using the existing focus system. I'm still exploring if it would be possible to know that focus was proceeded by a keypress and how hard it would be to implement. We may experiment with both behaviors and see which one folks prefer.

@robdodson robdodson changed the title Show focus ring when returning focus or programmatically focusing? Should :focus-visible match when returning focus or programmatically focusing? Mar 29, 2018
@robdodson
Copy link
Collaborator

robdodson commented Jun 19, 2018

To circle back on this...

@alice is working on updating our implementation so it takes into account what mode the user was in when programmatic focus was triggered. If the user was in "keyboard mode", then :focus-visible would match. Otherwise, if the user was in mouse or touch mode, it would not.

We were strongly motivated to go in this direction after discovering a use case that current :focus-visible did not solve. Specifically, on mobile, if you touch a button which opens a context menu that then programmatically focuses the first menu item.

A three dot context menu button with expanded menu items

On Android this will show an orange focus indicator—because it's programmatic focus— but that indicator is rather unhelpful if I just tapped the menu button with my finger. However, if I had a keyboard attached to my Android device, that indicator would be quite useful.

RE: the separate question of opting in... We're exploring if there is overlap with the inputmode attribute. Generally speaking, if your control would display a virtual keyboard when focused, then it's "text-y" and should probably always match :focus-visible. If your control would not display a virtual keyboard when focused, then it's probably "button-y" and would check the user's input modality before matching :focus-visible.

@Justineo
Copy link
Contributor

@robdodson Thanks for the update. In our real cases I found it’s hard to use simple heuristics to solve all scenarios. Automatically match :focus-visible according to users’ input modalities is good in most cases (hide focus indicators for pointer events and show them on keyboard navigation), while in certain cases input modalities can be mixed. A typical case is a modal dialog which I mentioned in my earlier comment (#88 (comment)), that mixed input modality can be more productive in certain cases and Chrome is already behaving like this for native alerts or confirms. I also agree with @alice that sometimes enforcing programatical :focus-visible can be confusing, especially for mobile devices that we won’t expect mixed input modalities and for popup option lists that we don’t want any of them to be “emphasized” by a focus indicator.

So I think we still need a way to manually specify the behavior other than the heuristics of browsers.

@Alohci
Copy link

Alohci commented Mar 20, 2019

What happens if the programmatical focus was driven by a non-input event, say a timeout, or a message to the window from another window? I think what I'd want is for the current state to be preserved. i.e. if the previously focussed element matched :focus-visible, then so should the newly focussed one.

@robdodson
Copy link
Collaborator

@Alohci I think in that case it would use the behavior mentioned in this comment: #88 (comment)

It'll use whatever state it was previously in.

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

No branches or pull requests

6 participants