algolia / instantsearch

⚡️ Libraries for building performant and instant search and recommend experiences with Algolia. Compatible with JavaScript, TypeScript, React and Vue.
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/js/
MIT License
3.73k stars 526 forks source link

Differentiate :focus and :focus-visible styles #5192

Open sarahdayan opened 3 years ago

sarahdayan commented 3 years ago

When clicking a pagination link in the Pagination widget, the link receives the focus state, which applies the :focus styles while it doesn't necessarily needs it. This is a different use case than tabbing to the link, where those styles are necessary to see where the focus is.

To account for this, we could differentiate between :focus and :focus-visible styles in our themes to avoid applying focus styles on elements which focusstate doesn't need to be made obvious.

The focus-within state isn't applied by all browsers, but we can set up fallbacks to account for those.

Haroenv commented 3 years ago

Is there really a problem with showing the focus indicator?

sarahdayan commented 3 years ago

Focus styles are undesirable when they result from pointer actions. Most browsers don't apply focus indicators when you click with a mouse/trackpad/finger, even if the element receives focus, because they consider they're not needed. However, as soon as we style the focus state explicitly, these styles always apply when the element receives focus (see demo).

https://user-images.githubusercontent.com/5370675/135835204-8642f434-4c8e-4af9-93fd-42b102af0c6e.mov

With focus-visible you only have styles applied when the browser considers these styles should apply. Using focus-visible is more respectful of browser's default behavior.

Additionally, in our Algolia theme it's even more problematic because we use the same styling for both hover and focus, which can legitimately create confusion. Different states need different styles. Right now if you're using the theme, it will look like the clicked navigation item is "stuck" on hover.

tkrugg commented 3 years ago

Thanks for the demo, it really clarifies the issue. Worth nothing :focus-visible support is lacking:

image source: https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible#browser_compatibility

I guess as we try to refine the "what good looks like" aspect of things, this could totally make sense!

sarahdayan commented 3 years ago

@tkrugg As mentioned in the issue, there are fallbacks for it so it's not a problem if some browsers don't support it.