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

Dropdown needs accessibility improvements #1147

Closed
carmacleod opened this issue Sep 25, 2018 · 11 comments · Fixed by #3586
Closed

Dropdown needs accessibility improvements #1147

carmacleod opened this issue Sep 25, 2018 · 11 comments · Fixed by #3586
Assignees
Labels
severity: 2 https://ibm.biz/carbon-severity type: a11y ♿ type: bug 🐛

Comments

@carmacleod
Copy link
Contributor

carmacleod commented Sep 25, 2018

The Dropdown component (I only tried the first one on the code page) has several accessibility problems:

  1. Having a label that disappears is not considered very accessible (similar to placeholder in text input not being considered very accessible). I have seen devs try to give this a label with <label> (which doesn't work here) or hook up a span with aria-labelledby. Fixing this "disappearing label" problem may involve a slight redesign. :)
  2. Typing Esc closes the list, but it also closes the side nav, so the key handler for Esc needs to stop propagation and maybe also prevent default.
  3. The markup is confusing to screen readers:
    • it thinks this is a "list with 2 items": the bx--dropdown-text item and the item containing the dropdown list. It may help to to change the outer <ul> and the 2 <li> into divs (or give them role="presentation").
    • it doesn't know that the dropdown list drops down, so it doesn't tell the user that there's a dropdown available
    • it reads all of the items in the dropdown list right away, even though they are not showing

Some aria may be needed to make this more understandable (maybe aria-haspopup="listbox" and aria-expanded="true/false"). Please read the "menu button" pattern and the "combobox" pattern in the ARIA Authoring Practices Guide to see what makes the most sense. They are both similar to what you are trying to achieve, I think.

@carmacleod
Copy link
Contributor Author

Also, I have seen this component used in other ways that the component may not have anticipated:

  • as a drop-down list of links in a nav
  • as a drop-down list of buttons that do something, like in a traditional desktop menu

You may want to provide similar, but different, components for those use cases.
Or at least show examples of how to use dropdown for these purposes and mark them up correctly for accessibility. This article compares the different accessibility markup for 2 use cases above.

@elizabethsjudd
Copy link
Contributor

We will be having the prototype mentioned here #1589 (comment) done this week and the remaining work within the next couple of sprints.

@stale
Copy link

stale bot commented May 1, 2019

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. Thanks for your contributions.

@stale stale bot added the wontfix label May 1, 2019
@dakahn dakahn removed the wontfix label May 2, 2019
@dakahn
Copy link
Contributor

dakahn commented May 2, 2019

Circling back on this and confirming certain screen reader issues still exist post v10

Tested using NVDA on Windows 10 in Firefox latest

  1. screen reader announces component as a "list clickable" and then proceeds to read everything in the list.
  2. screen reader does not announce that this component is a drop down menu
  3. screen reader does not announce when a new selection has been made
  4. While screen reader is running and the component has keyboard focus the user can hit the down arrow and navigate through the list (while it is closed). This is a strange one 😕, and would require further investigation

@carmacleod
Copy link
Contributor Author

@joshblack
Was this issue fixed? I don't see a relevant commit. I believe that React 1147 was fixed, but not Vanilla 1147 (this one)?

@tw15egan tw15egan reopened this May 8, 2019
@dakahn dakahn added the severity: 2 https://ibm.biz/carbon-severity label May 8, 2019
@elizabethsjudd
Copy link
Contributor

@dakahn is this issue for React or Vanilla? We will be working on dropdown in the next week or so and would prefer to make updates directly to Carbon instead of overwriting in our system. I could spare some time and pick this up if so.

@elizabethsjudd
Copy link
Contributor

@dakahn we'll be working on Vanilla dropdown in the next couple of sprints if you want to assign this to me I can work on it.

@tw15egan
Copy link
Collaborator

Related issue: #2600

@elizabethsjudd
Copy link
Contributor

I'm planning on breaking this issue up in to multiple PRs just to keep the size of the changes easier for reviewing. The first PR will have HTML/styles (with minor JS changes just to keep it "working") and the second PR will contain more robust JS changes for complete a11y updates.

@tw15egan
Copy link
Collaborator

Thanks for the update, @elizabethsjudd!

@elizabethsjudd
Copy link
Contributor

I combined the two PRs because the JavaScript wasn't as big of updates as anticipated. This PR should close this issue as the first 2 items were already fixed so I focused on the screen reader experience updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: 2 https://ibm.biz/carbon-severity type: a11y ♿ type: bug 🐛
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants