Navigation Block: Support non-li inner blocks

[priethor]

What problem does this address?

Currently, the Navigation block assumes all inner blocks are <li> elements, such as the navigation-link block, thus wrapping inner blocks within a <ul> element. However, other available blocks used for advanced navigations are not <li> elements, such as the Search block having a <form> or the Social Links a <ul> as their root elements, resulting in an invalid HTML such as the following:

<nav class="items-justified-right main-navigation wp-block-navigation">
  <ul class="wp-block-navigation__container">
    <li class="wp-block-navigation-link"><a class="wp-block-navigation-link__content" href="/" title="Home"><span class="wp-block-navigation-link__label">Home</span></a></li>
    <li class="wp-block-navigation-link"><a class="wp-block-navigation-link__content" href="/about" title="About"><span class="wp-block-navigation-link__label">About</span></a></li>
    <form role="search" method="get" action="http://navigationblock.local/" class="wp-block-search__button-outside wp-block-search__text-button wp-block-search"><label for="wp-block-search__input-2" class="wp-block-search__label">Search</label><p></p><div class="wp-block-search__inside-wrapper"><input type="search" id="wp-block-search__input-2" class="wp-block-search__input" name="s" value="" placeholder="" required=""><button type="submit" class="wp-block-search__button ">Search</button></div></form>
    <ul class="wp-block-social-links"></ul>
  </ul>
</nav>

What is your proposed solution?

The goal of this issue is to remove the Navigation block's <ul> wrapper from its inner blocks and therefore generate a valid, accessible HTML. However, some inner blocks such as the Navigation Link block would now need the wrapper they would no longer have, so we need to explore the best approach to solve this:

• Changing the available Inner blocks, so that, for example, users can't directly add a link and first need to add a link list. Although this is the simplest approach, it may not be the best user experience, requiring the user to go through many steps to add a link.
• Automatically adding any required wrapper element when adding an inner block that needs it. This means if the Navigation block were to contain links, followed by non-link blocks, and more links, multiple <ul> elements would wrap the different link groups (#24364 (comment)), generating a markup like the following:

<nav> <!-- navigation block container --> 
    <ul> <!-- update this to be owned by navigation link -->
        <li>Page Links</li>
        <li>
             <ul><li>sub menu</li></ul>
        </li>
    </ul>
    <form><label>Search</label><div><input><button>Search</button></div></form> <!-- search -->
    <div /> <!-- spacer --> 
    <ul>
        <li>Social Links</li>
        <li>Social Links</li>
    </ul>
    <div />  <!-- spacer --> 
</nav>

Related Issues

• Closes Navigation: Next steps for supporting non-links in Navigation block and screen #24364.
• Navigation: Try adding Links block #24644 explores adding a Links block.
• New "Pages" Block: Display a list of links to a site's pages #23689 adds a "Pages" block that displays a list of links to all pages.
• Navigation: Non-link blocks should be wrapped in a <li> #23915 originally discussed the solutions proposed.

[noisysocks]

Just noting that the two proposed solutions here were discussed in #23915.

[tellthemachines]

#30430 attempts the first solution, which as predicted is cumbersome for users, so I'm now looking into possibilities for the second. Something like this API would be super helpful here, but I'm not sure if there's history to why that was never implemented (I vaguely recall some discussion about it at the time but can't find any references apart from the issues already mentioned above).

To make the Navigation block markup fully semantic and accessible as per these recommendations, we need to:

• iterate through inner blocks;
• identify which of them are core/navigation-link blocks, and possibly also core/spacer blocks (as we should allow spacers to be inserted between link blocks)
• add <ul>s around any groups of adjacent navigation-links/spacers.

There's still a lot of potential for block markup to become messy, as if someone inserts a link, followed by e.g. a Search block, and then another link, we'll end up with two lists, each with one link inside 😬

I'm starting to think we could do worse than have a simple Navigation block that only allows links, and separately provide a bunch of blocks like Dropdowns, Link Lists etc. that can be composed into mega menus together with Images, Search, Columns and whatever else might be useful in that scenario.

[noisysocks]

Something like this API would be super helpful here, but I'm not sure if there's history to why that was never implemented (I vaguely recall some discussion about it at the time but can't find any references apart from the issues already mentioned above).

I had a crack at implementing that API but ran into some issues and then didn't get a chance to revisit. It might be easier now that useBlockProps() etc. is more developed.

[aristath]

Just my 2c, but wrapping elements in <li></li> (so the 2nd option) seems like the better option to me from a semantic & a11y perspective

[gwwar]

I think one of the simpler ways of fixing this would be to allow the Navigation block to render an li wrapper for each child. (We can also update navigation link to not render that top li wrapper). Since these are dynamic blocks I think as long as these changes happen in tandem things should mostly square out.

Are there any known blockers for that approach?

[priethor]

I think that's an even better approach than adding only the <ul> wrapper around the blocks, as it can make blocks like the Home Link Block work properly in other contexts as was mentioned in #30926

However, the Navigation Block and other parent blocks would need a way of knowing which inner blocks need a wrapper and which ones, like the Search Block, don't. Would a new property in the block metadata, telling the block's parent it can be wrapped/grouped, make sense?

[gwwar]

The search block does need an li wrapper too. It's invalid markup otherwise

without li	with li

parent blocks would need a way of knowing which inner blocks need a wrapper and which ones, like the Search Block, don't.

This might be a need for other container blocks, but I'm relatively sure that for a ul container, we should wrap all children in li. (Folks are welcome to correct me if I'm misunderstanding the spec).

[priethor]

That's right, I should have specified I was thinking of this proposal (comment) where lists of links should only contain links, and therefore multiple <ul> elements could exist and the Search block would be outside any nav/ul/li wrapper 😅

[tellthemachines]

I think one of the simpler ways of fixing this would be to allow the Navigation block to render an li wrapper for each child.

That approach was explicitly not recommended by the a11y team, see points 2 and 3 here. The problem is essentially that lists should contain only items semantically related to each other, but there's plenty more info and resources on the topic in that comment.

I've had #30551 open for a while, looking for feedback on the approach of changing the markup only on the front end (an alternative would be to try to also wrap nav links in a ul on the editor side, but we'd need to make changes to how inner blocks work in order to be able to do that).

