[Draft] Use template parts to preserve navigation between theme switches #36117
Conversation
…g the navigationMenuId attribute from the old menu to the new one
adamziel
requested review from
talldan,
noisysocks,
youknowriad,
draganescu and
getdave
adamziel
changed the title
adamziel
changed the title
|
This looks very smooth to me! 👏🏻 |
| @@ -189,6 +192,16 @@ function gutenberg_get_allowed_template_part_areas() { | |||
| 'icon' => 'header', | |||
| 'area_tag' => 'header', | |||
| ), | |||
| array( | |||
| 'area' => WP_TEMPLATE_PART_AREA_PRIMARY_MENU, | |||
There was a problem hiding this comment.
There could be an area per semantic menu type, or it could be more of a new, free field that can take any value..
|
Lovely that using semanting template parts (via areas) we can get rid of the naming convention. With the The UX problem here is: how do I figure out that it's not the "header" itself that is "primary navigation" but the navigation template part inside that? |
I'm not sure I follow. What are the steps from "I open site editor" to "ughh I'm confused" here? |
|
I added an alternative idea to this PR where "injections" are used. In short, this would be the template part file: <!-- wp:navigation { "initialNavigationMenuArea": "primary-menu" } -->Saving it stores |
| @@ -242,6 +242,16 @@ function render_block_core_navigation( $attributes, $content, $block ) { | |||
| $inner_blocks = new WP_Block_List( $parsed_blocks, $attributes ); | |||
| } | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
This could use an inline comment to explain what it does.
| @@ -11,6 +11,10 @@ | |||
| ], | |||
| "textdomain": "default", | |||
| "attributes": { | |||
| "initialNavigationMenuArea": { | |||
| "type": "string", | |||
| "default": "" | |||
There was a problem hiding this comment.
maybe we don't need the default here?
| /** | ||
| * TODO: Make changing nested entities affect parent entities in Gutenberg | ||
| * example: | ||
| * <!-- wp:template-part {"slug":"header"} --> | ||
| * <!-- wp:template-part {"slug":"primary-menu","tagName":"primary-menu","className":"primary-menu","layout":{"inherit":true}} --> | ||
| * <!-- wp:navigation { "navigationMenuId": {primary-menu} } --> | ||
| * <!-- /wp:template-part --> | ||
| * <!-- /wp:template-part --> | ||
| * | ||
| * If I set navigationMenuId to something else, it should save the primary-menu template part. Currently it doesn't and it seems like a bug. | ||
| */ |
There was a problem hiding this comment.
This should be an issue, not a @todo comment
|
This ended up being implemented as a Navigation Area block in #36178, so I'm closing this PR. |
This PR preserves the navigation menu between theme switches as described in #36087.
Here's a demo:
CleanShot.2021-11-01.at.15.21.47.mp4
Upsides:
This PR reuses the template parts system so users don't need to learn new mental models. The PR doesn't change the user interface in any way. Anything supported by template parts just works, e.g. the isolated template part editor. Also, the change is relatively succinct and self-contained. It introduces the concept of "applying attributes on theme switch," but not much besides that.
Downsides:
<name>-menu.html). This can be mitigated by listing these template parts explicitly in theme.json.Missing:
Test plan:
Mayland (blocks)and cloned it asMayland (blocks) 2primary-menutemplate part in header.html (e.g.<!-- wp:template-part {"slug":"primary-menu","tagName":"primary-menu","className":"primary-menu","layout":{"inherit":true}} /-->)primary-menu.htmlfile inblock-template-partsof each theme. Add a navigation block to each:<!-- wp:navigation { "openSubmenusOnClick": false, "orientation": "horizontal", "overlayMenu": "mobile", "showSubmenuIcon": false } -->"area": "primary-menu"