Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation block - preserve navigation block data on theme switching #35750

Closed
Tracked by #35521
talldan opened this issue Oct 19, 2021 · 11 comments · Fixed by #36178
Closed
Tracked by #35521

Navigation block - preserve navigation block data on theme switching #35750

talldan opened this issue Oct 19, 2021 · 11 comments · Fixed by #36178
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.

Comments

@talldan
Copy link
Contributor

talldan commented Oct 19, 2021

What problem does this address?

This issue is a follow-up to #34612. Currently, that issue has two PRs exploring solutions, one using template part post types (#35418) and another using a separate wp_navigation post type (#35746).

When switching themes, it'd be good to try to preserve navigation block menu data. The previously mentioned issue explores saving navigation block data to a post. This would make it possible to retain that data when a user switches themes.

There are some different permutations
a) migrating from a classic menu (associated with a location) to a navigation block in a block-based theme (associated with a template part)
b) migrating between block-based themes

One of the challenges is that in a block-based theme the Nav Block is just rendered in a template part as HTML. It doesn't have the concept of a 'location'. This might be something we need to implement.

@talldan talldan added [Type] Enhancement A suggestion for improvement. [Block] Navigation Affects the Navigation Block labels Oct 19, 2021
@talldan
Copy link
Contributor Author

talldan commented Oct 19, 2021

One problem is that the current implementations of #34612 all associate an instance of a navigation block directly with an instance of a navigation post. That is, the block has a postId attribute. When switching from a classic theme to block theme, there isn't an existing post type, one will have to be created. When that happens there's no easy option for re-writing the HTML of the navigation block that's rendered in a theme's template to ensure it has the correct postId.

Instead, maybe something more like the current menu system is needed. A theme defines locations (potentially the template part 'areas' are re-used), and the theme developer, when adding a Navigation Block in a template assigns that block to a location.

When a user then edits the Navigation Block, the resultant post isn't associated with the instance of the block itself (the block doesn't have a postId attribute now), instead the block has a location attribute, and the postId associated with that location can be more easily updated.

I think this is what #31971 tries to do for template parts, but I'm still trying to understand some of the technical details of that PR.

@draganescu
Copy link
Contributor

and the theme developer, when adding a Navigation Block in a template assigns that block to a location.

Yes, and that attribute should only be used for migration, right? Otherwise it would cramp users of the block editor from putting a navigation block wherever they want, I think. Or would this be a free text attribute? But even so, moving the block from header to footer but leaving the location to "top menu" would be very confusing.

There are at least two things about navigation persistence that I try to figure out:

  1. Location discovery
  2. Data migration

Location discovery: everything in @talldan 's #35750 (comment) for classic to block, plus the block to block problems because we don't have a way to see where a certain navigationPostId is used in the current theme before switching to a new theme.

Using theme mods (#31971) would allow us to store a relationship between navigationPostId and a template part slug, making it easy then to migrate them to a new theme on switch.

Data migration: when moving from a classic to a block theme, we'll need to copy the classic menu to a block menu and write a Navigation post. But this process is not fully implemented yet as there are classic menu features that are unclear if they can be migrated to blocks (custom fields and custom menu item types). I think an initial path is to discard what is not representable as blocks yet, considering the original classic menu remains untouched?

@getdave
Copy link
Contributor

getdave commented Oct 25, 2021

Going to start documenting what I'm learning about this topic. Will update this post as I go:

Current Menus Behaviour

TL;DR

  • Menu Locations are arbitrary strings stored on a per Theme basis (e.g. primary, secondary but also flibble-wibble).
  • Menu Locations have some convention of primary, secondary but this is not standardised. Some heavily used Themes use location based strings (e.g. footer).
  • Menus and Menu Locations are associated via key/value pairing in a Theme Mod.
  • Menu -> Menu Location associated may be carried across between Themes if the Menu Location string in the current theme exactly matches a Menu Location in the Theme being switched to.
  • Otherwise all Menu Locations are not carried between Themes.

Registration and Rendering

  • You register Menu Locations as follows as follows:
register_nav_menus(
    array(
        'primary' => 'Primary menu',
        'footer'  => 'Flibble Wibble menu', // can be anything...see?
    )
);
  • You output/render a Menu as follows:
wp_nav_menu(
    array(
        'theme_location'  => 'primary',
    )
);

This relies on logic for retrieving a menu by its location.

Data Storage

  • Menu Locations are completely arbitrary strings stored in a Theme Mod - this is an option with the key of theme_mods_{{YOUR_THEME_SLUG}}. As a result of this storage mechanic menu location slugs are not preserved across themes.
  • A Menu itself is stored as a Term of type nav_menu. Therefore it has a term_id field to serve as its identifier.

Screen Shot 2021-10-25 at 11 22 22

  • The association between Menu (term_id - see above) and Menu Location (theme mod) is stored in the Theme Mod for that Theme. For example here is the an excerpt from the Theme mod for my active TT1 theme:
// theme_mods_twentytwentyone
s:18:"nav_menu_locations";a:2:{s:7:"primary";i:14;s:6:"footer";i:42;}}

