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

[Accessibility] Accessibility feedback #655

Closed
changLiuUNSW opened this issue Oct 25, 2017 · 18 comments
Closed

[Accessibility] Accessibility feedback #655

changLiuUNSW opened this issue Oct 25, 2017 · 18 comments

Comments

@changLiuUNSW
Copy link

changLiuUNSW commented Oct 25, 2017

In the official Bootstrap 4 all the components written in JQuery are fully accessible.
https://getbootstrap.com/docs/4.0/getting-started/accessibility/
But in this library, most components are not accessible at all. Does this library totally ignore the accessibility?
Actually most big companies like us Bank, we take accessibility very seriously when we choose a third part library.
We really hope the author of this library can take accessibility seriously like React-Bootstrap.
https://github.com/react-bootstrap/react-bootstrap/wiki#100-roadmap (See Accessibility section)

@changLiuUNSW changLiuUNSW changed the title Does this library care about accessibility? Does this library totally ignore the accessibility? Oct 25, 2017
@TheSharpieOne
Copy link
Member

TheSharpieOne commented Oct 25, 2017

Yes, this library is 508 compliant as much as bootstrap itself is. We have keyboard navigation/control of dropdowns and such, provide aria-* where applicable, and allow the user of the library to add their own attributes to all of the components (such as when tying a DropdownMenu to it's label via aria-labelledby. Since we do not generate IDs, the user would need to provide that).
No library is perfect, and as you can see other issues have been opened about a11y with specific components and those issues have been resolved. If you have specific concerns, please list them here or in individual issues so that they can be addressed.

@changLiuUNSW
Copy link
Author

changLiuUNSW commented Oct 25, 2017

Take modal dialog for example (https://getbootstrap.com/docs/4.0/components/modal/):

  1. Modal container has role = dialog
  2. Modal content has role = document
  3. Modal must be focused when it is opened
  4. Tab and Shift+Tab do not move focus outside modal
  5. When modal closes, focus returns to the element that had focus before the dialog was invoked.
  6. Keyboard interaction Tab/Shift +Tab: move focus to the next/previous focusable element.

Similar to other components like menu, tooltip, accordion, tabs and etc. Actually all the accessibility implementation for those components can be found in Bootstrap 4 official website.

@TheSharpieOne
Copy link
Member

  1. Modal container has a default role of "dialog" but it is can overridden by providing you own role on Modal.
  2. Yes, modal content has a role of "document" and that role cannot be overridden, is that the issue?
  3. The Modal is focused when it is opened, you can even opt out of this by provided autoFocus={false} in case you want to focus on something within the modal (like an input) yourself.
  4. Modal focus is an open issue Modal should trap focus #310, but even bootstrap's focus trap has a major a11y problem as it traps the focus too well, the keyboard user cannot tab to get to the browser's chrome. It completely traps the user on the page. This needs to be solved first. The only worse than not having an a11y feature is having a bad one.
  5. This is related to 4, but probably could be it's ticket.
  6. This is how the browser works. Use the right tabindex values or do not use them at all and the browser will tab to the next and previous focusable element automatically. You should also order your components element logically.

So 4 and 5 are the only issues, 4 is being tracked, but event bootstrap has a11y issues with it's implementation. 5 is related (and would probably be fixed at the same time), but can be it's own issue.

@changLiuUNSW
Copy link
Author

changLiuUNSW commented Oct 31, 2017

Thanks @TheSharpieOne. Yes 4 and 5 are the only issues but both are very important for accessibility. Hopefully they can get fix soon.

The following are other accessibility issues I find:

Tooltips
https://getbootstrap.com/docs/4.0/components/tooltips/

  1. missing role="tooltip" for the tooltip container
  2. missing aria-describeby when it is open

Popover
https://getbootstrap.com/docs/4.0/components/popovers/

  1. missing role="tooltip" for the popover container
  2. missing aria-describeby when it is open

Tabs
https://getbootstrap.com/docs/4.0/components/navs/#javascript-behavior

  1. missing role="tablist" for the ul
  2. missing role="tab",aria-controls and aria-selected for the li
  3. missing id, role=tabpanel and aria-labelledby for tab panel

Collapse
https://getbootstrap.com/docs/4.0/components/collapse/

  1. missing aria-expanded and aria-controls for the toggle button
  2. missing id for collapse container

Radio buttons and checkbox buttons
https://getbootstrap.com/docs/4.0/components/buttons/#button-plugin

  1. semantically all radio buttons should be native radio input eg:
<label class="btn btn-secondary active">
    <input type="radio" name="options" id="option1" autocomplete="off" checked=""> Radio 1 (preselected)
  </label>
  1. semantically all checkbox buttons should native checkbox eg:
<label class="btn btn-secondary">
    <input type="checkbox" checked="" autocomplete="off"> Checkbox 1 (pre-checked)
  </label>

breadcrumb
https://getbootstrap.com/docs/4.0/components/breadcrumb/#accessibility

  1. missing aria-current="page" for current active link
  2. semantically breadcrumb list should be wrapped within nav with role="navigation" and aria-label="breadcrumb"

Button Dropdown and Dropdowns
https://getbootstrap.com/docs/4.0/components/dropdowns/

  1. missing aria-labelledby for dropdown menu

@changLiuUNSW
Copy link
Author

changLiuUNSW commented Oct 31, 2017

Here is the question I asked on Bootstrap 4 github page regarding their accessibility standard:
twbs/bootstrap#22406
You can see the reply from @patrickhlauke:

we strive towards making interactive controls/widgets expose all the correct roles/values/state properties (to satisfy, say, 4.1.2), to make things works with keyboard (2.1.1).

Actually we did test for Bootstrap 4 before and we find out most JQuery components they build are accessible for both keyboard and screen reader users and also meet WCAG 2.0 AA/AAA standard.

I think Accessibility implementation of this library should align with official JQuery components of Bootstrap 4, because Accessibility is also a very important feature of Bootstrap 4.
https://getbootstrap.com/docs/4.0/getting-started/accessibility/

@changLiuUNSW changLiuUNSW changed the title Does this library totally ignore the accessibility? [Accessibility]Does this library totally ignore the accessibility? Oct 31, 2017
@gergely-nagy
Copy link
Collaborator

gergely-nagy commented Nov 17, 2017

I'd like to work on this. Can I start doing it @TheSharpieOne ?

@TheSharpieOne
Copy link
Member

@gergely-nagy go for it

gergely-nagy added a commit to gergely-nagy/reactstrap that referenced this issue Nov 19, 2017
@changLiuUNSW changLiuUNSW changed the title [Accessibility]Does this library totally ignore the accessibility? [Accessibility] Accessibility feedback Dec 1, 2017
@joelwross
Copy link

Further issues: make sure you're not adding extraneous semantic HTML elements when using these components. For example, <CardTitle> has a default tag of h4, meaning that the "default" card examples have lots of extra headings. This interferes with navigation on Screen Readers. Instead, these components should have default tags that are not semantically loaded (e.g., span).

@gergely-nagy
Copy link
Collaborator

I didn't have much time in the last 2 months, but I will work on this issue again.

@mackbrowne
Copy link

I'm currently having some issues with the navbar. The tab key / arrow keys for dropdowns work as expected. However in all the examples the tab button will 'skip' all the links inside the dropdown when it's closed. For me, it tabs through all the links inside the dropdown regardless if the dropdown is open.

How do i fix this!? I just want it to work like the examples. I've updated all my packages to the latest and double checked that I have all the classnames the example has. Not sure where to go from here....

@TheSharpieOne @changLiuUNSW any suggestions? The boss is breathing down my neck for this one...

@TheSharpieOne
Copy link
Member

@mackbrowne Are you able to post code or create a demo showcasing this (https://stackblitz.com/edit/reactstrap-v6?file=Example.js)

@mackbrowne
Copy link

well this is awkward, I stripped out a bunch of stuff just to get the example working and it works now... the biggest difference I can see is that I've wrapped a bunch of Reactstrap components with styled-components. This has never been an issue in the past, so I can't imagine that's the problem. I just don't know what is....

Anyway @TheSharpieOne thanks for the sanity test. I'll report back when I figure out what specifically is breaking it.

@TheSharpieOne
Copy link
Member

I am closing this as I have opened #1012 to track tooltip a11y and #310 tracks the other outstanding issue.
As pointed out above by @changLiuUNSW, bootstrap itself doesn't solve accessibility (just look at their docs, none of the tooltips and popovers and such have roles), it provides a framework for the developers to determine accessibility attributes to apply to various component and apply those attributes themselves (said roles can be added to their docs easily, but each developer would need to also add them to their project in the same way, manually and individually). reactstrap is mostly the same but does provide some of those attributes out-of-the-box more-so than bootstrap to make the developer's life easier.
We probably need an accessibility page on the docs so I have open #1013 to track that effort.

@mackbrowne please open a new ticket with more information about your particular issue if you still require assistance.

@mackbrowne
Copy link

@TheSharpieOne and anyone else, just reporting back. I'm using animation to open/close the dropdown menu.

I found this snippet. it ended up being the issue.

display: block

on the DropdownMenu component, for some reason breaks the tabbing on the navbar, no idea why but that was the issue.

@TheSharpieOne
Copy link
Member

That snippet is for bootstrap v3, I may not be compatible with bootstrap v4 which is why you get that issue

@annetters
Copy link

Came here after I noticed the Accordion (collapse) does not have Bootstraps aria-expanded controls.

https://getbootstrap.com/docs/4.0/components/collapse/#accessibility

Has there been a fix for this?

@TheSharpieOne
Copy link
Member

@anevaude I opened #1627 to track that effort.

@annetters
Copy link

@anevaude I opened #1627 to track that effort.

Oh awesome, thanks! I'll follow that.

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

No branches or pull requests

6 participants