zillow / react-slider

Accessible, CSS agnostic, slider component for React.
https://zillow.github.io/react-slider
MIT License
883 stars 231 forks source link

ReactSlider does name accept name prop #255

Closed LinnJS closed 2 years ago

LinnJS commented 2 years ago

Description

Hello Zillow maintainers πŸ‘‹,

First off would say I love this open-source component and all the time you all took to make it accessible. Currently building a design system for my company with Tailwind and like to make most of the stuff from scratch but when I came across the slide component seemed clear I should bring in an open-source package. When I bring in packages I always prefer them to be headless and have a good Tailwind user experience which your component does beautifully and I love the fact it comes with zero styles.

With all this being said accessibility is always a top concern for me, as it seems to be for you all too. A nice way to make this easy in a TypeScript design system is to extend the native browser API for all bare HTML components, then allow a rest prop to be passed so all native props are supported implicitly. One of the most used props from the native browser is the name prop. Often this prop is needed to correlate labels to particular components to screen readers to know what label to read off when focusing.

Anyways all of this to say it would be nice for the name prop to be supported on the ReactSlider component so when using a label I can put a HTMLfor prop on the label and then put a name on the input so they are intrinsically connected.

I realize I could use an ariaLabel but would not want the screen reader to double read and it seems like a better DX to build the screen reader experience off of a visual user experience rather than duplicating work.

Maybe there is a reason its not on there but to me it seems like a no downside situation.

Code Example

Name gets rejected cause its not supported in SliderProps interface

Whereas the native input element is supposed to support name prop as spec'ed here MDN input spec

export const Slider = ({ label, value, onChange }: SliderProps) => {
  const name = toKebabCase(label);

  return (
    <div>
      <label htmlFor={name} className="block text-sm font-medium text-gray-700">{label}</label>

      <ReactSlider
       name={name}
        value={value}
        onChange={onChange}
        className="w-full pr-2 my-4 bg-purple-700 h-2.5 rounded-md cursor-grab shadow-toggle shadow-rose-900"
        thumbClassName="w-4 h-4 cursor-grab bg-white rounded-full shadow-3xl focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-purple-700 -bottom-0.5"
      />
    </div>
  );
};
kris-ellery commented 2 years ago

Hi @LinnJS, thank you for submitting an issue.

The ReactSlider output is a structure of div tags, there is no input tag to assign a name or id attributes. Speaking of, I think you meant the id attribute to connect label with input tags, the name does not create that connection.

That being said, ReactSlider should allow to pass transient props to the final DOM node. The question is, which one? Root div is only a wrapper, while the thumb div is the one user would interact with.

In the meantime, I would recommend using aria* props currently supported by ReactSlider.