feat: Adding a new ariaLabelledby prop for use with Assistive Technologies

[olliecurtis]

Hello 👋

Raising a PR to add an aria-labelledby property to the slider.

When using the slider with a separate <label> component to describe the component, when running axe tests we found that there was no way to tie the slider back to the label other than aria-label.

Hope this PR makes sense and follows contributions correctly. Thanks also for a great library!

[stonebk]

I think the attribute should go on the wrapping element rather than on the thumbs: https://www.w3.org/TR/wai-aria-practices-1.2/#wai-aria-roles-states-and-properties-18

If a slider has a visible label, it is referenced by aria-labelledby on the slider element. Otherwise, the slider element has a label provided by aria-label.

Rather than adding a new prop, it would probably make sense to pass unknown props through to the parent element (kind of surprised we aren't doing this already). That way you could just do this without requiring any new props or additional logic:

<ReactSlider aria-labelledby="foo" />

[olliecurtis]

Hey @stonebk

Thanks for the feedback on this! That was my original thought/idea behind this one and I had tried to add this on the slider itself, it seems that axe tests want it on the handle and doesn't seem to think it solves the problem itself.

Here is the error my code recieves when running axe tests on the use of my component.

<div class="bpk-slider__handle bpk-slider__handle-0 " style="position: absolute; z-index: 1; left: 0px;" tabindex="0" role="slider" aria-orientation="horizontal" aria-valuenow="25" aria-valuemin="0" aria-valuemax="100"></div>

    Received:

    "ARIA input fields must have an accessible name (aria-input-field-name)"

    Fix any of the following:
      aria-label attribute does not exist or is empty
      aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty
      Element has no title attribute

This is when the following code is used:

<div>
  <label id="label" htmlFor="slider">
   Slider
  </label>
  <Slider
   {...rest} // This not currently doing anything right now
   id="slider"
   min={0}
   max={100}
   value={25}
   ariaLabelledby="label"
   withTracks
   invert={invert}
   className={classNames}
   thumbClassName={thumbClassNames}
   thumbActiveClassName={getClassName('bpk-slider__handle--active')}
   trackClassName={trackClassNames}
  />
</div>

I also tried with adding aria-labelledby on the slider itself by adding it below the following line: https://github.com/zillow/react-slider/blob/master/src/components/ReactSlider/ReactSlider.jsx#L1109

What are your thoughts on this?

On this point:

Rather than adding a new prop, it would probably make sense to pass unknown props through to the parent element (kind of surprised we aren't doing this already).

Would you be open to contribution on this? Either in this PR or a separate one?

Thanks again @stonebk for your consideration on this PR!

[stonebk]

@olliecurtis looking a bit closer, it appears you are right that it is expecting aria-labelledby on the thumb. Here is an example showing that behavior: https://www.w3.org/TR/wai-aria-practices-1.2/examples/slider/slider-1.html

I think this PR makes sense, but you will want to follow the same logic as ariaLabel so that different values can be applied when there are multiple thumbs.

On this point:

Rather than adding a new prop, it would probably make sense to pass unknown props through to the parent element (kind of surprised we aren't doing this already).

Would you be open to contribution on this? Either in this PR or a separate one?

Yes, we would be open to this contribution! I think it would probably make sense as a separate PR since we'll proceed with this PR.

[olliecurtis]

Thanks @stonebk for the feedback on this! I have gone ahead and setup the same logic as applied to ariaLabel.

I think with this the PR is good for review? Let me know what you think, thanks again for the feedback and acceptance on this!

Yes, we would be open to this contribution! I think it would probably make sense as a separate PR since we'll proceed with this PR.

Brilliant, in which case I shall look at raising a PR to add this functionality!

[olliecurtis]

Thanks @stonebk for the feedback, espcially the commit messages! Have ammended the AT in the first commit also to make it clear what it means.

Apologies I missed the commit guidelines which I now see mentioned in the CHANGELOG file 😄

[stonebk]

react-slider@1.2.0