Add aria label to Nav block on front of site #39161
Conversation
getdave
added
[Focus] Accessibility (a11y)
|
Size Change: 0 B Total Size: 1.16 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
@getdave Thanks for the PR.
I think for me, I would be concerned that the aria-label appearing on the page multiple times could actually do more harm than good. Especially if it sends users mixed messages. E.g. If you named your menu "Main Navigation" and put the block in the header/footer, this is now confusing as usually this wording would only appear at the top of the site.
You are seeing my mistrust of users come out on this. :(
There was a problem hiding this comment.
I agree with @alexstine that we wanna be very careful with these. They should be named, yes, but the name should definitely not be duplicated. E. g. if there are several nav blocks, each should have a distinct name.
There was a problem hiding this comment.
It seems to me like there's a fair argument that if the same navigation menu appears multiple times, it should have the same name. But if not, it shouldn't have a fully unique name, and only be differentiated by order in the DOM, e.g. "Main Navigation (1 of 2)", "Main Navigation (2 of 2)". Having fully unique names would imply that the menus have different content.
Is it feasible to add menu counts to differentiate if a menu is reused?
|
Thanks for all the feedback folks. That's super helpful. So what I'm hearing is that:
Let me know if that is correct. I'll have to look into how we can do this but it could be possible. Thanks again all 🙇♂️ |
|
We can count on render or store the association between a navigation block and it's navigation CPT in any block template. Given that this association does not change often I think counting on render is not a good choice. Given we already store the navigation data in the DB whenever a navigation block is associated with a CPT we can send that to the API and have all the navigation CPTs have a prop that holds a count of how many navigation blocks are associated. My proposal is not fancy, I know. The other way, @getdave @alexstine is to combine the name of the navigation with the name of the parent template part. However this does not block a user to use the same navigation in the same template part multiple times. But for that we could add a warning in the block inspector "You are using this navigation in this template part already". <- I think this option is better than storing in the DB. |
|
@draganescu I think we should just store it in the DB or run a .length on render. We're talking about super simple information, right? |
|
Following the feedback, I searched the codebase for any prior art on something like this. I found something similar in the Template Part block: Therefore, I've opted to keep things simple and added a commit which implements a very basic method of ensuring a unique ID based on remembering the names of previously rendered menus and appending an ID if the menu has already been rendered. Eg: I'd be cautious about adding DB queries here as the block would need to run them on render. That said I'm open to suggestions or alternative PRs if folk believe that would be a better option. |
getdave
force-pushed
the
add/aria-label-to-nav-block
branch
from
281aa5c
to
239686c
Compare
|
@getdave Is there any way I could get a test site for this? The navigation block is still pretty inaccessible for me to configure. |
|
@alexstine isn't it more clear to have the label be Main Navigation Footer compared to Main Navigation 2? |
@draganescu I see an argument for looking at this. Indeed, in the future the Nav block may be more aware of its location within a given template. However, I think we'll need to jump through a few more hoops to get to that solution. This PR does solve the "must be unique" problem. I wonder how we feel about merging and iterating? Is this better than what we have now?
@draganescu Are you aware of any APIs that might allow us to achieve this? Also noting we'd still need the logic to de-duplicate within a given Template Part. |
@alexstine I'm really sorry but I don't have a throw away instance available where I can have this PR running. Are you able to
As all of the links are to external URLs it should create a configured Nav block for you. I hope that helps? |
|
@getdave When I test your built branch with the code you provided, the aria-label is empty. Thoughts? Thanks. |
Hi @alexstine. I'm really sorry this is proving tricky to test. Let me see what I can do to help. I appreciate your patience. I suspect what is happening is that the Navigation Menu Post is not being created from the inner blocks which have been pasted into the Code View. I think once you exit Code View you will need to reselect the Nav block in order for the items to be transformed into a Here's a video of me walking through the process. I hope that is able to help? Screen.Capture.on.2022-03-09.at.06-07-38.movI would suggest these are the new steps:
This whole problem tells me we need to show a notice when the Navigation menu is created from inner blocks. That way it would be obvious when the operation had succeeded. As things stand there is no visual or auditory feedback that this has happened. |
|
@getdave This is almost good for me. What I would like to see is every navigation output having a label. E.g.
Does this make sense? Probably just needs some conditional tweaking. In the future, I would like to see authors receive a warning when the same navigation block is added to the content with the same name. I believe we can teach accessibility through a means of guiding. However, this makes our problem better for now which is why I am in favor of this PR. 👍 Thanks. |
I took a look at this and at the moment I can't see a way to achieve it. When we render a given block X we don't know about any blocks that will be rendered afterwards. Therefore when it comes to outputting the block can choose to conditionally add a numeric ID based on other blocks. Scenario:
What we can't do is output At the moment I'm a bit stumped. Will ask around for any advice/input. |
|
From a user comprehension standpoint, I think it would better if the added count started at 2. Having the first appearance unnumbered is fine, but the 2nd should be numbered based on actual count. |
|
I must be honest. I am not sure excluding "1" is a good idea especially if the first item would be "2". Can you imagine? A user hears "My Navigation 2" and they are going to start thinking where "My Navigation 1" is. Honestly, I am not sure this is really going to help a lot at the end of the day. It would be great to have other screen reader users feedback in here. Maybe it is worth waiting for some type of accessibility check for this block. After all, sometimes the best things are worth waiting for. 👍 Just my thoughts... Thanks. |
|
@alexstine I'm not sure you understood what I was suggesting correctly. Based on @getdave's comment, the first found block doesn't know whether there will be additional, so it can't be numbered. Following on that, the first block would be 'Main', the second would be 'Main 2', because it's the second found block. I think this makes a lot more sense than the first block being "Main", and the second being "Main 1". To me, that would be very confusing. |
Yes, I completely agree. I still struggle with though if excluding the "1" will cause any confusion. You don't think this could play an issue at all? |
|
@alexstine I don't know about "no issue". I do think that a count of one is commonly represented by existence, without an actual number present. I think that the scenario where only one menu is present including a count would be worse. The presence of "Menu 1" strongly implies that there are more instances of this menu. It would be ideal to have the count present on all instances of a menu if there was more than one, but in my opinion, omitting the 1 is not a deal breaker, and is less of a problem than including a count unnecessarily. |
|
Sounds fair. @getdave Can you change the code so the second menu on the page gets number "2" to start? First menu will have no number, second will have "2", third will have "3", and so on. After giving this some thought, I think it is an improvement and we should get it in. |
I pushed a commit to do this. |
|
Thanks all. e2e test failure seems unrelated. Will re-run and look to merge. Tests still failing. Have rebased to see if that helps. They seem completely unrelated to this PR. |
Co-authored-by: Anton Vlasenko <43744263+anton-vlasenko@users.noreply.github.com>
getdave
force-pushed
the
add/aria-label-to-nav-block
branch
from
b8513f7
to
2a43c00
Compare
|
Looks like e2e tests have caught an unrelated bug. I'm going to force merge this one. |
Description
Currently when rendered on the front of the site the Navigation block has no accessible label to identify it. We have a
Menu namefield on the block which is saved to thepost_titlefield of the underlyingwp_navigationpost.This PR utilises the
Menu nameas thearia-labelattribute on the main<nav>tag on the front of the site (only).I added an e2e test to provide coverage.
Closes #24369.
Limitations
I'm aware of the following:
Advancedsection).wp_navigation) across multiple Navigation block's on the same page then you will have duplicatearia-labels. This is probably an edge case. We could add an additionalaria-label` field to the block if necessary but I'm unclear on how we could dedupe names across Nav blocks on a page.Testing Instructions
<nav>tag for presence ofaria-labelattribute.Advanced->Menu name.<nav>tag for presence of modifiedaria-labelattribute.Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).