Size Change: +6.71 kB (+1%)

Total Size: 1.05 MB

_{compressed-size-action}

Thanks so much for trying this. This is what I see:

Outside of the little spacer complexity at the end (where the two first items should be ul wrapped, as should the last one, but not the spacer), from a pure user experience perspective, this feels like the right path forward.

I understand the reluctance, but it is such a key part of the flow that it's crucial to get right. And this is a challenge that will not be unique to the navigation block, it will come up again and again. One incoming example is the table block receiving nesting support. Basic table markup looks like this:

<table>
     <tr>
         <td>The table body</td>
         <td>with two columns</td>
     </tr>
</table>

But when you add a header or footer, the markup changes:

<table>
    <thead>
        <tr>
            <th colspan="2">The table header</th>
        </tr>
    </thead>
    <tbody>
        <tr>
            <td>The table body</td>
            <td>with two columns</td>
        </tr>
    </tbody>
</table>

While the nesting system could replicate that 1:1, the user experience as it exists today, with each layer seletable, it would be unwieldy. This is an aspect where our friends at TinyMCE do it so well:

In other words, it's a problem we need to solve. In the case of the table block, a passthrough container block as discussed in #7694 might be a key ingredient in flattening the structure and showing a single block toolbar.

In the case of the navigation block, the challenge is that the ul needs to show up conditionally based on the content of your block. And to enable menus that have, for example, 2 menu items, a site logo, and then another 2 menu items, this is a challenge that needs solving. The missing UL in the editor is a tradeoff that feels worth it to me, and something I can help CSS up easily.

@youknowriad I'd love your thoughts here, as I know you've been looking at conditional divs, serverside rendered, lately.

Coming back to this after thinking about it more, I think the most important bit of all is that we should not put the onus on the user to figure out what type of blocks need be wrapped in what containers. Users see a navigation block and assume they can insert links to pages, perhaps search, perhaps social links.

When framed like that, what gets output on the frontend then becomes our headache to solve, either through server side rendering like in this PR, or revisiting the structure of the markup. With the server side rendering, the trade-off of markup that diverges between editor and frontend is easily worth it, as we can maintain similar classnames to target instead of the elements directly.

CC: @noisysocks @priethor @mtias @draganescu

I still feel like having unsemantic html on the editor side should not be used as an escape hatch.

We should use this solution only if we arrive to the conclusion that it is impossible to have a good UX and semantic editor markup. For now it's just complicated to see the best solution but not impossible.

The discussion in #31951 points to this solution as the best way forward, so I'm marking this PR ready for review now.

I've also fixed some conflicts and updated this PR to work with the responsive wrapper.

Updated this PR to also remove the li and uls from Navigation Link block on the editor. Front end output remains unchanged.

I've also excluded the Spacer block from the uls, and temporarily removed it from the list of allowed children of Navigation Link as we cannot semantically include it until we make changes to how Spacer renders. I'll be experimenting with how to solve that problem in the next few days, but it should be a separate PR.

Reviews and feedback appreciated!

Took it for a spin:

As far as I can tell, this one threads the needle! Very impressive work 👌

Overall this looks pretty technically sound to me, and the user experience here feels good! I smoke tested a bit, and couldn't find any issues.

In terms of moving this forward:

• Let's double check that folks were okay with the editor/published view . It feels that we are, I just wanted us to mark that down as a comment on the PR.
• 🔍 if there are any other styling changes needed.

@gwwar what does this mean:

Let's double check that folks were okay with the editor/published view .

@gwwar what does this mean:

That we were okay with diverging editor markup from published markup. This is mostly to document for other 👀 since I don't expect folks to be aware of all the conversation threads here on it.