plone
A11y: Wrap ToC Block within nav tag
Fixes #6082
📚 Documentation preview 📚: https://volto--6084.org.readthedocs.build/
Deploy Preview for plone-components canceled.
| Name | Link |
|---|---|
| Latest commit | 5e78fa68427e58ad00f3eb016eb8ce2b902881cc |
| Latest deploy log | https://app.netlify.com/sites/plone-components/deploys/6720995de2dda20008ed83c6 |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Thanks! @ichim-david for the in-depth explanation, from the contents you shared , I agree aria-labels can mislead , but if we want the screen reader to read as '....... ToC link' so the users know it is a ToC link , can we use aria but in a more descriptive way eg- ' Table linearisation a Table of Content link' something similiar in this line.
@Tishasoumya-02 aria-labelledby gives a better solution that aria-label but even it suffers from replacing the links title from the "Link Rotary" on VoiceOver app. I tried also aria-describedby and it says the "Table of contents" when you are tabbing around but it doesn't say it when using the Rotary system. I don't think there is a solution for your problem that is without trade-offs and I have yet to see any accessibility firms or websites provide extra info concerning the table of contents links. Have a look at these guys that make the axe linter that is used by everyone, even they don't have anything close to what you are proposing: https://www.deque.com/web-accessibility-beginners-guide/#what-is-a11y
Please investigate some more on this issue and if you find something that is worth investigating do share it. I also have one website that I want to bring to your attention when you want to use a certain aria role have a look also at the technologies support and try something that has as broad support as possible: https://a11ysupport.io/tech/aria/aria-describedby_attribute
@ichim-david I think there is room for two IMO valid opinions here:
-
One could argue that a screen reader will read "table of contents" (the title of the toc block) following a list of headlines. This will make it clear to the reader that the links are internal anchor links and not links to another page. If you have this view, we should leave things as they are. Maybe slightly improve the HTML structure.
-
One could also argue that it is essential for a "reader" to differentiate between links to another page and links on the same page (anchors). In that case, you want to be explicit and use an additional aria-label that tells the reader that this particular link is an anchor link (here is an article where someone tries this https://amberwilson.co.uk/blog/are-your-anchor-links-accessible/). If you have this view, I think this PR is exactly what you want (and still minimal compared to other options).
I don't have a strong opinion here. I wasn't able to find any good sources on this topic. However, I am not sure if websites that write about a11y are a "good enough" source for best practices and decisions.
Maybe @polyester or our friends from redturtle (@pnicolli @cekk et al) have an opinion on the topic.
@ichim-david I think there is room for two IMO valid opinions here:
1. One could argue that a screen reader will read "table of contents" (the title of the toc block) following a list of headlines. This will make it clear to the reader that the links are internal anchor links and not links to another page. If you have this view, we should leave things as they are. Maybe slightly improve the HTML structure.
I am in favor of this option. You have yet to convince me that option 2 is viable given that even the author who investigated this technique hasn't used this technique for its own table of contents on the page itself: https://amberwilson.co.uk/blog/are-your-anchor-links-accessible/ My vote is not to have aria-label on the links, aria-labelledby behaves better both when tabbing around and when using the rotary but I still don't like that it shows in the rotary duplication "Table of Content link" instead of the actual link text as such I wouldn't add any aria tags on the links.
All of the a11y blogs that I have read have not employed this technique and although yes, just like blogs about performance can suffer from performance issues, if this was an established pattern I think there would be more information about it and used more broadly. If there are better techniques discovered then we can test them and see if they have more pros than cons. Having a discussion and trying things is beneficial so this time isn't wasted but at the same time we should be careful with what aria tags we introduce as the rule is still that no aria rules are better than wrong aria rules.
I would go with option 1 as well for now, but improve the HTML structure. Currently, the ToC block seems to produce a
<div class="table-of-contents default">
<ul> ... </ul>
</div>
which would already be improved by having the div replaced as a
<nav aria-labelledby="table of contents" class="table-of-contents default">
instead. That's a fairly minimal, gradual approach.
I'd agree with using Option 1 here. Adding the navigation role as suggested above with a descriptive label along with the heading of the ToC provides enough context to the user about what the links will do. You could use an aria-label rather than labelledby on the <nav> to add more context about the ToC, but I wouldn't say this is essential and should be researched on a project-by-project basis. e.g:
<nav aria-label="Contents on this page">
<h2>Contents</h2>
<ul> ... </ul>
</nav>
GOV UK Publishing Components - Contents list is a good example of a well researched and accessible ToC
@Tishasoumya-02 @ichim-david @polyester @JeffersonBledsoe thanks for your valuable input folks! Then let's go with option 1 and improve the HTML structure of the toc block. @sneridagh I guess this is considered to be a breaking change. Can this go into Volto 18? We are still in alpha, right?
@tisto Yeah, no problem by being breaking. We will have to write the breaking notice and the upgrade guide update.
@Tishasoumya-02 could you take care in the next days?
@sneridagh @Tishasoumya-02 I don't think we can use any of the changes from this pull request. it would make sense in my opinion to open a new pull request with changes to the Table of contents block.
It's still not clear to me though what is going to be proposed exactly, if we should have a proposal and after feedback it's implemented or we give feedback after whatever you propose to change.
I say this because I've seen the comment from @JeffersonBledsoe https://github.com/plone/volto/pull/6084#issuecomment-2165251101 where he gives the example of how Gov Uk does it and then the simple nav wrap mentioned by @polyester.
I think if we have a proposal and we agree on the markup it would mean fewer changes in the pull request afterward no? It could be a simple gist or a code paste of a simple toc html output.
Reading through how Gov uk implements the content of table shared by @JeffersonBledsoe , I made some changes in the existing View.jsx of ToC Block (gist of changes below) , should I implement this in this PR for all to test?
<nav className= 'table of contents default' aria-label= 'Table of Contents' >
<div className= 'table of contents main default' >
<h2>Content</h2>
<ul> <li><li> </ul>
</div>
</nav>
@ichim-david @sneridagh
Not really, we can remove it. As for accessibility it does not serve any purpose
Reading through how Gov uk implements the content of table shared by @JeffersonBledsoe , I made some changes in the existing View.jsx of ToC Block (gist of changes below) , should I implement this in this PR for all to test?
<nav className= 'table of contents default' aria-label= 'Table of Contents' > <div className= 'table of contents main default' > <h2>Content</h2> <ul> <li><li> </ul> </div> </nav>@ichim-david @sneridagh
@Tishasoumya-02 I suggest the following:
- https://github.com/plone/volto/blob/main/packages/volto/src/components/manage/Blocks/ToC/View.jsx#L168 Change from div to nav and add aria-label Table of contents translated
- Remove data.title condition from here https://github.com/plone/volto/blob/main/packages/volto/src/components/manage/Blocks/ToC/variations/DefaultTocRenderer.jsx#L48 We want to get the fallback "Table of contents" added in case no title is added. If someone doesn't want a header at all they can use the Hide title and remove that altogether. Right now the condition ensures that the fallback will never show up. This way we don't have a new nav added and the div that was added from the default View but that div simply changes to be a nav plus aria label. The output will be thus:
<nav className= 'table of contents main default' aria-label= 'Table of Contents' >
<h2>Table of Content</h2>
<ul> <li><li> </ul>
</nav>
This way the user in need of a screen reader can find the section either from the Landmark rotary which will find "Table of contents navigation" or will use the Headings rotary and know that what follows is a table of contents.
One additional thing we also need to consider is the potential for there to be multiple Table of Contents blocks on a page. We don't want every ToC Block to have a nav of Table of Contents.
One additional thing we also need to consider is the potential for there to be multiple Table of Contents blocks on a page. We don't want every ToC Block to have a nav of
Table of Contents.
@JeffersonBledsoe that's a good point, perhaps the aria-label could contain the title. If you don't enter anything you get the fallback for Table of contents which is with Formatted Message. If you do enter something it could take that value. I don't know how often we would have a requirement where we have more than one table of contents present though. If you have another technique in mind I'm all ears :)
I don't know how often we would have a requirement where we have more than one table of contents present though.
In some documentation projects, it is not an uncommon use case. It allows to break up a wall of text with headings and explanatory text for greater legibility and context. Example of multiple ToCs:
- https://docs.pylonsproject.org/projects/pyramid/en/latest/#tutorials
- https://docs.pylonsproject.org/projects/pyramid/en/latest/#narrative-documentation
- https://docs.pylonsproject.org/projects/pyramid/en/latest/#api-documentation
...and so on.
One additional thing we also need to consider is the potential for there to be multiple Table of Contents blocks on a page. We don't want every ToC Block to have a nav of
Table of Contents.
If we could use a different landmark like section or main instead of nav , would that be an option?
If we could use a different landmark like section or main instead of nav , would that be an option?
@Tishasoumya-02 <nav> Is the right semantic here as the element will be used to navigate around the page. It will need a different aria-label for each Table of Contents as this is used as the 'name' of the element (what is read by Assistive Technology)
No, it should be a nav as that's semantically correct - it navigates. It's fine to have multiple navs on a page, it would be wrong semantically to have multiple main
Yet to test it, but looks much better from the code, nice work! This would need to be marked as breaking due to the markup update, right?
Additional gut feeling: having the aria-label as Table Of Content {TITLE_OF_BLOCK} might be excessive/ provide unnatural language depending on what you use for the title, but I'd just be guessing at what users of assistive technology would want to be conveyed. I would continue with the code you've already implemented for the label so as not to hold up this a11y fix.
cc: @ichim-david
Any change in the HTML structure is a breaking change. Projects could have CSS or customizations based on the HTML structure. Therefore it would break for them.
Yet to test it, but looks much better from the code, nice work! This would need to be marked as breaking due to the markup update, right?
Additional gut feeling: having the aria-label as
Table Of Content {TITLE_OF_BLOCK}might be excessive/ provide unnatural language depending on what you use for the title, but I'd just be guessing at what users of assistive technology would want to be conveyed. I would continue with the code you've already implemented for the label so as not to hold up this a11y fix.cc: @ichim-david
@Tishasoumya-02 The logic for aria-label with Table of contents isn't ok because you can't assume that the editor won't add any of the table of contents words and you can run into a duplication as seen in my screenshot
I think if we take the example of @polyester where the navigation is described by something it's probably that h2 https://github.com/plone/volto/pull/6084#issuecomment-2160572359
As such it could take the aria-label with the value of the title. If you write "Contents" it will read "Contents navigation". If you write "table of contents" it will say "table of contents navigation" and so on. This makes it in my opinion the least surprising value since it takes the heading value plus the navigation landmark semantic from the nav tag.
@davisagli @pnicolli I wanted todo some more changes such as making actual use of the data.title fallback which was translated saying "Table of contents" by setting that value as default to the title. That meant that you add the block and you get that title which you can change. If you wanted not to display it you had the hide title option.
Upon checking https://light-theme.kitconcept.io/ and seeing that you only have the title option empty by default
I removed that change.
The only front-facing changes are wrapping the table of contents with a nav tag instead of a div.
Given the fact that the styling references the class in order to style it, it won't affect your projects. If a title is added then that title will be used as part of the navigation otherwise it will simply say navigation. Depending on your needs we could provide the "table of contents" aria-label if no title is added or not so i'm open to suggestions on what you are ok with.
@ichim-david I don't have a strong opinion about whether or not we should have a default title (my gut feeling is to leave it as is with no default), or about whether or not we should put "table of contents" in the aria-label when there's no title (my gut feeling is to leave it out, if screen readers will indicate the nav element in some way anyway).
@Tishasoumya-02 @ichim-david Let's wrap things for the final release, and I think this is a good one to be included, so let's move this forward, the change is sound and it already improves things. We just need blessing from someone from the a11y team.
As I see, it's breaking and therefore needs proper news item .breaking and an entry in the upgrade guide.
@ichim-david I don't have a strong opinion about whether or not we should have a default title (my gut feeling is to leave it as is with no default), or about whether or not we should put "table of contents" in the aria-label when there's no title (my gut feeling is to leave it out, if screen readers will indicate the nav element in some way anyway).
I'd agree with @davisagli here; minimal intrusion is better so leave as is with no default.
And agreed with @sneridagh ; it's an improvement, so let's wrap up and get it in.