Any focus proceeded by a keypress should trigger focus-visible. #118
Conversation
| <script src="/dist/focus-visible.js"></script> | ||
| <script> | ||
| start.onclick = function() { | ||
| el.focus(); |
There was a problem hiding this comment.
Can you also add a test for focusing in a new microtask?
cb30b92 to
da0cc0e
Compare
|
Ignore the conflicts. I think I just forgot to rebase against latest master where the most recent change was some additional comments. |
src/focus-visible.js
Outdated
| elementToBePointerFocused = e.target; | ||
| // Clicking inside of a <select multiple> element will fire a mousedown | ||
| // on the <option> child, but focus will be set on the <select> itself. | ||
| if (elementToBePointerFocused.nodeName == 'OPTION') { |
There was a problem hiding this comment.
Note. If we keep this change then we'll need to do something different for elements inside of shadow dom.
There was a problem hiding this comment.
I think what we can do there is .getRootNode() or whatever the API is called. If that returns a shadowRoot, we'll need to set this to the element's host.
There was a problem hiding this comment.
da0cc0e to
2db555a
Compare
|
I went back to the previous approach. I moved the pointer exclusion stuff to a separate branch in case we ever want to use it. I also added a fix and test for a small bug I found. If you press a key on an already focused element, the polyfill thinks it's in keyboard modality but it never gets cleared out because there's never a focus event. If you then click on something with a mouse, we still think we're in keyboard modality. I just added a handler for any mouse/pointer/touch down event that forces keyboard modality off. Ready for review when you have time. |
|
First of all, thanks for this library. Really liking the polyfill efforts from standards orgs. What issue does this pull request solve? It seems to be the cause of unusual behavior when using cmd-tab to switch away from browser windows:
A simple reproduction can be seen on the demo page, where a press on the cmd key after mouse-only selection causes the focus style to be applied: The flashing of styles while using cmd-tab does not appear in version If there's a very important reason that the changes from this pull request were made, maybe there can be a smaller blacklist of modifier keys (window and tab switching keys, for example) that it won't trigger on? |
|
I can also open this as a new issue, if that would be more appropriate. |
|
Hey @karlhorky, I plan to follow up more but real quick, i think you'd be interested in this: We updated the heuristic so meta keys don't trigger keyboard focus. Specifically to avoid when folks are using keyboard shortcuts. |
|
Ah cool, you're on it already! Nice 🙌. Watching now. |
@alice PTAL