-
Notifications
You must be signed in to change notification settings - Fork 124
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
Comments
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. |
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. |
Interesting, do you know if this is how the browser version of |
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. |
Do you think this polyfill should move forward with the proposed behaviour, or does it need to wait? |
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. |
@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 When actioning |
It seems that programmatically focused elements are not receiving 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 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 |
@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. |
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
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 |
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 |
@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 And recently I found that in Firefox and Safari, mouse clicks on |
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 |
I prefer 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 ATM I have no idea about how to detect programmatical focus and always apply |
Yeah I plan to ship a fix for this today. I was working on another PR yesterday that I wanted to land first. |
Hoping this PR fixes the issue: #118 |
Also want to share a link to a comment to something somewhat related: #64 (comment) |
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... |
Yep. That's why I proposed to only 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 |
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:
|
i think i have a fix for this... |
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
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
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 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 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 |
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. |
As the spec allows UAs to decide whether the focus should be “visible”, I think mobile browsers can just choose to not match |
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, |
Agreed.
I disagree. I think we should only match |
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;
} |
I noticed Chrome Canary recently shipped 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. |
I know, I'm the one who implemented it :) 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. |
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 We were strongly motivated to go in this direction after discovering a use case that current 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 |
@robdodson Thanks for the update. In our real cases I found it’s hard to use simple heuristics to solve all scenarios. Automatically match So I think we still need a way to manually specify the behavior other than the heuristics of browsers. |
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. |
@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. |
I am facing an issue where, if I open a dialog element by clicking on a non-interactive element, the focus correctly moves to the button inside the dialog. However, when I trigger the dialog using an interactive element (a button), the focus does not shift as expected. Is this a browser bug, or is it intended behavior according to the spec? Below is a sample example demonstrating the issue Related issue:#268 |
@ChellappanRajan I'm not able to reproduce the issue. If I click on either button, they open the dialog, and then I can press Space and it will click the button inside of the dialog to close it. |
Yes, you are correct. However, I noticed that the focus ring does not appear on the button when the interactive element is clicked to open the modal. For example, in the screenshot below, the focus ring is visible on the button when a non-interactive element is used to open the modal But when I open the modal using an interactive element, the focus ring is missing on the button: I have tested this on chrome V 129 |
@ChellappanRajan how are you clicking on the non-interactive element? With a mouse or somehow with a keyboard?
Here is a video to demonstrate. At the beginning, I use my mouse to click on both buttons and I do not see a focus-ring in either case. Then, I switch to using the keyboard to click the first button and I do see a focus-ring when the dialog opens. I can't click on the second button with my keyboard so the video ends. Kapture.2024-10-24.at.15.18.36.mp4 |
@robdodson I’m encountering an issue with the dialog element on a fresh refresh. When I first open the dialog by clicking an interactive element with the mouse, then close it and reopen it with a non-interactive click, the focus ring on the button inside the dialog doesn’t appear. However, if I open the dialog for the first time using a non-interactive click, the focus ring displays. Another observation: if I open the modal by tab-focusing the 'Open Modal' button and then click the button while the focus ring is visible, the focus ring also appears on the button inside the modal. I've included a video showing the mouse interaction: MouseInteraction.mov |
Hm that behavior seems a bit odd. I'm seeing a warning in the developer console when the page first loads:
Can you try running this example locally. I wonder if the iframe from lit.dev is causing issues. |
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.
The text was updated successfully, but these errors were encountered: