Size Change: +5 B (0%)

Total Size: 2.45 MB

_{compressed-size-action}

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: joedolson <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: jeffpaul <[email protected]>
Co-authored-by: adamsilverstein <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Flaky tests detected in ee5de6a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19054815970
📝 Reported issues:

• [Flaky Test] ensures List View global shortcut works properly #51533 in /test/e2e/specs/editor/various/list-view.spec.js

What are the steps for testing it? I can see there's a tree and tree items in the DOM, but I'm not sure if I'm testing the expand/collapse correctly? Is it this?

If that's the case, then I feel like the tree role will probably end up overcomplicating things. In the issue, I was perhaps a bit thrown by the discussion as there was no screenshot of the UI.

For a start, I think there are expectations from users about how keyboard navigation should work when they enter a tree (https://w3c.github.io/aria/#tree). I think this role is really supposed to mimic native UI widgets, like file system trees, so a user might expect arrow key navigation, where up/down would navigate between messages, and left/right might expand/collapse branches of the tree. Plus you have to manage focus like a widget (active descendent or roving tab index).

Having tested the PR, it looks like each conversation is much simpler than a tree, and is a stack of messages with no branching. One aspect that I think it going to be a bit challenging for any implementation is that the last message ends up orphaned and not part of the collapsed messages. Here's what I'd consider looking into if I were implementing it, but I'm not an a11y expert by any stretch:

• The replies could be in a list (<li> elements, and I guess it's an ordered list so perhaps <ol>?). When a user tabs in it might be announced something like 'Replies, 5 items'. I'd keep the first message outside of and separate to that list considering it 'houses' the entire thread and provides context to the replies.
• The collapsing/expanding could be implemented more like an accordion. I'm not familiar with implementation details for those, but maybe it's easier than a tree. '2 more replies' would be the accordion text. Pretty simple
• If you need to keep the last message orphaned when collapsed, then I guess that's doable, just don't make it part of the list and just give it clear labelling like "most recent reply".

edit: Here's some 'diagrams' of what I mean:

Collapsed List

• Main comment
  • Replies Accordion (collapsed)
  • Most recent reply

Expanded List

• Main comment
  • Replies Accordion (expanded)
    • Replies List
      • Reply one
      • Reply two
      • Reply three
  • Add new reply (one part I'm unsure of is whether this should be in the replies list or not 🤔 )

Something else you may have to contend with is that there's no affordance for collapsing the list of messages, you can expand, but it looks like once it is expanded, the only way to collapse is to select a different block.

@talldan Thanks for the detailed feedback!

What are the steps for testing it? I can see there's a tree and tree items in the DOM, but I'm not sure if I'm testing the expand/collapse correctly? Is it this?

Sorry for the insufficient explanation. If you would like to test this using a keyboard, focus on a note that has the treeitem role and press the Enter key. The note should then open or close. This is the main reason why I changed the roles from list and listitem to tree and treegrid.

For a start, I think there are expectations from users about how keyboard navigation should work when they enter a tree (https://w3c.github.io/aria/#tree). I think this role is really supposed to mimic native UI widgets, like file system trees, so a user might expect arrow key navigation, where up/down would navigate between messages, and left/right might expand/collapse branches of the tree. Plus you have to manage focus like a widget (active descendent or roving tab index).

I agree. The Notes sidebar should probably also be controllable with the arrow keys, similar to the List View, but that would be a future enhancement.

One aspect that I think it going to be a bit challenging for any implementation is that the last message ends up orphaned and not part of the collapsed messages. Here's what I'd consider looking into if I were implementing it, but I'm not an a11y expert by any stretch:

Yes, I think this is the most difficult point. Therefore, for now, I have tried an approach that does not group the replies together.

Something else you may have to contend with is that there's no affordance for collapsing the list of messages, you can expand, but it looks like once it is expanded, the only way to collapse is to select a different block.

Pressing the escape key while in the note allows you to ccollapse the note.

The Beta3 release is coming soon, and to allow for further discussion on the correct approach, I'm punting this PR to 6.9.1. If it doesn't make it into the minor release either, I'll punt it to 7.0.

Based on #72594 (comment), I added an aria-label to the element with the tree role. It's Unresolved notes if the sidebar is floating, and All notes otherwise. The complete accessibility tree can be found in the "How?" section of #72650 (comment).

cc @joedolson

Thank you @t-hamano for updating this so quickly from @joedolson's feedback!

@joedolson Thanks for the review! Let's backport this PR to 6.9.

I just cherry-picked this PR to the wp/6.9 branch to get it included in the next release: 329d05c