base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[Slider] Issues with adding focus-visible styles to the thumb slot

Open danilo-leal opened this issue 1 year ago • 5 comments

As surfaced in https://github.com/mui/material-ui/pull/40332, there seems to be a bug with the focus treatment for the Slider component.

  • Suppose the focus styles are added to the thumb slot using our workaround class Mui-focusVisible through ${sliderClasses.focusVisible}. In that case, the problem is that they persist even if you move away from the thumb after clicking, dragging, and leaving it. That's illustrated in this comment: https://github.com/mui/material-ui/pull/40332#issuecomment-1887509912
  • If the focus styles are added using the native focus-visible pseudo-class instead, the above-described behavior does not happen anymore. However, the problem then is that none of the styles appear if focusing/targeting the Slider's thumb using the keyboard.

Search keywords: base-ui, slider, thumb, bug

danilo-leal avatar Jan 17 '24 22:01 danilo-leal

I see two potential solutions to fix the root issue here, each of them is going to require non-trivial refactoring of the useSlider hook:

  1. The first solution would be using the role="slider" directly on the thumb <span /> element and getting rid of the range input and use a hidden input instead. This would mean that we will need to handle all of the touch/keyboard events typically handled by the <input type="range" /> element we use right now. Also, we should validate before going deeper if this will be fully accessible, as there are some warnings around this pattern in https://www.w3.org/WAI/ARIA/apg/patterns/slider/
  2. The second solution would be to try to merge the thumb and the input into one element. This should allow us to use the input's native focus, focus-visible selectors directly on the thumb (as they would be one element).

mnajdova avatar Jan 22 '24 11:01 mnajdova

@danilo-leal @mnajdova we would like to pick this up

gitstart avatar Feb 01 '24 10:02 gitstart

Go for it. I'd assume that option 2, which @mnajdova has provided, is the preferred one to start exploring to solve this issue!

danilo-leal avatar Feb 01 '24 11:02 danilo-leal

I see two potential solutions to fix the root issue here, each of them is going to require non-trivial refactoring of the useSlider hook:

  1. The first solution would be using the role="slider" directly on the thumb <span /> element and getting rid of the range input and use a hidden input instead. This would mean that we will need to handle all of the touch/keyboard events typically handled by the <input type="range" /> element we use right now. Also, we should validate before going deeper if this will be fully accessible, as there are some warnings around this pattern in https://www.w3.org/WAI/ARIA/apg/patterns/slider/
  2. The second solution would be to try to merge the thumb and the input into one element. This should allow us to use the input's native focus, focus-visible selectors directly on the thumb (as they would be one element).

The second approach faces a challenge as the focus and focus-visible selectors persist when the mouse moves out of the host element. A pseudo-CSS snippet attempts to have an insight into the behavior when we finally merge both the input and thumb and can directly use the pseudo-class on the input element as shown below, but it doesn't provide the desired solution.

&:has(> input:focus-visible) {
    // box-shadow styles;
}

The reason why it seems when focus styles are added using the native focus-visible, behavior does not happen anymore is because the focus-visible style is not applied to the span element at any point in time, as the span lacks the focus-visible pseudo-class. The shadow effect is only derived from the MUI active class, which is removed when the mouse leaves the slider's thumb.

If the focus styles are added using the native focus-visible pseudo-class instead, the above-described behavior does not happen anymore. However, the problem then is that none of the styles appear if focusing/targeting the Slider's thumb using the keyboard.

While we await feedback/more clarity on this, we would like to state that one potential solution is to remove the MUI focus-visible class onMouseUp

cc @danilo-leal @mnajdova

gitstart avatar Feb 05 '24 14:02 gitstart

The second solution would be to try to merge the thumb and the input into one element. This should allow us to use the input's native focus, focus-visible selectors directly on the thumb (as they would be one element).

In the new API this is solved by not exactly by merging the thumb and input into one, but instead just swapping their tabIndex and fixing up the handling of keyDown

The (new) Thumb which is a span, can be styled directly with :focus-visible

Demo: https://deploy-preview-373--base-ui.netlify.app/experiments/slider/

mj12albert avatar May 22 '24 09:05 mj12albert