Navigation block: Unable to use sub-menu item & deletion deletes parent menu

[annezazu]

Description

When using the Navigation block and added sub-menu items, I noticed that there appears to be a regression in behavior where I'm unable to see the sub-menu item upon addition and, if I remove it, it also removes the parent menu item.

Step-by-step reproduction instructions

1. Add a navigation block.
2. Add a few menu items.
3. Add a sub menu item.
4. See initial state where you can't seem to add in a link to the sub menu container.
5. Try to delete the sub menu item and notice it deletes the parent.

Screenshots, screen recording, code snippet

sub.menu.item.mov

Environment info

• WordPress 5.9-alpha-52262
• Gutenberg 12.01
• TT1 blocks
• Chrome

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[annezazu]

Previous sub menu item behavior:

previous.sub.menu.item.mov

Marking this as a regression for now!

[jasmussen]

Yes indeed. This appears to be a side effect of showing the block toolbar below the block if the block is near the top of the screen, as ticketed in #29464. The behavior, I feel, still makes sense to have for blocks that truly bump against the edge; for example some of the TT2 patterns go all the way to the top for the header template parts.

CC: @Addison-Stavlo: do you recall if there are some levers we can adjust to this behavior? Also CC: @jameskoster, because an alternative fix might be to introduce the dark gray frame after all. I don't necessarily love doing this as it reduces the available real estate, but in weighing trade offs, it might be an option.

[jasmussen]

Another solution which might actually be better than the above is to change back the block toolbar absorbtion. Right now the block toolbar of submenu items is absorbed in the top toolbar, as opposed to being positioned near the block. This is ticketed separately as being a bit confusing due to the lack of spatial context. But in this case it might also mitigate the issue, as there would most likely be clearance enough above the selected first submenu block to accommodate the toolbar without bumping it below. CC: @getdave in case you have thoughts.

[getdave]

...to change back the block toolbar absorbtion. Right now the block toolbar of submenu items is absorbed in the top toolbar

As far as I remember it's a one liner fix. Simply remove this line

gutenberg/packages/block-library/src/navigation/edit/inner-blocks.js

Line 117 in 570751a

__experimentalCaptureToolbars: orientation !== 'vertical',

Other than that it's a UX concern. When we introduced the capture toolbars the UX with the toolbar locked to the nested submenu was very poor. If that's improved then we might benefit from removing the capturing behaviour on this block. I'll leave that decision to design folks though 🙇

[jasmussen]

Oh thank you, both for the fast response and the code piece.

It was definitely needed, back in the day! Every block had borders around them, and there was a great deal of UI weight to deal with. I do think we've definitely reached a point where the UI can handle it. The one thought I still have attached to this is that we might still want the submenu block to capture the toolbars, instead of the navigation block, the idea being that as you build out the submenu, it can be helpful to see the preceding menu.

Seeing as it's only applied when horizontal, I took the vertical version for a spin:

I think it's okay, and that if it helps mitigate this issue, it might be worth it.

[jameskoster]

Also CC: @jameskoster, because an alternative fix might be to introduce the dark gray frame after all.

FWIW, adding the frame in the Site Editor is also a consideration for #36535.

[Addison-Stavlo]

@Addison-Stavlo: do you recall if there are some levers we can adjust to this behavior?

I do not, unfortunately. There could be someway to go about that with more digging, but nothing I remember at a glance.

we might still want the submenu block to capture the toolbars, instead of the navigation block

I think this makes a lot of sense. If the toolbar is not positioned with respect to the selected block (the submenu in this case), then I think it becomes increasingly difficult for the toolbar to behave in a manner that is non-intrusive in all cases.

[talldan]

I also made an issue about this - #36335

It isn't just an issue in the nav block, we should also consider that positioning the toolbar under the block might be a problem for third-party blocks too. Any block that overflows its container like the nav block does will have this issue.

[Addison-Stavlo]

positioning the toolbar under the block might be a problem for third-party blocks too. Any block that overflows its container like the nav block does will have this issue.

Technically couldn't a 3rd party block could also overflow its container from above? In that case positioning it above the block would have the same issue.

This does seem difficult to handle well. Right now, bottom positioning should only happen if there isn't enough room for top positioning to happen without the toolbar obstructing the selected block. With the previous always-top positioning, there were instances where the toolbar would cover the navigation block itself when selected (and other blocks as well) which is why we tried moving it beneath the selected block when this would be the case. The bottom positioned toolbar can obstruct blocks that overflow their boundaries, but that seems less of an issue than the toolbar obstructing inside the well defined boundaries of the selected block itself (but still something we need to figure out how to handle). 🤔

Another idea might be that when the toolbar would obstruct the given block boundaries, instead of moving the toolbar below the block we move it into the fixed top-toolbar location. However, this would be a lot more 'jumpy' in editor interaction that I assume would not feel very favorable to folks either.

[noisysocks]

