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

Critical accessibility issues: aria roles and aria-label #3355

Closed
geoffrich opened this issue Jan 15, 2019 · 24 comments · May be fixed by baophucct/codesandboxer#1
Closed

Critical accessibility issues: aria roles and aria-label #3355

geoffrich opened this issue Jan 15, 2019 · 24 comments · May be fixed by baophucct/codesandboxer#1
Labels
category/accessibility Issues or PRs related to accessibility issue/reviewed Issue has recently been reviewed (mid-2020)

Comments

@geoffrich
Copy link

When running the Axe accessibility tester against the react-select v2 code-sandbox, two critical issues are identified:

  • Form elements must have labels. While you can pass aria-label as a prop to the Select component, it is not assigned as an attribute to the #react-select-2-input HTML element. Adding this attribute satisfies the Axe tester.
  • Certain ARIA roles must be contained by particular parents. Each .select__option has role="option"; however, the parent div .select__menu-list does not have role="listbox". Adding this attribute satisfies the Axe tester. Note that I had to use React Developer Tools to set menuIsOpen on the StateManager to see this violation.

Below are screenshots of the Axe test results.
Form elements must have labels

Certain ARIA roles must be contained by particular parents

@gwyneplaine
Copy link
Collaborator

thanks for this @geoffrey1218, we'll have a closer look at these to harden accessibility in selects.
If you have the time and thoughts on how to do so, PRs are warmly welcome.

@gwyneplaine
Copy link
Collaborator

@sarahbethfederman if you could have a look at this issue, that would be awesome

@sarahbethfederman sarahbethfederman self-assigned this Feb 4, 2019
@sarahbethfederman
Copy link
Collaborator

Hi! I'm not able to reproduce the first issue. May be that it was fixed in react-autosize-input. When providing an aria-label prop to the Select, it seems to be applied to the input. Here's a sandbox demo-ing this with a label value of "this is a label!!!" https://ryw61w7rnn.codesandbox.io/
Running axe on this sandbox produces no valid issues.

Looking into second issue now :)

@sarahbethfederman
Copy link
Collaborator

I think this issue can be closed

@sarahbethfederman
Copy link
Collaborator

