Size Change: +19 B (0%)

Total Size: 1.05 MB

_{compressed-size-action}

Thanks for addressing this so promptly! It's working well for existing submenus, but when adding a new one the inserter doesn't seem to be showing up in the submenu container.

Good catch. I think the styles hide empty submenus by default, so that will require some tweaking.

Ok, I've updated the Submenu PR to use Submenu in the Page List conversion, so this one should work correctly after rebasing.

I tested this and like mentioned above there was no way to add an item in the submenu.

Have updated this and gone a bit further to fix a lot of the broken navigation editor styles. Things should be much improved now.

This fixes the broken nav screen rules for me, nice work:

Because of those regressions, I wonder if we should extract some of those fixes and land them separately (and I'd be happy to help here). Per the discussion in #18395, #33775 could be one option for configuring submenus. But there are some styling issues which while they shouldn't necessarily be too hard to fix might nevertheless make it miss the next plugin release:

I'll try and help here as best I can, but in case we'd like to fix the regressions in separately, that's also on offer. Let me know.

I think it's ok to wait for the parent branch to be updated. There are a few CI issues as well to sort in that branch, but hopefully nothing major.

Isn't this a breaking change for classic menus? I mean, we don't expect a tags with no href in classic menus and the submenu "opener" is just that.

Testing this with TT1 results in a pretty weird menu:

and some weird markup:

<li id="menu-item-1080" class="menu-item menu-item-type-custom menu-item-object-custom menu-item-has-children menu-item-1080">
	<a>Some submenu</a> <!-- THIS HERE IS WEIRD -->
	<ul class="sub-menu">
		<li id="menu-item-1081" class="[..]">
			<a href="http://[...]">Consequat Ud Atet</a>
		</li>
	</ul>
</li>

@draganescu I don't think this changed anything, you could still create menu items without hrefs before this PR. Would you be able to share more details about what you're seeing? Unfortunately my dev environment needs to be fixed, so I'm unable to test.

(When I last tested the submenu block still prompted with the URL Control upon creation. That might not be the case though if the implementation has changed in the submenu block.)

Hmm, I was able to test and see what happens. For some reason the Link Control doesn't open straight away now, but if you click out and back into the submenu block it shows up. Seem like a bug with Submenu, it happens in the post editor too.

I can take a look at a fix tomorrow, but an issue would be appreciated.

you could still create menu items without hrefs before this PR

What I mean is that in the classic navigation editor adding a custom link with no href is not possible. Initially. Hence I overreacted with the words "breaking change".

Still it is possible to achieve an empty <a> in the classic navigation editor: one would add say # to the URL field and then remove the href from the link props.

But now it's auto happening and I wonder if we increased our odds for themes to encounter surprise <a> tags with no href, which are valid but behave differently than anchors with href.

Maybe because it was hard to add a custom link without a href (although possible) many themes just expect href to be in menus. And what we do now with automatically making href-less anchors by adding a block is a very different thing.

What I mean is that in the classic navigation editor adding a custom link with no href is not possible. Initially. Hence I overreacted with the words "breaking change".

Still it is possible to achieve an empty in the classic navigation editor: one would add say # to the URL field and then remove the href from the link props.

#18866 indicated that people have been using this as a roundabout way of creating non-link parents for submenus, so not allowing the behaviour at all is more of a breaking change 😅

I'll look into why the link control isn't opening at first; it should do so.

#34798 should fix the issue.