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

WCAG support #216

Closed
ellinge opened this issue Mar 11, 2019 · 9 comments
Closed

WCAG support #216

ellinge opened this issue Mar 11, 2019 · 9 comments

Comments

@ellinge
Copy link
Collaborator

ellinge commented Mar 11, 2019

There's currently no support for keyboard navigation and no ARIA-attributes present.
Here are some guidelines one should consider complying with WCAG:
https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html

Here are some of the highlights on attributes one should consider implementing:

  • role
  • aria-haspopup
  • aria-expanded
  • aria-selected
@mrchief
Copy link
Collaborator

mrchief commented Mar 11, 2019

Thanks for opening this. Having WCAG support would be great. I believe there are no clear guidelines for a mixed or combo control like this.

This component essentially is a mix of:

  • an input
  • a dropdown
  • a TreeView
  • label + checkboxes (each tree node)
  • tags (selected tree nodes that have text plus a "x" button)
  • node action item(s) - these are optional but essentially they are just buttons

To fully implement WCAG in a meaningful way, we'd need the following:

To add ARIA attributes

To add Keyboard navigation

  • Figure out what Keyboard navigation should look like with all possible variations (e.g., with or without checkboxes, with or without action items, scrolling, adding/removing/navigating tags, etc.)
  • Handle scroll virtualization
  • PR for Adding keyboard navigation capabilities (feat: Add keyboard navigation #225)
  • Automated tests to ensure compliance (preferably integrated with CI)

As you can see, the scope can be quite broad for getting it right a 100%, but I think we can probably add stuff incrementally in multiple PRs.

I'll start looking into it, and I'm open to ideas, suggestions, and PRs for this.

@ellinge
Copy link
Collaborator Author

ellinge commented Mar 25, 2019

Did some first tries to include a11y-testing through axe:
https://github.com/ellinge/react-dropdown-tree-select/tree/feat/%23216-tests

It only detects missing labeling for search input though. Not sure it will detect our scenario/needs. Since some dom-nodes are not rendered by React, it propably cannot detect that we expand and collapse child nodes. Not sure of it's capabilties. But it probably will be able to validate the attributes we apply I guess.

They also state

There is limited support for JSDOM. We will attempt to make all rules compatible with JSDOM but where this is not possible, we recommend turning those rules off. Currently the color-contrast rule is known not to work with JSDOM.

Here is the rule list:
https://github.com/dequelabs/axe-core/blob/develop/doc/rule-descriptions.md

ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Mar 27, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Mar 31, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 1, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 1, 2019
ellinge pushed a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 2, 2019
ellinge pushed a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 2, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 2, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 3, 2019
ellinge pushed a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 3, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 4, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 4, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 4, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 5, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 5, 2019
@ellinge
Copy link
Collaborator Author

ellinge commented Apr 6, 2019

There’s automated tests in place in #225 that will give the same result as Chrome's audit.
Scroll virtualization? Do you mean replacing infinitescroll with a virtualized list/window instead?
#225 currently scrolls to current focused item

@mrchief
Copy link
Collaborator

mrchief commented Apr 6, 2019

Scroll virtualization

I meant scroll should work with arrow/pg up/dn keys...

@ellinge
Copy link
Collaborator Author

ellinge commented Apr 6, 2019

Scroll virtualization

I meant scroll should work with arrow/pg up/dn keys...

Alright, then that's in place in #225 also. A virtual window would be nice though and solve another open issue I think. Lots of potential nodes to render on page down 🐢 Most of them are based on fixed heights though so unsure how that would be solved good with custom css and long labels etc...

The audit for Chrome/axe doesn't solve more than validation of added aria-attributes for us, custom controls are open to interpretation. But Code climate seems to be aware of them already so perhaps they are redundant and should be removed.

@mrchief
Copy link
Collaborator

mrchief commented Apr 6, 2019

A virtual window would be nice though

Do you mean something like react-window or similar? I've considered many in the past and most of them require a fixed height which is kind of a downer for me. It's an additional burden for the caller which I would like to avoid. The current solution seems like a good one though, is there something bothering you?

Lots of potential nodes to render on page down

Page down should render the next page, similar to what happens when you reach the last item via mouse scroll (it renders the next set of 100 items, which is our page size).

The audit for Chrome/axe doesn't solve more than validation of added aria-attributes for us, custom controls are open to interpretation. But Code climate seems to be aware of them already so perhaps they are redundant and should be removed.

I was wondering the same. Between code climate and our RFC assertions, we should be ok so yeah, remove axe if you prefer so.

ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 6, 2019
@ellinge
Copy link
Collaborator Author

ellinge commented Apr 7, 2019

Page down should render the next page, similar to what happens when you reach the last item via mouse scroll (it renders the next set of 100 items, which is our page size).

It currently goes to the actual last item. Guess End should only that that then (or arrow up on an unfocused control)

I was wondering the same. Between code climate and our RFC assertions, we should be ok so yeah, remove axe if you prefer so.

Realized since we set them dynamically, code climate won't detect them.
So, I'll leave axe since that's at least "rendered", even if only virtually.

ellinge pushed a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 8, 2019
ellinge pushed a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 8, 2019
mrchief added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 14, 2019
# Conflicts:
#	.codeclimate.yml
#	.prettierrc
#	src/index.js
#	src/input/index.js
#	src/tag/index.js
#	src/tree-node/index.js
#	src/tree-node/index.test.js
#	src/tree/index.js
#	yarn.lock
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 15, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 18, 2019
ellinge added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 20, 2019
mrchief added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 22, 2019
mrchief added a commit to ellinge/react-dropdown-tree-select that referenced this issue Apr 24, 2019
# Conflicts:
#	__snapshots__/src/index.test.js.snap
@mrchief mrchief closed this as completed in 7133ed2 May 3, 2019
mrchief pushed a commit that referenced this issue May 3, 2019
@mrchief
Copy link
Collaborator

mrchief commented May 3, 2019

🎉 This issue has been resolved in version 1.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mrchief
Copy link
Collaborator

mrchief commented Jun 10, 2019

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

2 participants