Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add aria-label prop for listbox element #770

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sbeam
Copy link

@sbeam sbeam commented Oct 6, 2020

Our accessibility audit revealed that an aria-label attribute was required on the search results listing given by the ul[role=listbox element. These changes make it possible to pass this down as an optional prop.

@hugoholgersson
Copy link
Collaborator

So here you seem to re-use some commits from PR #645... This PR might be easier to review if you did it all in one commit that includes an updated README and also new tests.

Especially, the README-change you cherry-picked from #645 is no longer accurate. Here you change the aria-label of the <ul>s, but #645 changed the aria-label of the <input> search box...

I understand we use inputProps to pass attributes to the <input>... So perhaps we could need a new, similar, top-level prop to pass attributes to the <ul>s, say listboxProps?

@sbeam
Copy link
Author

sbeam commented Oct 15, 2020

@hugoholgersson you are right on all counts. Thanks for looking. I'll take care of that soon as possible

@mlp73
Copy link

mlp73 commented Nov 30, 2020

Looks promising. Any chance to see that PR merged? @sbeam

@thibaudcolas
Copy link
Contributor

thibaudcolas commented Oct 12, 2021

Without this fix, it’s impossible for a react-autosuggest implementation to be "WAI-ARIA compliant" as it says on the tin. Would be great to get this merged, as well as removing the extra listbox on the parent container (#701, #830).

In the meantime here is an alternative: #778 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants