[Data Views] Updating keyboard navigation in list layouts #59637
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewhayward
added
[Focus] Accessibility (a11y)
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
The design of the List layout will likely change a bit for 6.6 but this seems like a solid improvement regardless. Did you consider making the up/down presses select the item instead of just focussing it? |
|
Thanks for working on this! Current interaction sounds good, though select as Jay mentioned may be nice to try. I'll review the code tomorrow.
I couldn't reproduce in trunk 🤔 Gravacao.do.ecra.2024-03-07.as.18.21.29.mov
This happens in trunk and can be addressed separately. |
|
I've looked at the aria patterns and
|
|
Summary of roles & interactive elements 👍 <ul role="grid">
<li role="row">
<div role="gridcell">
<Button>Main content of the list item</Button>
</div>
<div role="gridcell">
<div role="button">Details button</div>
</div>
</li>
</ul> |
oandregal
reviewed
Mar 8, 2024
| const shownData = useAsyncList( data, { step: 3 } ); | ||
| const usedData = deferredRendering ? shownData : data; | ||
| const selectedItem = usedData?.findLast( ( item ) => |
There was a problem hiding this comment.
This changes how selection works for the list item. Particularly, in the case of some items being pre-selected from a different view and then switch to the list view.
Before:
Gravacao.do.ecra.2024-03-08.as.14.31.42.mov
After:
Gravacao.do.ecra.2024-03-08.as.14.30.12.mov
Until we figure out whether it makes sense to have multiple selected elements in the list view, this is a good approach.
oandregal
reviewed
Mar 8, 2024
| { usedData.map( ( item ) => { | ||
| const id = getItemDomId( item ); |
There was a problem hiding this comment.
Why not using getItemId directly? By definition, that's each item's unique identifier.
There was a problem hiding this comment.
Is this something related to how Composite/Ariakit works? Otherwise, I don't follow what the ids are for at this level. Judging by what I see at runtime, I presume 1) Ariakit needs this tracking and 2) we want to make sure the id doesn't conflict with any other. If so, can we go simpler and do something along the lines of list-item-${getItemId( item )}?
There was a problem hiding this comment.
I'm largely unfamiliar with Ariakit's Composite, but I find it weird that it 1) uses DOM ids to track the items and 2) the concept of a store attached to a component.
I understand that, by using Ariakit's Composite, we want to hide a certain layer of complexity (handle onClick/onKeydown events + tab/arrow-key mechanics) though I'd argue that complexity is familiar for most people while the complexity added by Ariakit's Composite mechanics is unfamiliar for most. In terms of maintenance and evolution of this code, I feel a bit uneasy about this trade-off.
There was a problem hiding this comment.
Asked around to some folks and others think this is fine.
|
Code-wise, this is ready in my view. The only thing I'd like a second opinion on is the use of Ariakit's Composite. If we go that direction, this is ready to land. |
oandregal
approved these changes
Mar 11, 2024
This was referenced Mar 11, 2024
oandregal
force-pushed
the
data-views-list-layout-semantics
branch
from
bbb4e40 to
e179975
Compare
|
I've rebased the PR and set it to auto-merge when tests pass. I have upcoming work on selection that I want to base on this PR. Thanks for your work here, Andrew! |
oandregal
added
[Type] Enhancement
|
Size Change: +414 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
29 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds grid semantics to the list layout, and makes use of
aria-labelledbyandaria-describedbyin list items.Why?
The list layout currently presents as a long list of buttons, each its own tab stop, which makes it difficult to reach the associated panel content. This is further complicated when grid items have additional detail buttons, doubling the number of tabs stops. Changing the list to a grid, and supporting Up/Down key navigation, means we can reduce the list to a single tab stop, providing easier access to the associated content.
Using
aria-labelledbyandaria-describedbyon list items allows us to reduce the information density, such that each item only presents with the primary field as a title, with the other fields available as additional descriptive content. This means that information can be scanned much faster.How?
Compositecomponent family has been used to add arrow key navigation to the list and list items.aria-labelledbyandaria-describedbyrespectively.Testing Instructions
List layouts should not visually be any different, and behaviour should continue as before.
Testing Instructions for Keyboard
Issues
postIdis given as search parameter)