Table of Contents: Add aria label to the nav element #71586
Merged
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
|
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. |
|
Size Change: -23 B (0%) Total Size: 1.95 MB
ℹ️ View Unchanged
|
6 tasks
mikachan
approved these changes
Sep 10, 2025
Member
mikachan
left a comment
There was a problem hiding this comment.
This is working well for me and code changes look good! Looks like the test fixtures just need regenerating to get the tests to pass.
t-hamano
reviewed
Sep 10, 2025
Contributor
t-hamano
left a comment
There was a problem hiding this comment.
I think we don't need to update edit.js or save.js files, as the aria-label support should handle everything by default. If it's not working as intended, it may be an issue with the aria-label support itself.
|
Flaky tests detected in 672461d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17996636298
|
Contributor
Author
It wasn't working for me, I'll test again but the frontend wasn't showing it at all |
Contributor
Can you show me the HTML you used to test it? |
Contributor
Author
do you mean the markup of the block? |
Contributor
Yes.
How did you test that it doesn't work? |
Contributor
Author
|
Contributor
|
I see. You want to set the If you want to set a default value, I think there are two approaches:
@Mamaduka Do you know of any other optimal ways to set a default value for aria-label block support? |
Member
We should avoid this as much as possible. Using |
# Conflicts: # packages/block-library/src/table-of-contents/save.js
Member
|
Thanks both!
I've tried this out in the latest commits, and hopefully, this is also a good start for switching this block over to server-side rendering. If folks think this is a good way forward I'll update the PR title and description, too. |
Contributor
|
@mikachan, I don't think this much code is necessary. The current ToC block should statically save the complete HTML via the save function, so dynamic rendering is not necessary. Whether or not to rig it dynamically is something that needs to be considered separately. It should be simple, just apply a default For example, see the code for adding a block class name to a heading block on the server side. Even for a ToC block, the code size should be about this much. |
Member
Mamaduka
reviewed
Sep 24, 2025
Contributor
|
The changes look great, but is there any reason why block fixtures have been updated or removed? |
Member
No! Thanks @t-hamano, this would've been left over from me messing around with the way the list was being rendered, I think 😅 I was seeing a bunch of errors related to them when I was playing with the server-side rendering. I've reverted all the test fixture changes now. |
t-hamano
approved these changes
Sep 25, 2025
Contributor
t-hamano
left a comment
There was a problem hiding this comment.
LGTM! I confirmed that the custom label also works as expected by using the following HTML:
<!-- wp:table-of-contents {"headings":[{"content":"Heading","level":2,"link":"http://localhost:8888/?p=486#heading"}],"ariaLabel":"My Label"} -->
<nav aria-label="My Label" class="wp-block-table-of-contents">
<ol>
<li><a class="wp-block-table-of-contents__entry" href="http://localhost:8888/?p=486#heading">Heading</a></li>
</ol>
</nav>
<!-- /wp:table-of-contents -->
<!-- wp:heading -->
<h2 class="wp-block-heading" id="heading">Heading</h2>
<!-- /wp:heading -->Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
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?
Part of #42229
Adds an aria-label attribute to the wrapping nav element of the block
Why?
This is helpful for accesibility
How?
Using the ariaLabel support and adding a default value for it (otherwise it doesn't show any)
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast