Navigation Editor: Unexpected behavior when reloading menu with placeholders

[gwwar]

Description

If we create a menu with placeholder links, save, then reload the menu we'll see some inconsistencies:

• Tag, Category and Custom placeholders look correct
• Post and Page placeholders render as items like #474 (no title) (the menu id created)
• Submenu placeholders disappear

We do show a validation error on save, so I was also surprised to see that the menu serialized. (Overall it probably is friendlier to allow saving, but we may want to make the warning a bit clearer).

Placeholders after reload	Published view

Step-by-step reproduction instructions

1. Visit the Gutenberg experiments page /wp-admin/admin.php?page=gutenberg-experiments and enable the Navigation Screen
2. Visit /wp-admin/admin.php?page=gutenberg-navigation and create a new menu
3. Open the global inserter (Blue plus button in top left)
4. Add all block types from the inserter
5. Save
6. Reload the page
   Expected: menu doesn't save OR placeholders appear as they did before
   Actual: See video/image

Screenshots, screen recording, code snippet

CleanShot.2021-09-30.at.11.33.13.mp4

Environment info

Gutenberg trunk (d78bbc8)

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

[gwwar]

So looking at the endpoint code, I think it's reasonable to enforce a url/title for a custom link, say (we're linking directly to WordPress.org). I think the problem we're running into here, is that we also assign the "custom" type for empty placeholder links that we haven't decided to pick anything for yet. (Kind/Type isn't specified yet either). This is exacerbated by recent behavior changes to allow direct links in the inserter too in #34899. If we save these empty placeholders, they will be visible in the published view.

In the spirit of aiming for a lighter navigation block experience #34041, what do folks think about changing the direct insert links from custom links to Post Type in the navigation editor. (Alternatively we could try to serialize non-specified placeholders as page placeholders but that may be confusing.)

gutenberg/lib/class-wp-rest-menu-items-controller.php

Line 415 in 1f75f8f

'menu-item-type' => 'custom',

explicit custom link	attributes set

CleanShot.2021-10-05.at.11.26.24.mp4

[tellthemachines]

In the spirit of aiming for a lighter navigation block experience #34041, what do folks think about changing the direct insert links from custom links to Post Type in the navigation editor. (Alternatively we could try to serialize non-specified placeholders as page placeholders but that may be confusing.)

Given these blocks are now inserted directly, I don't think we should restrict them to a type such as Post or Page, because that won't allow users to search for other types (if we restrict to Post users won't be able to search for pages, if we restrict to Page they won't be able to search for categories, etc.); the default was set to custom link to allow maximum flexibility.

I'm not sure exactly what's happening, but I'd expect the missing text we're setting here to be respected for all cases, not just taxonomies.

Fwiw my testing shows post and page types a bit different from yours after reload: instead of #474 (no title) I'm just seeing "Untitled". But if I assign the menu a location, the menu items will display as "Untitled" on the front end, and a link is generated using the menu item number as if it were a post id, e.g. http://localhost:8888/?p=474.

Another interesting thing I'm noticing is that, when an empty custom link or submenu is present, post, page and taxonomy type links are multiplied on reload if we press "Save" multiple times. So, if I insert one custom link, one page and one category, and press "Save" three times, on reload I see three pages and three categories. Not sure if this is related at all 😕

[gwwar]

I'm pretty flexible with what folks decide to go with, provided that the behavior is consistent. 👍 I think these cases might be a little easier to debug if there's a code-view equivalent in the navigation editor that shows the menu item serialization, but otherwise it should be doable by observing the API requests.

So, if I insert one custom link, one page and one category, and press "Save" three times, on reload I see three pages and three categories. Not sure if this is related at all

Sounds like a similar category issue. It's mostly likely due to to logic here:

gutenberg/packages/edit-navigation/src/store/actions.js

Lines 196 to 204 in c8148c2

	/**
	* Updates every menu item where a related block has changed.
	* Sends a batch request with one PUT /wp/v2/menu-items for every updated menu item.
	*
	* @param {Object} navigationBlock A navigation block to find updated menu items in.
	* @param {number} menuId Menu ID.
	* @return {Function} An action creator
	*/
	const batchUpdateMenuItems = ( navigationBlock, menuId ) => async ( {

I'm guessing that we send the same batch request 3 times. This can probably be avoided with optimistic updating on the client (where we assume the save worked, and only modify state otherwise if we see an error response).

[Mamaduka]

Closing the issue since the editor is no longer maintained and has been removed from the project.