react-restart / ui

MIT License
132 stars 22 forks source link

Incorrect `aria-labelledby` in `useDropdownMenu()` #50

Open catamphetamine opened 2 years ago

catamphetamine commented 2 years ago

https://github.com/react-restart/ui/blob/main/src/DropdownMenu.tsx

  const menuProps: UserDropdownMenuProps = {
    ref: setMenu || noop,
    'aria-labelledby': toggleElement?.id,
    ...popper.attributes.popper,
    style: popper.styles.popper as any,
  };

aria-labelledby is forced to be the id of the toggle element. But that's not correct.

Consider a color selector with options:

Initially the toggler button's text is: "Select a color".

After the user selects "Green" color, the toggler button's background color becomes green and the text inside the button says: "Green".

Then the user focuses out of the toggler button, and then focuses back in. A screen reader says: "Green". "Green" what?

Suppose option labels are "A", "B", "X123".

A correct way would be somehow allowing a custom arial-labelledby property/option, or maybe just an addAriaLabelledBy: false opt-out flag.

catamphetamine commented 2 years ago

This hook is used in react-bootstrap's <Dropdown.Menu/> component, which makes it not be able to support a custom aria-labelledby property.

https://github.com/react-bootstrap/react-bootstrap/blob/master/src/DropdownMenu.tsx

const [menuProps, { hasShown, popper, show, toggle }] = useDropdownMenu({...})

// props.aria-labelledby === 'Custom label'

return (
  <Component
    {...props}
    {...menuProps}
    style={style}
    className={...}
  />
)
jquense commented 2 years ago

A Popover/dropdown isn't a select input replacement. There is a ton of additional a11y to make a select element work correctly and that is beyond the scope of what the dropdown is for out of the box.

I'm ok with making things like this opt out for folks that want to build more complex elements but it takes a significant more than this to make something like a color picker accessible. Generally speaking a dropdown is the wrong category of widget for that.

catamphetamine commented 2 years ago

A Popover/dropdown isn't a select input replacement.

I see.

Well, there's nothing else resembling a select replacement in the components list. https://react-bootstrap.github.io/components/dropdowns/

For some reason, Bootstrap doesn't want to add a custom Select component. https://getbootstrap.com/docs/5.0/forms/select/

A native select's styling capabilities are very limited in terms of the options list style.

Maybe they have their own reasons like better accessibility. It's not our choice as developers though, because we have to implement whatever design document gets passed down.

jquense commented 2 years ago

There are lots of options for custom form input libraries! I am partial to http://jquense.github.io/react-widgets/ myself.