[gwwar]

Thanks for the extra context @tellthemachines! That's a good point. 🤔 I do feel that whatever solution we come up with though should ideally align editor/published markup, and that the user won't necessary need to manage this level of detail themselves in UX interactions.

@priethor (or other folks) would it be possible update this issue summary to add an example of current markup, and an example(s) of proposed markup that aligns with a11y recommendations?

For example are we more aiming for something like this?

<nav> <!-- navigation block container --> 
    <ul> <!-- update this to be owned by navigation link -->
        <li>Page Links</li>
        <li>
             <ul><li>sub menu</li></ul>
        </li>
    </ul>
    <div /> <!-- spacer --> 
    <ul>
        <li>Social Links</li>
        <li>Social Links</li>
    </ul>
    <div />  <!-- spacer --> 
    <form><label>Search</label><div><input><button>Search</button></div></form> <!-- search --> 
</nav>

[tellthemachines]

I do feel that whatever solution we come up with though should ideally align editor/published markup, and that the user won't necessary need to manage this level of detail themselves in UX interactions.

Yeah, we've had some back and forth about that in #30551 and #30430 😅

The other possibility is if we could iterate through the children returned by useInnerBlocksProps, we'd be able to match editor and front end markup without having to burden users with intermediate blocks.

[gwwar]

Yeah, we've had some back and forth about that in #30551 and #30430 😅

@tellthemachines, definitely this one will be tricky to get right, and it could be the case that we need to build new mechanisms for this.

I'm happy to review both approaches, though I'd like to understand what target published html we're comfortable with before I dive in, and what needs the navigation editor has.

🤔 Some early thoughts from me is that a home link is likely to be a new block as well, so the ul wrapper block in 30430 is potentially viable, but we'd likely need to add a lot of extra handling to ensure that a user doesn't need to interact with it. So for example, we might want to ensure that:

• Clicking on the ul wrapper would instead select the closest child target
• It's not selectable by block navigation (for example list view, or the parent button would skip one level up)
• Auto add when adding a home/navigation link without it existing. Note that adding this wrapper might be necessary on a non-empty navigation block, a simple example would be a nav block that has nav links, a spacer, then more nav links
• Remove the wrapper when last li child is removed

[tellthemachines]

In terms of semantics the aim should be to have only links inside the list element (so Home block should definitely be in there) and everything else - search, other lists such as page list, or social icons - outside as direct children of the nav element.

An exception is the Spacer block, which we need to allow inside submenus and between nav links. I think ideally Spacer shouldn't render any markup at all, and take the form e.g. of a margin-bottom on the previous block. But that might be out of scope for this piece of work. So perhaps we could, for now, wrap Spacer inside an li whenever we render it in a links list, and maybe use an aria role of presentation on the li so it doesn't get read out by a screen-reader.

the ul wrapper block in 30430 is potentially viable, but we'd likely need to add a lot of extra handling to ensure that a user doesn't need to interact with it

Another issue with the wrapper block is how the inserter is going to work: it should be inside the wrapper for adding a link block, but outside for other types of block, except we really don't want to show more than one inserter because that would be horribly confusing.

Also, there may be more than one wrapper: if someone adds a Home link, followed by a Search block, and then a regular page link, we would need something like

<ul><li>Home</li></ul>
<form>Search</form>
<ul><li>Page</li></ul>

[jasmussen]

An exception is the Spacer block, which we need to allow inside submenus and between nav links. I think ideally Spacer shouldn't render any markup at all, and take the form e.g. of a margin-bottom on the previous block.

I'd like to see margin controls on virtually every block we have in the library, which would benefit notably the space-between option we have for justification, and enable truly magical things if we were to enable a horizontal display for groups (#24473).

But even then, I still see a place for the spacer block to exist. There's something useful in having just an empty block, for when you need to separate sections in a document without using a specific separator. Throw in custom unit support (#22956) and you can essentially have a responsively scaling section divider. Notably for megamenus, I could see the spacer being an important tool to handle layout that might be more intuitive that applying margins.

Admittedly on the slightly crazy side, I've applied a background color and some transforms to the spacer on a website I made:

[priethor]

Updated the proposal in the issue description with example accessible markup.

[tellthemachines]

if we were to enable a horizontal display for groups (#24473).

A Flex block! Nice 😍

But even then, I still see a place for the spacer block to exist.

Absolutely, we shouldn't get rid of the block. Users love it 😄 But we could make it work in a way that doesn't output markup.

[tellthemachines]

Now that #30551 has been merged, we can close this one too!