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

Display focus ring in select options only when focus is visible #1604

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Jul 5, 2024

This PR changes a bit how we display the focus ring in SelectNext options, by making it visible only when the focus itself is considered visible by the browser (:focus-visible instead of :focus), which happens when interacting with components with the keyboard.

select-with-arrows-2024-07-05_12.24.26.mp4

On the other hand, for users using the mouse, the focus ring becomes a bit annoying when it's always visible, and it can even be mistaken as part of the selected items style. With the changes in this PR, the focus ring won't show when using the mouse.

select-with-mouse-2024-07-05_12.23.44.mp4

There's only and edge case (which was likely the motivation to use focus:ring instead of focus-visible-ring), which is that, if you open the listbox by clicking the select, and then navigate through the options via arrow keys, it feels as if the first item is being skipped, as it is initially not showing a focus ring.

select-edge-case-2024-07-05_12.25.17.mp4

However, I think this is an edge case (people using the keyboard will usually use it all the time), which does not justify the UI implications.


For reference, this is how it currently looks like in main branch, with the focus ring always visible:

select-always-ring-2024-07-05_12.27.03.mp4

@acelaya acelaya requested a review from robertknight July 5, 2024 10:27
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change makes sense, but it also highlights IMO a need to make the selected item stand out more. It currently relies on a small red strip on the left border and bold font, both of which I think are easy to miss at a glance.

Slack thread for reference - https://hypothes-is.slack.com/archives/C1M8NH76X/p1720175368976119

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

Successfully merging this pull request may close these issues.

2 participants