Block Comments: Make comment thread focusable and improve keyboard navigation #71883
Conversation
This file contains hidden or 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
|
Size Change: +280 B (+0.01%) Total Size: 1.95 MB
ℹ️ View Unchanged
|
t-hamano
added
[Type] Enhancement
t-hamano
force-pushed
the
block-comment-focusable-thread
branch
from
cbf766f to
e8faa2f
Compare
|
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. |
3 tasks
t-hamano
changed the title
Mamaduka
reviewed
Sep 25, 2025
Mamaduka
reviewed
Sep 25, 2025
t-hamano
commented
Sep 25, 2025
|
Flaky tests detected in d200041. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18099522446
|
joedolson
requested changes
Sep 25, 2025
Contributor
joedolson
left a comment
joedolson
left a comment
There was a problem hiding this comment.
Thanks for working on this! Here are a few changes I think would improve it:
- Map
esckey to move focus back to the thread on an open thread. For long comment threads, navigating through an open thread might take a long time, so it should be closable. - Resolve button on an open thread should close & focus the thread.
- Agree that reading the whole list item is too much. I think it needs to react more like a control. An
aria-labellike "[first 5 words of first visible comment]" would help. Usearia-expandedto indicate whether a comment is opened or closed. Enter when focused on an opened thread should also close the thread; addingescas a close option is something that I think would be better for usability, as well.
Contributor
Author
I tried a different aria-label. When a thread is focused, a screen reader should announce the following text. I think this is consistent with #71843. |
Contributor
|
My reason for including the comment text was related to WCAG 2.5.3: Label in Name, which stipulates that the visible text on a control is in the accessible name for that control. This is enable to support speech activation, and allow users to activate a control using the text they can see, rather than needing to position a cursor using speech or use grid selection controls. It's not 100% clear whether 2.5.3 applies in this case; it's applicable to user interface components, and the WCAG definition of a user interface control is "a part of the content that is perceived by users as a single control for a distinct function". I'm not sure that a user would perceive this panel that way. The question here becomes "how would a speech activation user activate this panel." Practically speaking, I'm not sure voice activation would perceive this panel as something that's command clickable - I couldn't get it do it, using the aria-label in this PR, so I suspect that ultimately won't matter here. I was able to do it pretty easily using the mouse grid, however. That's not efficient, but works. I do think that it should be possible to activate the panel using voice control without using the mouse grid, however. The logical thing to me is that if a user uses the 'Actions' panel to Edit a comment, that should activate the panel. |
Member
|
I tested this on a Mac using VoiceOver and navigating around the comments by keyboard. Everything worked much better with this PR vs trunk, nice work! |
Contributor
Author
|
Ok, so I applied the comment excerpt to aria-label in 6b99421. I had to create our own utility function, but it should work properly in locales other than English. For example, if the comment is Is there any other information that should be provided? |
Contributor
|
@t-hamano It might be good to include the object name in the |
Contributor
Author
Updated in 550e582 |
Member
|
Are there any other blockers here? It would be nice to land this refactoring so that we can improve |
Contributor
Author
|
We can improve |
Mamaduka
approved these changes
Sep 29, 2025
Member
Mamaduka
left a comment
Mamaduka
left a comment
There was a problem hiding this comment.
Yes, this looks good from a code perspective.
I just wanted to ensure that accessibility improvements are also beneficial from @joedolson's perspective.
4 tasks
adamsilverstein
added a commit
to adamsilverstein/gutenberg
that referenced
this pull request
Sep 29, 2025
…vigation (WordPress#71883) * Block Comments: Make commment thread focusable * Restore thread id attirbute * Remove event.stopPropagation * Close focused thread by enter key * Close focused thread by esc key * Focus current thread when the thread is resolved * Add thread comment as aria-label * Add aria-label * Fix e2e test * Improve aria-label * Use comment excerpt for aria-label * Add "Comment: " prefix to comment aria label Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: joedolson <joedolson@git.wordpress.org> Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Member
|
@t-hamano, should we create an issue for e2e test coverage of this feature? |
Contributor
Author
|
Here it is: #71962 |
Mamaduka
reviewed
Sep 30, 2025
| variant="tertiary" | ||
| className="editor-collab-sidebar-panel__more-reply-button" | ||
| onClick={ () => setFocusThread( thread.id ) } | ||
| onClick={ () => setSelectedThread( thread.id ) } |
Member
There was a problem hiding this comment.
While working on #71969, I noticed that clicking on "more replies" causes a focus loss.
Contributor
Author
There was a problem hiding this comment.
Good catch, I just wrote a PR about focus management, so I'll fix it there 🙂
t-hamano
added
[Feature] Notes
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? How?
This PR makes the comment thread container focusable, improving the keyboard navigation experience.
Why?
The Block Comments has a lot of a11y issues as reported in #71249, and there have also been many reports of issues with focus loss.
In some cases, we may want to move focus to the comment thread itself. I believe making the comment thread itself focusable should help solve the focus loss issue, especially.
How?
This PR has the following enhancements:
tabIndex={0}.role="listitem"to the threads and wrap them in an element with therole="list". This allows a screen reader to announce which thread in the list has focus.aria-expandedattribute to the thread.Enteron a focused thread expands or collapses that thread.Escin an opened thread closes the thread, and the thread gets focusedTesting Instructions
Testing Instructions for Keyboard
Screenshots or screencast
fd57c4905582e9a91957b59a604f4ba7.1.mp4