Experiment: dedicated menu and menu item blocks for navigation editor #34496
Conversation
|
Thinking about this some more, I don't think a 'Menu' block is actually needed, the editor can just use menu items directly. I might consider removing 'Menu'. |
|
Size Change: +5.13 kB (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
|
I've implemented the placeholder from #23953 in this PR as it was very easy to do without the technical blockers of having to extend the nav block. |
talldan
force-pushed
the
try/dedicated-menu-block-for-navigation-editor
branch
from
9c5bff4
to
9c6f594
Compare
talldan
requested review from
Mamaduka,
getdave,
adamziel,
anton-vlasenko and
draganescu
|
I think this offers a lot of opportunities:
I think there could be some questions:
|
|
Thanks for exploring this 🏅. It works well at first glance. Here are some responses. I feel like we'll need to consider this very carefully...
I'm not clear on how/why it's not possible? What benefit does it bring saving only edited items? Would you say this is a major win or a nice side effect?
I see now that this PR drops the Nav Block entirely within the Nav Editor. Is this effectively a "Classic" Navigation block?
How is it easier? I'm genuinely curious.
How do they have better scope for support this? What specifically would be easier?
I agree the placeholder was much nicer 👍
I'm guessing this is mostly not having to disable all the "unwanted" Nav block features right?
I'm confused. Are you suggesting a Menu Item blocks as children of the Navigation block?
Only if we don't use the Navigation block and utilise this new Menu block instead. Otherwise we'll have to make sure Nav block changes work for the Menu block.
What happens if we decide to allow for block based themes to opt into block based rendering of menus. Wouldn't Themers now need to write styles for the Menu block as well as the Navigation block? It's another area of inconsistency and complexity to be considered. |
This is something that will be necessary before shipping to core, and this way will be a much cleaner architecture, the best option we have now is to diff each menu item for changes.
Yes, but I think we're at the point where the navigation editor changes the navigation block so much that it's unrecognisable from what you see in other editors, I don't think users will perceive much of a difference in how the editor works.
At the moment there are two types of validation that would ideally be implemented, client side validation (catching issue before sending menu items to the REST API) and the ability to show validation errors from the REST API. The navigation block could have its own client side validation (#31716), but there's no reason for this to have the same specification as what's required in the navigation editor because the block doesn't use the REST API by default. Showing errors from the server is even more difficult, the editor needs to hook into the link block which has limitations on what can be achieved.
It's just a matter of having full control over the block. Whereas we want to avoid adding too many backwards compatible features to the navigation and link blocks, because its purpose is to be used in full site editing.
Yes, there's that. The navigation block can also be cleaned up because there are a few places where the nav editor adds conditional logic to the block. The block styles are much much simpler.
I'm suggesting having no parent block. The navigation block in the navigation editor at the moment is entirely featureless and it adds extra UI complexity in terms of being another thing a user can select.
I don't fully understand this, could you go into more detail?
The menu block would only ever render using the walker system, outputting backwards compatible HTML. Themes already support this. Additionally themes could opt in to navigation block support, which is where a themer can add additional styles. When this is detected, the editor can offer the user the option to convert from a menu block to a fully featured navigation block. But one advantage here over the current implementation is that the user can opt to keep their menu the same. |
|
I like this a lot. I think the code duplication is a minor problem in this context. As the navigation block evolves more and more towards a full fledged mega menu builder, the code duplication will be less that the code abstraction we would otherwise need on top of the navigation block to keep if from doing unsupported things or breaking. Before we move on, what are we losing by not continuing with the navigation block as the infrastructure to edit classic menus? |
I think the main things are:
|
|
I really like the idea here of decoupling the blocks because there are different concerns and workflows that the navigation editor has that the other block editors do not. Trying to share code for different concerns tends to constrain how quickly we iterate (since we're adding complexity) and adds fragility as contributors are not aware of the use cases we need to handle (or when we sometimes forget to test new features in all environments).
Overall: Navigation Editor
Post/Site/Widget Editor
|
|
I think @gwwar has a good point. In the Navigation screen, we have to have feature parity with the previous one. The Navigation block, on the other hand, doesn't need to be constrained by this. So if the decoupling of blocks will let us iterate much faster, that's a big pro for me right now. Of course, we can always come back and start thinking about reusing components once both features are more stable. |
|
+1 For having two blocks here. I think trying to keep the existing block generic enough to work in both places, is just going to slow both projects down ( full site editing / navigation editor ). I also agree, saddling the nav block with all the backwards compatibility constraints doesn't benefit the nav block.
Consider my comment here. Handling custom fields, meta boxes, filter and actions, will require a lot of complex code that is only relevant in the navigation editor. A dedicated block would help with this. |
| */ | ||
| function register_block_edit_navigation_menu_item() { | ||
| $post_types = get_post_types( array( 'show_in_nav_menus' => true ), 'objects' ); | ||
| $taxonomies = get_taxonomies( array( 'show_in_nav_menus' => true ), 'objects' ); |
There was a problem hiding this comment.
At some point I would love to load this data via the REST API.
|
While I was initially a big fan of having a legacy menu block, I started to wonder if this PR indicates that we've strayed too far from the original goals of the project. Looking at our Navigation Editor tracking issue, which is currently prioritized around removing the experimental label from the project, this PR is very good at achieving that. Nevertheless, while this would help with our short term priority, it wouldn't help with our long term aims for this project. In fact, the exploration in this PR is a prime example of why we're trying to use the navigation block in the navigation editor: to figure out, via a real world implementation, the friction points and shortcomings of:
If making a classic menu block is the only viable solution to the above, it means we've hit quite a lot of these shortcomings, so kudos everyone, now let's improve them. Primarily the new block editors (widgets and navigation) started as refinement processes to power:
This approach here may unblock, and potentially speed up, the development of the navigation editor, but it has a few shortcomings:
In conclusion, I fear if we pursue this path we’ll lose insights for improving the navigation block and the block platform as a whole, plus introduce a host of duplicate efforts. I believe we're looking to build the best block based experience possible in WordPress and this needs to push the boundaries of the platform in new and exploratory environments that only projects like the navigation editor can create. The development speed gains from this exploration are not worth the trade off — so I suggest let’s close this out and focus on resolving the limitations of the navigation block and of the block platform. |
|
First, thank you for doing this exploration and writing out all the perils you've identified. I wanted to chime in here because I've been against a separate block for a long time and I still don't think it's a good solution but I have a clearer perspective of where the mismatch is coming from. Or, to be more precisely, I think if we break too early we miss on a series of important architectural considerations that we need to address in the navigation block through and through. For example:
In answering these I think it'd become more apparent that the characterization and the perception of a classic navigation block being intrinsically different is a bit off:
@talldan's comment about considering not having a "menu" block at all I think comes from perceiving part of the problem but describing it differently. I agree we don't need it for the classic case, but I'd also argue we don't need it for the block case when it comes to representing a menu structure in most situations. Let me articulate that better. The current parent wrapper of the navigation block — The container block should be opaque to its children when looked at from the perspective of the template that holds it. We just need to treat the inner contents as what gets saved separately and accessed independently of the navigation block container that wants to customize its look. In other words, we need to do the conceptual shift from: <!-- wp:navigation -->
...
<!-- /wp:navigation -->To this for most common scenarios: <!-- wp:navigation { id: "menu" } /-->The classic navigation screen concerns itself with the The template that includes a menu completely eliminates the need for a "location" because that becomes an implicit aspect of use, but it does absorb configuration aspects that are decoupled from the menu structure. For example: <!-- wp:navigation {"id":"menu","isResponsive":true,"fontSize":"medium"} /-->Can be used in a header template while a footer can avoid the responsive collapse and use a different font-size. The menu contents are shared. The inner contents could be shaped in a way that is not purely relational data driven (like a mega menu) in which case reuse is mostly a matter of template part reuse when it fits. I think we need to look at a next step of treating wp:navigation as an implicit template part (or different CPT, doesn't matter conceptually) first and foremost. I think some concerns will dissipate then. A dedicated screen would just be the equivalent of an isolated template part that discards the template customization to focus on the inner content arrangement. The transposition between current data and block-represented data is a much more analogous enterprise if you discard the parent for purely link-based navigations. It's still possible that once we do these initial steps the difference that will surface is between purely relational data (link items) and menus that mix some aspects of presentation (mega menus or menus with a site logo in between items). That difference would exist in a block theme context as well, so it's not a classic vs new distinction. |
|
I like the idea of a separation between the data part of a menu and the visual representation as part of making the navigation block reusable. It allows us to draw parallels between the full site editing use of the navigation block and the use in the navigation editor, which become very similar. I do think that could help improve the model that we're using. The tough part seems to be the backwards compatibility aspect of this model. A question is if the navigation block should be able to read and write classic menus or whether the classic menu should be read once and then saved from that point on as a navigation block (essentially migrating data). The first way is similar to what we do now and means maintaining a lot of technical complexity, something that has been challenging. The second is cleaner, but perhaps harder to maintain back compat and also means for users switching back to the classic menu editor the story becomes interesting. The classic menu editor could probably be made to read from navigation block menus without too much effort, there's already code to do the conversion, so there are options. |
|
I think two way sync compatibility would be unreasonable to maintain, as you say, and it's not a pattern we have used elsewhere before. For post content, once you go with blocks going back is not warranted to have consistent results; same has been the case for widgets. Consolidating how we store block related data is a good benefit for streamlining operations. The thing with navigation is that with this system we do have more options so we can offer more paths. For example:
|
I'd say that once the user opts into blocks for navigation other than However, if the navigation is all "simple" (just links and no other blocks) we can store as either blocks or menu items. This measure of backwards compatibility will allow the navigation editor to continue to fulfil a role as a nice "onboarding ramp" for Full Site Editing whilst that project is maturing. In terms of data storage and how we handle transforming the different data types...I wonder if we could look to normalize towards having a single unified "schema" for navigations. In this world we could represent the structure of a navigation in some kind of universal format (JSON or even HTML + block grammar). Then we could have different means of persisting that data:
This would mean we'd need to create some kind of transform layer ( That way we'd have a Navigation block that would only need to be aware of the universal schema and wouldn't need to care / know about classic Menu data structures. This would also nicely reduce duplication of the existing code for transforming menu items -> blocks (and the inverse) which currently lives in both the editor and the block. |
Migrating to blocks could be a one way path, but IMO only when the user switches from a classic theme to a block theme. I don't know if automatic upgrades, consolidating menu storage as block storage across theme types, are a way to go, even if the nested link blocks block fomat could be packaged to be perfectly digestible by any Walker. People may depend on the current DB menu storage, split across six tables :) I think the schema or similar approach proposed by @getdave above would be the way to find the common ground format between the JSON we get from the API and the html we get from the block storage. |
|
I think that, enabling blocks in navigation, will likely end up being a opt-in feature, via a theme supports. It is unlikely that existing themes will allow blocks, like search to be put into menus, as it the css / styling would not likely "just work". So we have to consider backwards compatibility here. Think about these user stories.
Again, I strongly recommend using term description to store block data, see #34612 (comment) for more details. |
I think for non-block themes it would definitely be bad to impose block html being rendered on the frontend, I agree with that, it would very likely become a backwards compatibility mess. At the same time, the navigation editor already depends heavily on the idea of translating menu items to blocks and back again. It could be possible to make a walker to work with link blocks by converting those blocks back into the menu item format before the HTML is output, and that would have better backward compatibility results. What would be difficult is catering for all the extensibility that can happen with menu items, things like custom fields.
Something that I hadn't considered at all, thanks for mentioning it. Let's continue the discussion on #34612 about that. |
Just to crosslink my comment #34612 (comment) where I suggest we move this transforming concern out of the block. |
To make sure we're all talking about the same thing here 😅 , would the "template part container" in this case be the Navigation block? Asking because we've been discussing these ideas here and are unclear on that point.
Wouldn't the current setup where the Navigation contents inherit context from their parent work? The link items themselves shouldn't have any style controls, but if styling is present in their container they use it. |
@tellthemachines My understanding is that would be the case, but I don't really see the connection to that comment of mine. |
This discussion is becoming super confusing 😅 You were replying to my question on whether the nav editor would be using the Navigation block or just its contents. Because the Nav block has all the styling options etc, there really isn't anything in the block itself that we need to use in the editor, at least data-wise. I may not have been fully clear with my question above. I should have asked instead:
Is the template part container used in block themes the same as the one used in "classic or focused modes"? Because I took |
Yes this is what I currently believe.
The implication for the Nav Editor is that it no longer utilises the This is very similar to the existing classic Menus screen which is only concerned with the structure/data of the navigation and not the presentation of the Navigation. |
Description
Something I've wanted to try for a while, is removing the dependency on the navigation block in the navigation editor.
Instead, this introduces a menu block and a menu item block, which is intended to correlate with the same functionality provided by the current WordPress menu editor.
This has been a bit of an exercise to see what gains there are from such an approach.
I've found it removes a lot of complexity.
The downside is quite a bit of code duplication, particularly for menu items / navigation links:
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).