Nav block: remove list markup #31827
Conversation
|
Size Change: -16.8 kB (-1%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
ellatrix
force-pushed
the
try/nav-block-remove-list
branch
from
76dfbe5 to
2531718
Compare
|
I understand all your points. From an accessibility viewpoint though are we confident that removing the affordances provided by lists wont provide a poorer UX? For example I believe the total items in lists are announced where as that wouldnt work for more generic markup. What if the user has a giant navigation? Without that affordance a user of assistive tech might dive into a series of items without any idea of how deep the rabbit hole goes. Not saying your points arent valid but just checking weve considered all a11y considerations. My knowledge may be out of date but Id rather be safe than sorry. |
|
This PR would solve #29036 and open many possibilities. On the other hand, regarding navigation accessibility, as far as I know, it was discussed in #24364 (comment) determining links indeed need to be wrapped with a |
We should be able to add a label for that. |
There was a problem hiding this comment.
Thanks @ellatrix for making us rethink our existing assumptions! ✨ If I read the threads correctly, this assumption might sum up to we must use a ul for accessible navigation links.
On the technical side of things, I do prefer keeping things simple so this approach has a lot of appeal on that front. All types of blocks that we insert into navigation block (including spacer, social links, etc) will now validate as correct html5 markup which is also very good.
I briefly tested this in VoiceOver and one of the main differences is that UL will list how many links there are. We might be able to work around that by using a good combination of role="navigation" and a good generated aria-label to help make up for this.
If anyone has access to the JAWS screen reader I think it'd be good to help spot any other differences here.
The blocking factor for this change I think is whether or not the community here thinks this will work for screen readers. I'd be interested in finding folks who use screen readers daily to provide some feedback, but barring that we might need to bring this up for discussion with a wider group.
So TL;DR, this is very elegant and will 💯 work technically, but we should get feedback on if this is acceptable on the a11y side of things.
| @@ -146,7 +146,7 @@ function render_block_core_navigation_link( $attributes, $content, $block ) { | |||
| 'style' => $style_attribute, | |||
| ) | |||
| ); | |||
| $html = '<li ' . $wrapper_attributes . '>' . | |||
| $html = '<div ' . $wrapper_attributes . '>' . | |||
There was a problem hiding this comment.
Following this line of thinking, I think it should be possible to drop this div wrapper too here in navigation-link and home-link.
This change would need to rework the css for how submenus work, of course.
This is non blocking since changes here work well and now markup validates properly, but I think it'd be worth exploring.
| Example before | Example after |
|---|---|
 |
 |
There was a problem hiding this comment.
Definitely, but I didn't want to touch the styling as part of this PR. :)
There was a problem hiding this comment.
Yeah, this could be done as a follow up too if we choose this approach 👍
|
In the editor, there is still an Disclaimer: I am not a screen reader user, the only time I use a screen reader is for testing. I don't think the usability of the block falls on if a list element is used or not. I found both versions, trunk and this PR, very difficult to use. Where I, as a beginner screen reader user try to move to the next item using tab, I leave the block and end up on the post settings sidebar. Using arrow keys: up and down -nothing happens. Left or right: the text of the current menu item is read out one letter at the time. Even though all the information below is read out, I am still not able to tell that all menu items have sub menus except for the last item: With list: With div: |
This is a key example. Imagine if the list had 100 items and it didn't announce the number of items? It would provide a terrible user experience. We can use labels but 9 times out of 10 the most accessible route is to use plain old HTML. I am of course open to the fact that this might be the 1 time HTML doesn't cut the mustard but IMHO we should definitely be bringing this decision up with the a11y teams before we proceed. Update: I just read @gwwar's response in which she puts this far more elegantly. I agree with Kerry. |
|
Howdy @ellatrix ! I believe @tellthemachines 's #30430 also tries to free the navigation block from the So, while removing the default UL > LI markup for the block is good in my opinion, at the same time we need to enable a way to place in the block simple "link list" based navigation. The problem we're facing is one of UX complexity. We don't know what the initial intent of the user is when a navigation block is used, and how the complexity of the block will evolve as the user adds content. The reason we have a list based markup is only because we did not start with the premise of the navigation as an open canvas, but more on the pattern of the classic WP menus, which are nested lists of links. For nested lists of links ULs are great. Once one starts to imagine the navigation block as a canvas for any kind of menu (including marketing heavy mega menus) the problems of learning curve, initial action etc. immediately pop up. So this is where adding a passthrough block shines as an idea. We make a block like the one in #30430 and the user never knows it's there. Neither of these solutions (this PR, #30430, a passthrough block, rendering markup on front end only) really solve the problem that by allowing various blocks in menus we're making it a mini editor which is basically a special case of the group block. I think the navigation block is now at a point where the problems it has are not so much about how to implement it but about what do we want it to achieve? I mean this example from @jasmussen does it really need to be handled by a single block? |
|
I'm actually starting to think that lists do fulfil a purpose here. While we could add labels etc., it does nicely group links together as a single entity next to spacers and search blocks, which you probably don't want to mix with links in the menu. For example, it's probably not great if a user could move a search block between links, they should rather only be able to move it before and after the ... list. So with that thought, I'm closing my PR. |
|
Ok, apparent it should be possible to move non-links into a list of links, such as search or the site title, so in that case I'm again for removing the list markup. I'll add a label for the item count shortly. |
|
Just to confirm, I should be testing the Navigation block after you push the change to add count label? Thanks. |
|
@alexstine Yes, I'll add a count label in a bit. I can ping you once I've added it. 🙂 |
|
Will there still be a way to define subgroups within the navigation if lists are not used? That's traditionally been done via nested lists, but I'm not clear how this is going to happen without lists. Any flow content is permitted within a I'm having a little trouble imagining exactly what the end result for this block will be for either the editor or the user, though, so it's hard to comment until after testing. Even then, it sounds like it'll take some serious testing. |
I'm not clear on this point, could you link to sources/discussion? A big part of the a11y concern was isolating links from non-links, so I wouldn't expect it to be possible to move a non-link inside a list of links. |
Andrei summed it up nicely. Semantic accessible markup is a given. We also need to retain the current user experience — additional containers surfaced to the user are not going to be intuitive. |
|
Should we close this one now the markup change has been addressed elsewhere? |
|
Sure :) |
Description
We're shooting ourselves in the foot by making the navigation block markup overly complex. Using lists in navigation elements is not even necessary. It's not because we've always been doing it that way that it is the correct way. I believe the the navigation links will be announced just fine, and if we need to be more descriptive somewhere, we can always add labels (list count and order comes to mind).
Rule of thumb: if you have to completely remove list styling, you should probably not be using lists.
For example, it does make sense to use list if it's a big table of contents of the site, but that's not how this menu will be used I believe.
It's not too late to change since we haven't released the navigation block yet.
While it doesn't really matter if a navigation section uses lists or not, it does matter for us because we're making the navigation overly complex by having to manage the lists. Without lists, you can potentially embed any block in the navigation block without it needing a list item wrapper.
I just removed the list markup here, but otherwise left the structure intact so I don't need to change CSS. However, with the
ulelement gone, the replacementdivshould probably be removed in a follow-up.How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).