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

Supporting future block types in the navigation editor. #30006

Closed
talldan opened this issue Mar 19, 2021 · 9 comments
Closed

Supporting future block types in the navigation editor. #30006

talldan opened this issue Mar 19, 2021 · 9 comments
Labels
[Block] Navigation Affects the Navigation Block [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@talldan
Copy link
Contributor

talldan commented Mar 19, 2021

What problem does this address?

At the moment the Navigation editor is designed to work with a very specific structure of the Navigation Block.

The expected structure is a navigation block with mostly nested Navigation Link blocks. When saving in the editor, those links are mapped directly to nav_menu_item post types—the same storage format used by the existing menu editor in WordPress. For backwards compatibility these menu items are rendered using the same walker system as present. Which ensures theme styles still work even with the new block editor interface.

It does support other block types when a theme opts in to this feature—Search, Social Links are a couple. These blocks are saved as html in a custom type of nav_menu_item.

But there are proposals to change the structure of the navigation block significantly. The potential future blocks that are relevant are:

  • The (proposed) Links List block. An empty container for any manually entered links.
  • The (still undecided) Submenu block. This would be the default way a user creates submenus.

These blocks are more challenging because they contain Navigation Link blocks. Some ad-hoc code would need to be written to be able to convert these blocks back and forth from nav_menu_items.

What is the goal

To make the navigation editor less susceptible to bugs that can occur from changes to blocks, while at the same time providing the navigation block with some scope for iteration.

What is your proposed solution?

My first thought is that we could drop the use of the navigation block in the editor, and build a separate classic menu block for the editor when a theme hasn't opted in. This would make it much easier to manage backwards compatibility.

A second option could be to consider using the transform system for converting from/to menu items, which could allow blocks to define declaratively how they should be converted from block data to menu items. It would be a way to transition from a system that's largely hard-coded and limited to one that's more dynamic, and could support a few different block types (possibly even third-party block types). But I don't think it'd be completely straightforward, the data structures are quite different!

And a third option could lie in the block itself, potentially in a basic mode of the block where it only supports links.

@talldan talldan added [Block] Navigation Affects the Navigation Block [Feature] Navigation Screen [Type] Discussion For issues that are high-level and not yet ready to implement. labels Mar 19, 2021
@talldan
Copy link
Contributor Author

talldan commented Apr 1, 2021

I've been exploring some of these options:

My first thought is that we could drop the use of the navigation block in the editor, and build a separate classic menu block for the editor when a theme hasn't opted in. This would make it much easier to manage backwards compatibility.

This still seems feasible and relatively easy to implement.

A second option could be to consider using the transform system for converting from/to menu items

I had a look at this idea, but I feel it will end up being a little bit of a stretch for block transforms.

  • A block tree isn't really a one-to-one mapping of a hierarchical menu. A links list block is just a wrapper with inner blocks, in terms of a menu it most closely represents an array of menu items. I thought about having a menuItemList transform type, but an array of menu items contains no metadata about what kind of block it should map to for a transform. So I don't think this is feasible for supporting various types of wrapper blocks. If there's only one supported wrapper block, there's not really any point.
  • Transforms generally work by matching against the top-level thing (block, html, file), and then any children are handled within that transform function by creating an inner blocks array. That concept doesn't really map so well to menu items, which can be very deeply nested. There's a need for a better way to handle recursively transforming menu items by walking a tree.

And a third option could lie in the block itself, potentially in a basic mode of the block where it only supports links.

This is also feasible, it generally places the onus on the block to provide backwards compatibility by maintaining a mode where only a few block types are supported.

@jasmussen
Copy link
Contributor

I shared some concerns in #30430 (comment) about adding yet another nesting level to the navigation editing experience. Keeping in mind blocks and nesting do not have to match HTML, how much of this stuff can we do server-side just for the rendered output? CC: @youknowriad as I recall you have looked at serverside tweaks lately.

@jasmussen
Copy link
Contributor

jasmussen commented Apr 6, 2021

I tinkered a bit in #23745 (comment), and found that allowing the Paragraph block inside dropdowns would be an excellent win, and that there might be a way for us to avoid additional wrapping containers (such as rendering the links list serverside only on the frontend, and avoiding a columns block for mega menus by showing a slider, a la the gallery).

@draganescu
Copy link
Contributor

In my head:

  • when classic themes (or hybrid/universal themes) opt in to display blocks in navigation locations we should not store the data in the old classic menu format, but instead store it as a CPT of type template/navigation.
  • opting in to blocks in navigation locations will display the render result of the navigation block, and in the navigation editor we should show and use the block without any configuration, just like it works in any other block editor (post/site/widgets).
  • switching back may be possible if we can have metadata stored in the CPT that shows the simple hierarchy of link data without design blocks.

To the point of this issue, the easiest way forward I can think of is to only allow link as child of navigation and always map to that. The structural updates that change how the link block is available - say, only inside a links list block - should be something that is worth updating the navigation screen for once. For the addition of a submenu block nothing will change in the navigation editor as we discard this option, unless the theme opts in case in which we use the full block without any configuration.

A navigation block that only allows link and link list as inner blocks + required parent (adding a link automatically adds a link list) = a classic menu block :)