This is a duplicate of #36335 but since this issue has more discussion I'll close #36335.

[jasmussen]

Outside of completely disabling the "show toolbar below block" feature, is it possible to add an edgebuffer so it only activates when the block is most literally touching the top edge? For example for header template parts?

[talldan]

Technically couldn't a 3rd party block could also overflow its container from above? In that case positioning it above the block would have the same issue.

I don't think many would do that because the toolbar has always been above the block.

[Addison-Stavlo]

Outside of completely disabling the "show toolbar below block" feature, is it possible to add an edgebuffer so it only activates when the block is most literally touching the top edge? For example for header template parts?

I think there are a few (20 maybe) pixels you could squeeze out of the top sticky position to fit it above on some of those edge cases, but it will also wedge the sticky positions closer to the top bar which could be a bit ugly. Or by 'edgebuffer' do you mean above the template in the editor canvas? As that may get rid of the need for the below option altogether, but may look a little funky.

I don't think many would do that because the toolbar has always been above the block.

True, but my point is maybe the issue isn't simply whether it is above or below but how we determine where above or where below it actually needs to sit. Simply placing the toolbar above the block has conflicts and can completely disable the users ability to interact with some blocks near the top of the editor canvas. How we are placing it below has issues as you noted due to blocks that may overflow boundaries underneath. At the moment both of these previous iterations are too naive to supply a solution without conflict with the given real estate currently available in the editing canvas.

So we either need to make this location determination logic more complex, add more buffer in the editor canvas, or take this interaction out of the editor canvas for this edge case. An example for each of these options:

• Keep the conditional bottom positioning, but manage to include any popover/dropdown for children blocks in the calculation of where the bottom position should be so we aren't obscuring anything like this.
• Get rid of bottom positioning + Add a ~100px buffer above the template inside the editor canvas such that even if the top most block is selected there is enough scroll position for the toolbar to be truly above the block.
• Get rid of bottom positioning + replace its behavior with conditionally enabling the block settings in the top toolbar in this edge case.

[jasmussen]

I think there are a few (20 maybe) pixels you could squeeze out of the top sticky position to fit it above on some of those edge cases, but it will also wedge the sticky positions closer to the top bar which could be a bit ugly. Or by 'edgebuffer' do you mean above the template in the editor canvas? As that may get rid of the need for the below option altogether, but may look a little funky.

I did mean those maybe 20 pixels might be enough in many cases.

One of the challenges with allowing the toolbar to overlap the top bar in the past was when you would scroll the page enough that the toolbar would scroll off the page, it would sit above the top toolbar. But I wonder if that would behave differently with the recent refactors (iframe, popovers, etc.) 🤔

[Addison-Stavlo]

I did mean those maybe 20 pixels might be enough in many cases.

Probably for many initial cases, but as soon as a user wants a header that has their navigation as close to the top of it as possible we will encounter the conflict again. It seems like it would be better to find a more robust way to go about it?

[jasmussen]

It seems like it would be better to find a more robust way to go about it?

Yes, it increasingly seems like adding the dark frame around it might be the longer term solution, but for that to land it needs to consider some context making it a 6.0 or later thing. And so I was just wondering if a small buffer fix could be a mitigation in the mean time.

[Mamaduka]

Hi folks,

I wanted to check in to see if we have a solution to land in 5.9?

[jasmussen]

I took a quick look at our two current paths forward, keeping or reverting the showing of the toolbar below the block. If you'd like to play along, you can try removing the exclamation mark here.

Here's what it looks like if we keep the toolbar above the block at all times:

Here's below, but showing also how it obscures the submenu creation:

The direct solution here seems to be the dark frame, which I suspect we will add eventually. But it won't make it for 5.9, so in the mean time, we have to weigh the tradeoffs.

Of those options, I'd suggest we keep the option to show the toolbar below the block, mostly because the use case where the toolbar obscures a submenu is an edgecase that, looking just at core blocks, happens only to a navigation block that sits within the top 72px of the editing canvas, when building submenus. While this is a case to support, I suspect it's not the most common use case.

In addition to the dark frame, there are numerous future enhancements we can make to the navigation block, which all focus on removing the need to build the navigation block right in the canvas. For example, since it loads its content separately now, not only should you be able to edit the contents in template isolation mode, but if the site doesn't have any menu created when starting out, we can probably pre-populate it with the pages on your site (#36667 and see also #36858).

[Mamaduka]

I'm removing this issue from WP 5.9 project board since RC1 was released last night.

We can ship focus mode for the Navigation block in the next release.

[chthonic-ds]

I'm unable to see the sub-menu item upon addition and, if I remove it, it also removes the parent menu item.

Noting in 5.9-RC3 the second part of OP's issue is still present: the "Remove submenu" tool also removes the parent item.

[courtneyr-dev]

Removing the submenu removes parent still.

I also noticed when I add submenu and begin typing, it actually adds to the beginning of parent text.

