Filter out duplicate menu entries when creating a Nav block from existing menu

[getdave]

Description

In #36307 we learnt that because we're auto-creating copies of classic menus on Theme switch from classic -> block based theme, we end up with duplicate entries under the the Create from existing menu... option in the Nav block placeholder.

This PR is a initial (possibly naive) attempt at resolving that situation by filtering out any classic menus that already have a block-based facsimilie.

Potential concerns

• What if I've edited the block based facsimilie but I broke things and want to restart from the original Classic Menu?
• We're relying on the slug containing the ID of the original classic menu.

How has this been tested?

1. Activate classic theme.
2. Create some Menus and name them clearly.
3. Switch to a block based Theme.
4. Go to Site Editor.
5. Create Navigation block.
6. See Nav block placeholder.
7. Click Select existing menu...
8. There should be no group in the dropdown for Classic Menus (we've filtered those out).
9. Under Menus you should see entries for each of your classic menus (those you created above). These will be prefixed with Classic Menu: - note this PR does not add that functionality.

Bottom line: we're just verifying any classic Menus which have block-based facsimilies are not listed.

Screenshots

Types of changes

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[jasmussen]

To be sure I understand this correctly, let me describe a flow and how I imagine this PR might handle it:

• I have an existing menu I created in the classic navigation editor, which I called "My Menu".
• I upgrade to a block theme, and in the site editor's Header template, I click the Navigation block and select "My Menu" from the Existing menus dropdown. It shows up under a subheading, "Classic Menus".
• I save, and I'm done, and my menu was ported over to the navigation block.
• Now I insert a new navigation block, one in the footer, for example. In the "Existing menus" dropdown, I see the "My Menu", a single instance of it, which is in fact the copied version that was created when I chose this item the first time.

Is that correct? If so then that sounds good to me. I don't imagine importing legacy menus to be a common enough occurrence that you need access to them for all eternity, and simply filtering based on copied name seems like a fine way to make sure you choose the right one, the one that stays in sync.

Let me know if that was helpful.

[getdave]

@jasmussen not quite. Basically what happens is

• You are on a Classic Theme.
• You create some Menus.
• When you switch to the block theme we (already) take each classic Menu and make a block-based copy of it prefixed with Classic menu: (eg: Classic menu: Main Menu).

What is happening on trunk is that when you switch to a block theme and you then create a new Nav block you have the option to Select existing menu... and under that drop down you see both:

1. The original classic Menu (under heading Classic Menus).
2. The block-based facsimile of the classic Menu that was created on Theme switch (listed under Menus with the prefix Classic menu: ).

My concern is that this is confusing because you see both the original Menu and the block-based version of that same menu. As people who worked on this feature we understand why that happens, but I suspect an average user will be quite confused with so many references to

• The term Classic menu.
• The same menu name being repeated in different contexts.

However as @draganescu explains #36307 (comment) we need to be sure we're not stopping users from achieving other workflows.

My suggestion in this PR is that if a copy of the classic Menu exists as a block-based menu then we filter it out of the list of Menus shown under the Classic Menus group in the dropdown.

But...I'm open to alternative suggestions as I'm really not sure what the best UX here is.

[jasmussen]

Ah, thanks for explaining.

If we already make a migrated copy, do we need to also show the pre-migrated versions?
If we were to filter pre-migrated versions out, could we perhaps even remove the "Classic:" prefix?

If menus are already properly migrated, it seems like we wouldn't need any classic sub-section at all, or am I missing an aspect?

[talldan]

If we already make a migrated copy, do we need to also show the pre-migrated versions?

I think that's the purpose of this change, to avoid showing the pre-migrated versions.

If menus are already properly migrated, it seems like we wouldn't need any classic sub-section at all, or am I missing an aspect?

Currently not all menus are migrated, only those assigned to locations. But perhaps it's worth reconsidering that and migrating all menus and as you say there might no longer be a need to show classic menus in the placeholder. I think we'd need to consider the potential impact of that. Is there any situation in which a user might still need access to the classic menu. The permutations are a bit mind boggling when you consider hybrid/universal themes, as they might not go through the theme switching process 🤯.

There is another option, which is a little more involved, but removes some risk, it would involve all of these steps:

• Stick with the current behavior and only migrate menus at locations on theme switch
• Hide the original classic menus in the dropdown for any migrated menus
• Remove the distinction between classic and block menus in the dropdown
• If a classic menu is selected, migrate it automatically at that point

[javierarce]

Sorry for being so late to the party.

If I have grasped the details of the problem, the solution that @talldan mentions in his last comment seems perfect to me as it is entirely transparent to the user.

But if we were to show the classic menus, something that I think would make that placeholder less confusing would be to label those menus differently.

I don't know if this is technically possible, but would it make sense to:

1. Instead of "Classic menu", say something more directly associated with the previous user action, so it's recognizable, like: migrated or imported?
2. Style the label as if it were a tag?

[talldan]

There is another option, which is a little more involved, but removes some risk, it would involve all of these steps:

• Stick with the current behavior and only migrate menus at locations on theme switch
• Hide the original classic menus in the dropdown for any migrated menus
• Remove the distinction between classic and block menus in the dropdown
• If a classic menu is selected, migrate it automatically at that point

There's a problem with this. If we have a classic menu 'Main Menu' and the user selects it, that menu is migrated to a block menu. The system proposed would hide the classic version of 'Main Menu' and only show the block menu. But what happens then if I delete the block version of 'Main Menu'? The classic one shows in its place, which would seem really buggy. Like the delete didn't actually have any effect.

Seems like it needs a bit more thought.

[getdave]

Do we need a separate area in the UI for the action of "converting" a classic menu rather than "inserting" an existing block menu. Perhaps we're blurring things and the distinction afforded by separate UI would help?

[draganescu]

To implement @talldan 's solution, which I also think it would be the best experience, we need a way to keep track of which classic menus were converted. Doing a filtering out of classic menus that have the same slug (name+id) as a block one is a way but it's guesswork.

[getdave]

Hide the original classic menus in the dropdown for any migrated menus

So still show all the classic menus but only those which haven't already been migrated?

If a classic menu is selected, migrate it automatically at that point

So yes, this seems to imply we'd still show classic menus (unless they have already been migrated). If the user selects one of these unmigrated menus then we'd convert it on the fly (at the point of selection).

This needs a tracking mechanism. This isn't a priority for 5.9 so this is looking more and more like a 6.0 thing.

[getdave]

We should pursue an alternative approach in a follow up PR because the consensus is leaning towards a more complex tracking mechanic.