(someone yell at me if I'm not supposed to close issues)

@bmsuseluda
Copy link

doesn't work for me if isSearchable is false. Can you check the reason behind this?

@sarahbethfederman
Copy link
Collaborator

@bmsuseluda can you be a bit more specific about what part isn't working for you? Happy to look into this.

@bmsuseluda
Copy link

Sorry for my short message.
I'm using the following code:
<ReactSelect className={styles.dropdown} options={options} value={value} defaultValue={defaultValue} isLoading={isLoading} isDisabled={isDisabled} onChange={handleOnChange} placeholder={placeholder} components={{ DropdownIndicator, Menu }} isSearchable={false} ariaLabel={label} />
My problem is that the aria-label will not be set on the input element if isSearchable is false.

@IanLunn
Copy link

IanLunn commented Jul 4, 2019

Issue should be reopened. aria-label isn't applied if isSearchable is set to false.

The input reported by Axe is <input id="react-select-2-input" readonly="" tabindex="0" class="css-gj7qu5-dummyInput" value="">

@bmsuseluda You should actually be using hyphen cased syntax for aria attributes but that is not the issue here -- aria-label doesn't work either.

@ZLJasonG
Copy link

ZLJasonG commented Aug 22, 2019

Can confirm this is still broken in 3.0.4

@ZLJasonG
Copy link

Looks like the issue is in Select.js renderInput, if !isSearchable it renders a DummyInput without applying the aria attributes that it does to the normal Input it renders otherwise.

@nice-rain
Copy link

@ZLJasonG I am having this exact same issue too. If when isSearchable is false, it will not pass through the aria label.

@lauracabre
Copy link

I am also having this issue. @sarahbethfederman could this issue be reopened?

@mkandler
Copy link

For anyone else who stumbles upon this thread and is looking for a temporary fix...
You can add an accessible label to the input by wrapping the component in a <label> element. This will work with screen readers and VoiceOver.

<label>
  Name of label
  <Select ... />
</label>

@bladey bladey added the category/accessibility Issues or PRs related to accessibility label May 28, 2020
@bladey bladey added pr/in-review PRs currently in review by maintainers for the next release issue/reviewed Issue has recently been reviewed (mid-2020) and removed pr/in-review PRs currently in review by maintainers for the next release labels Jun 17, 2020
@bladey bladey added the issue/has-pr Issue has a PR attached to it that may solve the issue label Jul 6, 2020
@kschwarz1116
Copy link

kschwarz1116 commented Jul 20, 2020

I'm still encountering this on 3.1. isSearchable doesn't appear to be the only cause, but I'm still investigating on that front.
@bladey & @snyk-bot, it looks like the linked PR doesn't actually fix this, it was only referencing #3407 which was why this issue got closed and then re-opened before.

@bladey bladey removed the issue/has-pr Issue has a PR attached to it that may solve the issue label Jul 21, 2020
@bladey bladey added issue/reviewed Issue has recently been reviewed (mid-2020) and removed issue/reviewed Issue has recently been reviewed (mid-2020) labels Aug 24, 2020
@onebree
Copy link

onebree commented Nov 11, 2020

Applying aria-label simply to the <input /> isn't enough. Just upgraded to v3, excited by the aria-label inclusion. However, that's the only place it gets applied. There's no other aria properties to set for say, the X to remove a multi-select option.

I am using React Testing Library, which locates elements like a user would (text, labels, aria-label or aria-role), and because this X is actually an SVG, I'm not sure how to immediately select it (without breaking RTL conventions)

@kylemh
Copy link

kylemh commented Nov 11, 2020

Your point is still valid, @onebree, but just in case this helps ya: https://github.com/romgain/react-select-event

@onebree
Copy link

onebree commented Nov 11, 2020

@kylemh Thanks! I'll definitely consider adding that to the project. In the meantime, I was able to call container via RTL, and querySelector from there

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Nov 14, 2020

I've started a fork of react-select. Feel free to resubmit this issue on the fork and/or submit a PR to resolve this issue on the fork and we can get it merged and released.

EDIT: 🎉 I've archived the fork now that we've got some momentum in this repo and Jed is involved again. Sorry for the disturbance!

@ebonow
Copy link
Collaborator

ebonow commented Jan 18, 2021

Greetings,

This issue was reopened due to the use-case of the aria-label not being applied to the "DummyInput" which impairs accessibility to the Select when isSearchable = false.

I can confirm that this behavior has been addressed here: #3090
demo: codesandbox
Screen Shot 2021-01-18 at 11 48 43 AM

Since the described behavior which caused this issue to be reopened has been resolved, I will be closing this issue again with the understanding that this will give more visibility to remaining accessibility issues.

@ebonow ebonow closed this as completed Jan 18, 2021
@marisev
Copy link

marisev commented Apr 27, 2021

Hi @geoffrich ! In your initial question you referenced react-select v2 code-sandbox, and I see that all options in this example have role="option". Any chance you could share what was the trick to get the role="option" applied in the example?

The only way I see from the docs is by customizing Option, like this: <components.Option role="option" ... />. But it doesn't really work. I am curious how it was implemented in your example. Thanks in advance!

@ebonow
Copy link
Collaborator

ebonow commented Apr 27, 2021

Greetings @marisev,

Attributes can be applied to a custom component by accessing the the innerProps.

I posted an implementation method here specifically for accessibility that might be of interest. #4322 (comment)

Please let me know if you have any questions.

@marisev
Copy link

marisev commented Apr 27, 2021

@ebonow Thank you Eric, I just tried using innerProps and it works! Really appreciate your help, you saved my day!
PS: now I see information about innerProps in the docs, it just wasn't obvious how/when to use it. But it makes sense now.

@ebonow
Copy link
Collaborator

ebonow commented Apr 27, 2021

@marisev glad it's working for you Marina! No worries as the documentation is admittedly a bit dense and easy to miss things. It's in the process of being completely redesigned so it should be a bit easier to sift through everything in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/accessibility Issues or PRs related to accessibility issue/reviewed Issue has recently been reviewed (mid-2020)
Projects
None yet
Development

Successfully merging a pull request may close this issue.