@talldan
Copy link
Contributor Author

talldan commented Apr 16, 2021

For the addition of a submenu block nothing will change in the navigation editor as we discard this option, unless the theme opts in case in which we use the full block without any configuration.

How about if it becomes the only way to create a submenu? That seems to be what the issue proposes.

@gwwar
Copy link
Contributor

gwwar commented Apr 26, 2021

When backed by a nav_menu_item can we limit Navigation UX functionality to what the customizer currently supports (eg nested links)? If folks want new functionality like search, social links, spacer (or anything else we add), we'd then move the source of truth to block data. (No more bidirectional saving when folks choose that).

A second option could be to consider using the transform system for converting from/to menu items,

This is possible, but we'd need to be careful to make sure folks adding new functionality get tests ❌ to understand how all this behavior fits together. It potentially might be a lot of work.

Do folks know if we've outlined some basic user stories for how people will interact with building navigation menus? I'm finding it a little difficult to reason about some of the tradeoffs without this.

For example we have:

  1. A person who's using a block based theme, starts from FSE etc ...
  2. A person who has an existing site using X theme that supports Y, and wants to add an extra link ...

@getdave
Copy link
Contributor

getdave commented May 17, 2021

Updated comment

Having reviewed the way the Nav Editor works some of my original comment is incorrect. I have updated the Nav Editor documentation to account for this.

Essentially it's already possible for block types other than core/navigation-link to be saved as nav_menu_items in the DB. We do this by serializing the block data and persisting the items with a type of block.

See the README for full technical information on this process.

Original comment - some info may be incorrect

When backed by a nav_menu_item...

This will only ever be the case in the Navigation Editor screen right?

...can we limit Navigation UX functionality to what the customizer currently supports (eg nested links)?

This seems completely reasonable. If the Navigation Editor screen is designed to be a replacement for the existing Menus screen (just with the addition of a Block based UI) then there will be no expectations that additional block types (eg: social...etc) should be able to be inserted.

Trying to support these blocks and mapping to `nav_menu_item's feels like an unwinnable task.

Only allowing link blocks to be inserted in the Navigation editor screen may act as a great way to prompt folks towards using a fully block based Theme.

A second option could be to consider using the transform system for converting from/to menu items...

Let's say we have a Menu created using the existing Menus screen. There's no way to know what block types this should map to other than the standard core/navigation-link block (and it's variations).

For example, when parsing nav_menu_items to a core/navigation block, you can't say "here's a list of menu items, these should be rendered as a core/navigation-list block". This is because there's no programmatic means of determining that kind of mapping from the nav_menu_items - there is no metadata.

As a result, any attempt to serialize (on save) more complex block types such as core/navigation-list will be by definition lossy. This is because you can't map a "list of links" to a nav_menu_item (which is how Menus are saved). Therefore when you attempt to save you will lose the information that "this was a navigation list block" and thus when the Navigation Editor screen attempts to parse the menu back into blocks they will all be just standard core/navigation-item blocks.

@jasmussen
Copy link
Contributor

I created #33278 to suggest an expansion of allowed blocks in the navigation block, which seems relevant to this conversation.

@kathrynwp
Copy link

Closing this issue due to the Navigation Screen project being moved to an inactive status on the feature projects page in coordination with the project leads. (The developer documentation in the initial post are no longer accessible)

If this work is picked back up, issues can be revisited and reopened as needed. Thanks for pitching in on this early feature so the wider WordPress project could benefit from the lessons learned!

@kathrynwp kathrynwp closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2022
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] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests

6 participants