fix(TreeView): improved a11y experience#6480
Conversation
| ### Accessibility | ||
|
|
There was a problem hiding this comment.
This section/table no longer needed with the a11y doc update being made in org.
|
Preview: https://patternfly-pr-6480.surge.sh A11y report: https://patternfly-pr-6480-a11y.surge.sh |
| {{#if tree-view-list-item--IsExpandable}} | ||
| aria-expanded="{{#if tree-view-list-item--IsExpanded}}true{{else}}false{{/if}}" | ||
| {{#if tree-view-node--HasCheckbox}} | ||
| {{#if tree-view-list-item--HasCheckbox}} |
There was a problem hiding this comment.
I don't think this was working as intended since the tree-view-node was a child of tree-view-list-item, so I just made this update to be able to pass things down to children.
cb7fd16 to
4285f53
Compare
There was a problem hiding this comment.
Makes sense to me, but definitely defer to @mcoker's opinion!
|
@jessiehuff @mcoker The latest commit updates tabindices to match what is currently mentioned in the treeview a11y docs update PR. Basically only the first focusable item in each treeview example has a tabindex of 0, all other focusable elements have tabindex of -1. I've also removed the tabindex off of the The intent is that only a single element inside the treeview will ever have a 0 tabindex, and arrow keys must be used to navigate the treeview (updating the tabindices as you navigate through). The React keyboard navigation logic will need to be updated to work more properly (the PR I have up currently helps improve it). The issue with having complicated tree views where there may be action items, checkboxes separate from the expansion, or selection separate from the expansion, is that it isn't clear at all that those things exist/are separate especially for screen reader. Similar to our Menu examples where there may be secondary actions, a user may not know they can navigate horizontally in addition to vertically. That, and Left/Right Arrow are expected to have specific functionality depending on the expanded state of the currently focused tree view node, which may be confusing/frustrating if the expectation is that "Right arrow should expand this parent node" but focus is instead moving from the expand toggle to the node text button. A basic tree view is probably easy enough to traverse and understand, but beyond that some sort of visible text description/instruction may need to be included. For example, something like:
(just added a div element inside What do you both think? Maybe this is something that requires a separate, more in depth discussion, but wanted to propose the above in any case. |
|
🎉 This PR is included in version 5.4.0-prerelease.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* fix(TreeView): improved a11y experience * Updated aria-label to match React PR * Updated tabindices * Reverted changes affecting tabindices
* fix(TreeView): improved a11y experience * Updated aria-label to match React PR * Updated tabindices * Reverted changes affecting tabindices
* fix(release): updated code to release patch to npmjs * fix(TreeView): improved a11y experience (#6480) * fix(TreeView): improved a11y experience * Updated aria-label to match React PR * Updated tabindices * Reverted changes affecting tabindices * fix(pagination): fixed display for per page menu toggle (#6586) * fix(pagination): fixed display for per page menu toggle * chore(pagination): s/pagination-options-menupagination-menu-toggle * chore(pagination): remove top expanded, add toggle aria-label * Update pagination-menu-toggle.hbs remove aria-label --------- Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com> Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com>
|
🎉 This PR is included in version 5.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #6478