Size Change: +983 B (0%)

Total Size: 1.07 MB

_{compressed-size-action}

Thanks for taking a stab at this. I'm tripping myself on some of the testing steps, and find myself unable to select a previously created menu. What am I doing wrong in this GIF?

@jasmussen That's interesting. You should be seeing a two-step block placeholder, first step should look like this, similar to when a template part is added:

@jasmussen That might happen if your code isn't re-built for some reason, maybe the build failed? It looks like you have the PHP changes but not the JavaScript changes.

Thanks, I'll give this another shot. Back in a moment.

This is an impressive PR:

Whether data should be saved as a custom post type or as a template part, I'm unqualified to chime in on. Focusing purely on the navigation block part of it, the primary effort would be to integrate a menu picker into the existing setup state so we end up with a one-step placeholder. I'll note down a task to do some mockups here.

Took this for a spin and just like @jasmussen I am amazed how impressive it is. I did not have to figure it out at all, it just worked. The one problem I found is that the front end does not render the menu I see in the Site Editor. Is this known or am I doing it wrong?

I see this in the editor:

And this rendered:

This one looks pretty interesting @talldan! Nice work. ✨

Things that need fixing

❌ From testing I believe top level attributes on the nav block like vertical variation aren't being saved. Would it be simpler to insert a block reference, and have the navigation CPT store both the nav block and the inner-content?

Context on Flow

So for folks reviewing, this PR plays around with the serialization of the block. Instead of storing link content inline, we instead create a nav CPT, and simply link to the post id:

Nav block points to the CPT id	CPT content stores the inner content (everything but the nav container)	If we edit 3 Nav blocks, we see these queued up for save on publish

In terms of workflow, there's a new step when adding a nav block, we give it a name. I understand that we're still prototyping here. Some suggested feedback for the new flow:

• To make this a little friendlier we might want to suggest something more memorable to start as a placeholder prompt like "Header menu" or "Nav menu" etc. Since each is backed by a CPT, it'd be ideal to also show the navigation menu name in ListView and when we select the block. (Similar to how we show named headers).
• Step 2 likely needs some copy changes or some 🤔 After naming a menu it's confusing to pick from other named menus. Maybe something like populate, or start from? Can we also start from content from other CPT nav menus?
• More of an enhancement for another PR but I wonder if at step 2 we can show some pattern placeholders w/hover preview. It'd make this step less intimidating.

Questions from summary

Handle existing navigation blocks in content. Offer a path for saving their inner blocks?

There's a lot of similarities here for saving navigation content with a CPT and reusable blocks. Provided we are pretty clear on what is backed by a CPT and what's saved inline to a post, I think it might be a simple as showing some actions like "convert to reusable menu" and "convert to regular blocks" (or whatever we decide to call these sorts of nav blocks).

Decide what to do about the wp-admin view of navigation posts. Maybe hide it and then follow up with work on a way to edit in isolation?

Probably a good opportunity to edit these CPTs with a block based editor like the nav editor. The post editor is serviceable enough in the meantime, but folks might need some explanation of what it is. We could maybe add a filter for this CPT, and add a wrapper nav block that doesn't get serialized? Or perhaps save the entire nav + contents in one place.

What should we publicly call the user facing post type for a navigation block?

There's already "Menus" so how about "Block Menus"? No strong feelings on this one.

Misc thoughts

One thing that comes to mind is that if we use a CPT, there's very little difference here between backing a nav block with a CPT vs one with an existing menu. The latter would likely need a simple adapter, and some extra logic limiting the type of inner blocks we can add.

Overall, I think the idea here is pretty promising. It be great to get some feedback from folks more familiar with how folks prefer building sites. Perhaps a +1 review from folks more familiar with themes, and another from APIs?