Here "primary" is associated with Menu ID 14 and footer is associated with 42.

Theme Switching Behaviour


Template Areas


@getdave
Copy link
Contributor

getdave commented Oct 25, 2021

Desired Behaviour / User Stories

I'm going to document below what I believe are the behaviours we want from navigations when switching Themes. I'm trying to represent these as user stories (As a admin, when I X, it should see Y):

User Stories

User: website Admin

  • As a website admin, when I switch Theme (either block or classic), I should (where possible) continue to see my existing navigations in the new Theme in most appropriate area by default.
  • As a website admin, after switching between Block Themes, I should see a means to select a different navigation to display in this area if I wish.
  • As a website admin, if I have a navigation assigned to a navigation area, when I switch Block Themes, if my new theme has no matching navigation area, I should see a warning prompting me to configure my navigations.
  • As a website admin, if I have a navigation assigned to a navigation area, when I switch Block Themes, if my new theme has no matching navigation area, I should see a (fallback) list of top level pages (max of 6) displayed on the front of my site.
  • As a website admin, when I switch from a classic to a block theme, I should see block-based navigations automatically created for any Menus that are assigned to a Menu Location which can be mapped to a matching navigation area in my new Theme.
  • As a website admin, when I switch from a classic to a block theme, if there are no Menu Locations which map to a given navigation area in my new Theme I should see a prompt requesting that I configure my navigations.

User: Theme Developer

  • As a Theme Developer, I should see a means to designate certain areas of my templates as being distinct "navigation areas".
  • As a Theme Developer I should see predefined navigation areas provided by WordPress Core (e.g. primary, secondary, tertiary).
  • As a Theme Developer I should have a predefined schema or well-codified convention for naming my navigation areas should I wish to add new ones.
  • As a Theme Developer I should be able to programmatically retrieve the navigation (i.e. the menu items from the CPT) currently assigned to a given navigation area.
  • As a Theme Developer I can designate a portion of my template as a navigation area via controls on the navigation block.
  • As a Theme Developer my navigation area is not bound to any given navigation block instance (i.e. it is purely for configuration purposes).
  • As a Theme Developer I have the option to add a navigation block without designating it as a navigation area.
  • As a Theme Developer I do not need to add navigation items in order to create a navigation area.
  • As a Theme Developer I should not be able to assign more than one navigation to a navigation area.

Terminology

  • "navigation" - a set of menu items (only), likely persisted to a Navigation Custom Post Type.
  • "navigation block" - the wrapping core/navigation block. Concerned with presentation not menu item data.
  • "navigation area" - (theoretical) an area of a template designated as a location into which navigations can be assigned.

@adamziel
Copy link
Contributor

adamziel commented Oct 25, 2021

As a website admin, if I have a navigation assigned to a navigation area, when I switch Block Themes, if my new theme has no matching navigation area, I should see a (fallback) list of top level pages (max of 6) displayed on the front of my site.

Would you elaborate on this one? I don't quite follow.

As a website admin, if I have a navigation assigned to a navigation area, when I switch Block Themes, if my new theme has no matching navigation area, I should see a warning prompting me to configure my navigations.

At the risk of sounding controversial: What are the downsides of migrating and assigning the old menus at "random"? Random could mean that the menu with most items goes to the footer slot, or the most clicked one goes to the primary slot. Are there any chances this would cover > 50% sites and provide a sensible defaults for the rest?

@getdave
Copy link
Contributor

getdave commented Oct 25, 2021

(Dave) As a website admin, if I have a navigation assigned to a navigation area, when I switch Block Themes, if my new theme has no matching navigation area, I should see a (fallback) list of top level pages (max of 6) displayed on the front of my site.
(Adam) Would you elaborate on this one? I don't quite follow.

Certainly. What I'm trying to convey is that if it isn't possible to map a Menu Location from the Classic Theme to any of the navigation areas in the new Block Theme, then we need to avoid a completely "broken" experience on the front end of the site. What I'm suggesting is rather than render an empty navigation, we simply render the top 6 Pages on the website as a fallback. We would also surface warning (as described in the other Stories) to the user to alert them to the need to manually configure this Menu.

At the risk of sounding controversial: What are the downsides of migrating and assigning the old menus at "random"? Random could mean that the menu with most items goes to the footer slot, or the most clicked one goes to the primary slot. Are there any chances this would cover > 50% sites and provide a sensible defaults for the rest?