WP 5.9-RC3-52619
Chrome Version 97.0.4692.71

remove-submenu-removes-parent.mp4

[courtneyr-dev]

For comparison, I also tested in G12.5.20220122. Passes.

submenu-g12-5-x.mp4

[getdave]

Removing the submenu removes parent still.

Hi @courtneyr-dev. Thanks for coming back to this. Can you confirm that your expectation is:

• you add some items to you Nav block
• you choose one of those items and you convert to a submenu
• you add items to that submenu
• you remove the submenu items
• you remove the submenu
• you still expect the item that was originally converted into a submenu to be there

So the point is you want to get rid of the "submenu-ness" but retain the item itself as a top level item?

The technical issue is of course that the top level item is no longer a standard link block but rather a submenu block. Therefore when you delete it, it makes sense that it's removed rather than converted into some other block types.

Options as I see them:

1. Expose a button to revert a submenu to a standard menu item.
2. Automatically revert the block to a standard link if all menu items are removed from it.
3. Do better (via tutorials, docs or even design updates) at explaining to users that this is now a submenu block and they need to use the built-in transform rather than delete if they wish to retain the menu item.

Below is a video showing how to use the transform to convert a submenu back to a normal link without having to delete it first:

Screen.Capture.on.2022-01-24.at.15-35-07.mov

Open to suggestions as this isn't a simple one and probably requires design input.

[courtneyr-dev]

Your summary of expectations is correct. I like option 2.

Automatically revert the block to a standard link if all menu items are removed from it.

Solving retaining the parent menu item in any form is good.

[talldan]

I would go for the first option (Expose a button to revert a submenu to a standard menu item.). I'm not sure about option 2, users might remove all items (e.g. via multiselect) in order to add some different submenu items. The submenu switching back to a link would make that process more complicated.

I'm removing this issue from WP 5.9 project board since RC1 was released last night.

We can ship focus mode for the Navigation block in the next release.

@Mamaduka Does that mean 6.0? Navigation focus mode would be a pretty heavy feature to ship in a minor release.

I think there's probably still scope to ship an improvement to the toolbar issue for 5.9.1.

[Mamaduka]

@Mamaduka Does that mean 6.0? Navigation focus mode would be a pretty heavy feature to ship in a minor release.

@talldan, it depends on the changing size; we don't want to introduce more bugs in the minor 😄 . So I think that means shipping small features and bug fixes only.

I think there's probably still scope to ship an improvement to the toolbar issue for 5.9.1.

That would be great!

[getdave]

I would go for the first option (Expose a button to revert a submenu to a standard menu item.)

I think your instinct is correct. Having things happen automatically seems like a great idea until it isn't. Then it just starts to feel confusing.

I'll draft a PR to make an explicit revert button and see what folks think.

[tellthemachines]

Noting here that, if there are no items in the submenu block, it's possible to transform it back into the link it started out as by using the transforms menu (click the block icon/block name in the toolbar) and selecting "Custom link". The transform options are not available if there are items in the submenu, in order to avoid accidental deletion of a whole submenu.

[jasmussen]

The transform options are not available if there are items in the submenu, in order to avoid accidental deletion of a whole submenu.

That's a totally valid concern. Since we have explicit undo/redo buttons and that you can explicitly uncheck a change before saving it, as a rule of thumb I tend to think we should build these guard-rails only when the concern has proven itself to be a problem. The risk is we add confusion to the flow (why isn't the transform available here when it's available everywhere else?) even if it's with the best of intentions.

[getdave]

I'd really appreciate any input on #38203 which implements a button in the UI to revert. @tellthemachines has expressed valid concerns and I've suggested potential work arounds. That said, having just read @jasmussen's comment about the undo/redo and not over optimising things, I'm not sure which way to go now.

Options very much welcomed and appreciated.

[courtneyr-dev]

The other part of this is still when we add a submenu item and begin typing, the text goes before the parent item, not jumping the cursor to the submenu item. When I add the submenu item, I expect the cursor to go to that item. When I begin typing, I expect this to be in the submenu item.

submenu.item.mp4

Running 6.0-alpha-52690 nightly + Gutenberg Version 12.6.20220207

[getdave]

@courtneyr-dev I've tracked this in a new Issue #38617

[getdave]

With the merge of #38203 some of the UX around this is elevated. However the core issue cannot go away for the reasons I stated in #36977 (comment).

Therefore I propose we close this with the understanding we can now signpost users towards the "convert back to link" button.

[talldan]

It seems like this shouldn't have been closed. The issue with the block toolbar overlapping the submenu is still a problem.

There's now a new bug report in #40382 for this - please don't close that one without fixing it 😄 .

[getdave]

This was my mistake. In my defence this appeared to have become a dual issue and I lost sight of the first part which was the overlap with the submenu block.

Whilst I take responsibility, it does point to the need to have Issues that track one thing.