About the 2d slider

[diegohaz]

Currently, the saturation and brightness picker is using a single role="slider" element, which is supposed to be one-dimensional, but is being used to control a two-dimensional pane.

One of the problems with this approach is the lack of aria-valuenow, which is a required attribute on sliders, but can't be set here because it's dealing with two different axes.

This may be also confusing for screen reader users who expect the slider to work as described in the WAI-ARIA docs.

One of the solutions is using a role="grid" element. This has been discussed on w3c/aria#432. The markup could be something like this:

<div role="grid" aria-label="Saturation and Brightness" aria-rowcount="100">
  <!-- 80% brightness -->
  <div role="row" aria-label="Brightness 80%" aria-rowindex="80">
    <!-- 20% saturation -->
    <div role="gridcell" aria-label="Saturation 20%" aria-colcount="100" aria-colindex="20" />
  </div>
</div>

Another solution would be having two sliders:

<div role="slider" aria-orientation="vertical" aria-valuenow="80" aria-label="Brightness" />
<div role="slider" aria-orientation="horizontal" aria-valuenow="20" aria-label="Saturation" />

Moving the arrow keys perpendicularly to the currently focused slider would move focus onto the other slider.

None of these solutions should affect the current layouts.

[omgovich]

Hi! 👋 Thanks for using react-colorful and for the issue. I'll dig into the problem and try to roll out an approach that would provide better a11y and keep the bundle size as small as possible 👌

[eGene]

Saturation and Hue components should accept props from user, so that we could override aria-* attributes. This way I could just pass something like

saturationInputProp={{
  'aria-valuenow': '#12,34,56',
  'aria-label': 'Цвет'
}}

This also solves issue with hardcoded strings if I need non-english labels.

[omgovich]

Hey guys! Don't think I forget about you. I've been discussing the issue with guys that know more about a11y than me.

The thing that bothers me much and I still didn't get — which element must take the focus? grid or gridcell?

[omgovich]

Hey @diegohaz. Could you please check https://omgovich.github.io/react-colorful/

I deployed a version with role="grid" (see the PR) there but seems to me that it doesn't work well: My macOS VoiceOver doesn't say anything when I change value.

[diegohaz]

Hey @diegohaz. Could you please check https://omgovich.github.io/react-colorful/

I deployed a version with role="grid" (see the PR) there but seems to me that it doesn't work well: My macOS VoiceOver doesn't say anything when I change value.

Yeah! I can see the same behavior. VoiceOver also has its own shortcut keys to navigate grids, and they don't seem to work with this virtualized grid approach.

Have you tried the solution with two sliders?

[omgovich]

Not sure I understand how to make it work 😞

The thing is we need to set focus on the entire brightness/saturation area, but I don't understand how to do that if there are two separate div-s with role="slider"...