I feel like heuristics around this could be troubling. I don't think we could extrapolate that because Menu A has more items than Menu B then it deserves to be in location A or B.

My idea was that we can consider the two scenarios somewhat differently:

  • Migration Block Theme to Block Theme - use new standards to maximise seemless transition.
  • Migration Classic Theme to Block Theme - try to achieve seemless but acknowledge that isn't always achievable and instead provide adequate fallbacks and warnings.

In the former we can start to actively codify new best practices by providing the tools to do so (e.g. standardised default navigation areas plus ability to assign navigations to navigation areas via the UI). This helps to smooth cross Theme transitions.

In the later case, we can look to provide the best possible experience (i.e. "best fit" mapping of Menu Locations to navigation areas) whilst still acknowledging we cannot provide the perfect solution given the disparate nature of the existing Menus system. Indeed, in order to mitigate the impact of transition, we should be sure to provide excellent fallback experiences and actively surface potential Menu problems to users via the UI.

@talldan
Copy link
Contributor Author

talldan commented Oct 26, 2021

As a Theme Developer I should see predefined navigation areas provided by WordPress Core (e.g. primary, secondary, tertiary).

Template parts do already have an 'area' concept of their own (the wp_template_part_area taxonomy), and it might be that the navigation block can use those for its locations. They're typically 'header', 'footer', 'sidebar':

function gutenberg_get_allowed_template_part_areas() {
$default_area_definitions = array(
array(
'area' => WP_TEMPLATE_PART_AREA_UNCATEGORIZED,
'label' => __( 'General', 'gutenberg' ),
'description' => __(
'General templates often perform a specific role like displaying post content, and are not tied to any particular area.',
'gutenberg'
),
'icon' => 'layout',
'area_tag' => 'div',
),
array(
'area' => WP_TEMPLATE_PART_AREA_HEADER,
'label' => __( 'Header', 'gutenberg' ),
'description' => __(
'The Header template defines a page area that typically contains a title, logo, and main navigation.',
'gutenberg'
),
'icon' => 'header',
'area_tag' => 'header',
),
array(
'area' => WP_TEMPLATE_PART_AREA_FOOTER,
'label' => __( 'Footer', 'gutenberg' ),
'description' => __(
'The Footer template defines a page area that typically contains site credits, social links, or any other combination of blocks.',
'gutenberg'
),
'icon' => 'footer',
'area_tag' => 'footer',
),
);
/**
* Filters the list of allowed template part area values.
*
* @param array $default_areas An array of supported area objects.
*/
return apply_filters( 'default_wp_template_part_areas', $default_area_definitions );
}

So maybe when a nav block is within a template part that defines the header, the nav block could automatically assume the 'header' location.

There are some problems to think through:

  • what if there are multiple nav blocks in a template part with an 'area'?
  • what if a template part with an 'area' is nested in another template part with an 'area'?

Still a good avenue to explore, I think.

@draganescu
Copy link
Contributor

@getdave this looks like a great summary. Thanks for documenting the classic navigation preservation mechanism 👏🏻 In terms of userstories, maybe they're a little too detailed to start with? The exploration goal is split in two right now:

  • how can we preserve navigation swithching from classic to block themes? Can we test mapping location to X in block themes and see if that works, using the same rudimentary string matching for now. What would X be?
  • how can we make navigation work when switching block to block themes? If we rely on X above, can we make the heuristics better?

Then we can dive into how theme building implements this and so on.

@getdave
Copy link
Contributor

getdave commented Oct 27, 2021

As an initial exploration let's proceed with the following:

  • Navigation block will define an attribute "area".
  • This will be a choice of 3 values - primary, secondary, tertiary.
  • Add UI to Nav block to allow Theme Developer to select an area.
  • Add logic to ensure only a single Nav block can occupy a given "area".
  • When the Nav block is assigned an "area" then it updates the assigned Navigation Post (which provides the items) to also be assigned to that "area".

@noisysocks
Copy link
Member

@adamziel @talldan @getdave: How do you feel about punting this from WP 5.9? Given both #36002 and #36117 are a work in progress, I’m not sure how feasible it is to get this in by Friday. It seems that there's still some exploration to do? I think exploration and iteration is generally best done in the Gutenberg plugin where things aren't rushed.

@adamziel
Copy link
Contributor

adamziel commented Nov 3, 2021

@noisysocks I think it's an important feature and I'd say let's keep fighting :-) there's #36178 now that could be it. It misses a few things but @talldan will take it for a spin while I sleep. Let's see how far we can get.

@noisysocks noisysocks linked a pull request Nov 3, 2021 that will close this issue
@noisysocks noisysocks assigned adamziel and talldan and unassigned getdave Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
5 participants