fix: tag-list-item: use focus-visible #4226
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
|
| @@ -81,12 +81,12 @@ export const TagListItemMixin = superclass => class extends LocalizeCoreElement( | |||
| padding: 0.25rem 0.6rem; | |||
| text-overflow: ellipsis; | |||
| } | |||
| :host(:focus-within) .tag-list-item-container, | |||
| :host(:focus-within:hover) .tag-list-item-container { | |||
| .tag-list-item-container:has(:focus-visible), | |||
There was a problem hiding this comment.
The :has(:focus-visible) is really awesome for dealing with this scenario. If we're going to use it though, we should couple it with @supports for backwards compatibility.
foo1:has(:focus-visible) {
...
}
@supports not selector(:has(a, b)) {
foo1:focus-within {
...
}
}The above is concise, but for the best backwards compatibility we probably would reverse it because I think the selector option is a bit newer. Unfortunately that makes the CSS a little less tidy though...
foo1:focus-within {
... // "fallback" styles
}
@supports selector(:has(a, b)) {
foo1:focus-within {
... // undo the above since we don't really want those focus styles if `:has` is supported
}
foo1:has(:focus-visible) {
...
}
}There was a problem hiding this comment.
Thanks! That's really helpful.
One thing I'm seeing with :focus-visible is that on Safari, if I click one of the tag list items, giving it focus, then press the right or left arrow key, those items do not get the focus ring (they're not triggering the focus-visible styles). On Chrome they do, and they do if I tab into them on Safari then they do. From some searching it looks like Safari handles keyboard focus on click differently than Chrome, but I'm not sure. Have you seen this before? Is this just expected?
There was a problem hiding this comment.
Np! No, I haven't seen that before. I just did a quick check and it looks like d2l-file-input's usage of :has(:focus-visible) is working in Safari as expected. I also did a quick check with d2l-tabs since it uses arrow keys to manage focus along with :focus-visible, and it also appears to be working as expected as well. 🤔
There was a problem hiding this comment.
Oh interesting, I just tested Safari with tabs and I'm seeing the same issue as with tag-list. Maybe it's my browser version? I'm on Safari 16.6. I'm going to see if I can update.
Update: on 17.1 now and still seeing the issue in tabs (and tag-list) where if I click one of the tabs then press the arrow keys there is no focus ring.
There was a problem hiding this comment.
Oh I was doing something slightly different - I was tabbing into the list from outside (rather than clicking a tab), and then using the arrows.
I think the browser tracks whether the last interaction was keyboard for the purpose of :focus-visible, and it would seem that Safari doesn't regard arrow keys as keyboard interactions, so its state would just be whatever it was when focus entered into the list of tabs. So if tabbing in and then using arrow keys, then it works, but if we click in and then use arrow keys, it doesn't. Whereas, it would seem Chrome regards a broader set of keys as keyboard interactions.
I wonder if there is a way we can trick Safari into being in keyboard mode. For example, before we move focus in response to the arrow keys, perhaps we can send a key event of some type that will force Safari's state. I think we do something similar in the testing library. Like a shift/ctrl/cmd key or something.
|
Closing for now. This PR is referenced in the Rally defect. |
Rally defect