(Optionally) rendering classic navigation data source in Navigation B… #30852
Conversation
…k-nav-menus' magic instead)
jffng
requested review from
ajitbohra,
spacedmonkey and
TimothyBJacobs
as code owners
jffng
added
[Block] Navigation
|
I am very torn about this because while I think this is pretty smart, I feel that there is too much focus on temporary solutions and that this is doing theme authors a disservice. This only works if you also have access to the previous navigation menu in the admin area where you could assign menus to theme locations, and this menu is currently hidden, even with the PR applied. I also think that the term theme location should be used over "source". The classic way of presenting menus is something we should put behind us not least because of the masses of broken inaccessible menus and menu walkers that theme authors use today. |
💯 with you on this. But the issue is, we need to provide a solution from the old way to the new way. You are right that a block theme using this will not be able to update the menu with the old navigation menu, but what about a classic theme that tried to implement the navigation block using |
|
Then they could use the block with the block settings, for example a reusable block, not the old menu. They are not using the block if the output is using the classic function, the themes existing JS and CSS. |
|
And if this is not a version of WordPress where the edit template link in the editor is also disabled, then users have access to changing the navigation block directly and using the "use existing (menu)" option. |
|
Some more context:
To make universal themes a reality we need a way to be able to edit navigation from the Customizer as well as from the site editor. We could move the navigation to a template part and have the user edit their navigation in a template part, but for users who are used to using the Customizer it feels like a big ask to get them to edit their menus in a whole new place. Note: with @mtias @youknowriad I'd love to get your thoughts on this. |
|
Besides the Universal themes angle, I think it's particularly helpful for a user in a world where both classic and block themes will be at their disposal that they don't have to change their menu every single time they change themes if they are looking for a new one. This tool would help with that a lot and it's not imposing that you use "the old way" of editing a menu, it's just mapping what was already there so the user doesn't have to do it manually if they migrate from a classic theme to a block one. |
It'd be good to have a more thorough description of the problem this solves, as it'll help others join the conversation. The nav block does already have an option to pull in the content from a classic menu, so is the issue more around the editing interface for the block? |
The aim for universal themes is to be able to use different UIs to edit the same content. For example with navigation, it would be great to be able to edit the navigation in both the Customizer and the Site Editor. |
|
But is there any actual user testing that supports either way? |
The goal is for universal themes to work in either of those interfaces but not both at the same time, as that could be quite confusing indeed. Block themes would allow editing certain content through the Template Editors (the Site Editor or any future iteration over it as seen in #29630) while classic themes would allow editing the same content through the Customizer, similarly to how the Navigation is edited as a block in the Site Editor when using block themes or through the Navigation Editor if a classic theme is active, but both interfaces can't be used at the same time. Another benefit of these universal themes is they would work with older WordPress versions that don't support block themes. |
|
It sounds like a challenge would be how WordPress migrates the classic content over to FSE content when a user upgrades their WP version. This change proposes using classic menu data to drive the navigation block, but how does a navigation block with the right |
@talldan thank you for this feedback, I updated the description based on these conversations! |
|
Thanks @jffng that's a really helpful description now 👍
As The implementation is here at the moment - https://github.com/WordPress/gutenberg/blob/trunk/lib/navigation.php#L248-L315. At a quick glance I can see not all the link attributes from menu items are being converted, only the URL and label, so things like 'open in new tab' might not work. For the purposes of this PR it might be an option to decouple this from the nav editor, migrate this code over to the block itself so that the theme opt-in isn't needed.
As you mention in the description, some thought will need to go into block editor experience when the block is configured like this, as I imagine this would be the default setup in a theme. What's mentioned in the description sounds a lot like how the Page List block works, so it might be worth checking that out. That block is also not editable and can be converted - #30390. |
|
Thanks for the PR and conversation. It's the best way to move things forward. And thanks for the ping, Dan! Some of the technical aspects I will defer to the other wonderful folks pinged. I'd like to touch on this bit:
Ultimately, the Navigation block is just that: a block, and having a dedicated screen (or section of the customizer) to edit just a single block is not a key part of where I personally see full site editing going. Inserting the Navigation block directly in your header area, in full context of its surroundings and with access to responsive tools, I see as the best interface for editing your navigation. When menus grow big enough to be unwieldy, that's where I see the List View coming into play for sorting, organizing, and arranging huge amounts of items. It's one of the reasons why so many list view improvements are added to the tracking issue: #27593. I think of List view as a miniature version of the navigation screen, but able to be leveraged by any block that's complex enough to benefit from it. That's the key bit: in embracing the site editor as the context for editing all content of your site, we also embrace the block interface, and any improvements we make there benefit all blocks. The re-used header area ensures the same block is used across your site. You can even wrap it in a Reusable block container if you need it in the flow of your content. In addition to that, the Page List block (as Dan notes) was created to show a dynamic version of all the pages of your site, keeping them in sync with new publications or deletes. So whether the dedicated navigation screen (or section of the customizer) remain available indefinitely for all types of themes or not, my approach to improving the Navigation block is based fully on the idea that the best experience will be editing the block in its natural context. |
|
|
||
| if ( ! current_theme_supports( 'block-nav-menus' ) ) { | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
What is the role of checking for this here? The expected use of this supports is kind of the opposite, as here the result is rendering a classic menu, while the supports is meant to enable rendering of a block menu.
There was a problem hiding this comment.
The purpose isn't to render a classic menu, the purpose is to render a BLOCK menu with a classic data source. That flag ensures that what comes out of the call to render the menu is navigation block markup. Ideally it would be preferred to not require that theme supports and simply (in this situation) ALWAYS render block markup. But this was the cleanest path to done at the moment.
| } | ||
|
|
||
| $block_attributes = $attributes; | ||
| unset( $block_attributes['location'] ); |
There was a problem hiding this comment.
I don't follow the swapping values and the unset. It's probably related to the updates in wp_nav_menu but at least it needs a comment, and ideally we don't need to do it.
There was a problem hiding this comment.
When crafting a Navigation Block that leverages a classic navigation data source you will still be able to apply stylistic attributes (justification, font size, etc). These attributes should be passed in when it actually renders. HOWEVER if the 'location' attribute is set when the rendering is attempted, and there are no menu items to be rendered, the block enters a loop. This ensures that the attributes passed to actually be rendered exclude the 'location' attribute preventing the loop.
There was a problem hiding this comment.
It looks like a problem to be fixed in the block then :)
Just some context that I'm fixing all of this parsing/serialization in #31004
That's already fixed in #30956 |
|
Firstly, thank you for this PR 🏅 . It looks really good and it's great to see the discussion around this as well. These kinds of issues are critical to ensuring we ensure users have a seamless experience when upgrading to FSE. My understanding of this proposal is that the user can continue to edit their navigation within the Customizer and then see those changes reflected (automatically) in the This seems like a reasonable proposal until we consider that this appears to go against the route we've been taking with the existing Menus screen (ie: We provide an "upgrade" route for users coming from an old Theme by either:
Whilst I appreciate the motivation behind allowing users to continue using the interface they feel familiar with (eg: Customizer) to edit their navigation, this feels antithetical to the motivation of FSE which is to get users over to the (some would argue) superior UX of the blocks system. As with the old Menus screen, I would expect the Indeed, by providing the user with an option to continue editing using an old interface we are disincentivising them to upgrade. I would suggest this is doing them a disservice as in the long term the block based version of the Navigation will ultimately continue to gain improvements in UI and functionality which will far exceed (🤞 ) that of the Customizer based experience. Folks have mentioned that the block doesn't currently parse/serialize menu data to block form correctly. This is (currently) true but that will be resolved in #31004 (reviews appreciate) so soon we should have upgrade paths ready for users who are used to editing menus in the Customizer, namely:
A technical solution is a nice approach but I can't help but feel it's going somewhat against the grain of streamlining the experience of allowing users to upgrade to a block-based world. Happy to be told I'm wrong however! 😄 I'm new to this so I felt a fresh perspective might be helpful. Thanks again for the PR and the discussion. |
| if ( isset( $args->block_attributes ) ) { | ||
| $block_attributes = $args->block_attributes; | ||
| } |
There was a problem hiding this comment.
We could probably just do this inline with the array below?
That's absolutely right. Not as a long-term solution for navigation but to get us through this transition process.
The MAJOR MAJOR motivator is is that we (or maybe just me...) are trying VERY HARD to separate 'Block Based Theme' from 'Full Site Editor'. You have to have a Block Based Theme to take advantage of the Full Site Editor. You do NOT have to use the Full Site Editor to take advantage of building Block Based Themes. (Or, at least we are VERY CLOSE to making this possible. Managing navigation is one of the very last remaining hurdles.) If a user is using this block in the Full Site Editor I would expect them to use the Navigation block (complete with upgrade path) how it's being built today. However THE MOST IMPORTANT THING WE ARE ATTEMPTING here is to use the Navigation block in a block-based theme WITHOUT the Full Site Editor. If a user doesn't have access to the navigation block AT ALL then managing it via the customizer (or kin) is the only way they can. |
|
Thanks for your detailed response @pbking 👍
That context is useful. If the motivation is to avoid the Full Site Editor then can I ask why we're not looking to use the dedicated Nav Editor screen which is currently being developed? This is not the "full site editor" but a screen dedicated to creating a navigation menu. Do you see that being available to folks who don't want to opt into the full I appreciate the Navigation Editor screen isn't ready yet, but with more folks testing and working on the project it is moving forward. I'm curious as to why this would not be a preferable route to perpetuating the use of the Customizer when the user is using a block-based theme.
Could you advise on the scenario where a user is using a block based theme but does not have access to the Site Editor or the Navigation block? Again, I appreciate you being open to discussing my contrary opinion 👍 |
|
Ha ha. Pretty sure I'm the one with the contrary opinion. :P We're currently developing a theme that renders everything with do_blocks() from PHP templates in order to disable the FSE and still allow us to build themes with HTML templates. This is one example where implementing a navigation block with a location target would be useful. The block is inserted into a header template and assigned a location, but the user doesn't have the ability to edit that header template directly. It's a "classic" theme, but built with new tools. Ideally (I day dream about this one) the FSE is an optional thing that a theme can express support for. This way a theme could be built (very close to today!) using html block-based templates and be a USEFUL theme immediately, without waiting for the FSE to be complete and ready.
My understanding (which is often/usually flawed) is that the nav editor makes use of the navigation block, but still stores things in the same data source as classic navigation. It would be a great solution and (if I understand) there's no reason it couldn't be (one of the options) where navigation is edited. Indeed this PR leverages that work. |
I see. Like a sort of "headless" FSE. I can now understand where the motivation for this use case comes from.
Correct. If used within the Navigation Editor then it saves to the Navigation menu (Post Type: ‘nav_menu_item’). When the block is used within a FSE template however, it saves as a block.
My thought was just we're creating so many areas where folks can edit Navigation. I always saw the Navigation Editor as "deprecating" the existing menus screen and the associated customers variant. Perhaps @draganescu - who has been working on this far longer than I - will be able to clear up the intention here? |
|
Just wanted to bring up the fact that the connection between the Navigation Block when manually inserted, and chosen from an existing one defined on a navigation screen, is a one-way street:
It is meant as a shortcut to migrate existing menus, and once the navigation block is inserted, that's where it's edited. |
It's also a mechanism that needs to be repeated manually by the user should they decide to use a different block theme immediately after. |
|
One solution present is the "Page List" block which you get when you press "All pages" in the navigation setup. This is likely going to be the single block that we pre-insert in navigation block patterns, as it immediately gives you the structure of your site as a navigation block, and stays in sync with creations or deletions. |
That's a good point @talldan. Pretty much ALL of the solutions that we have explored involve using that method of rendering navigation markup; even the solutions that we have explored exclusively in the theme. What alternatives could we use to get from 'classic navigation data' to 'navigation block markup' without re-re-reinventing the wheel? The only thing that comes to mind is implementing the functionality of No matter the solution, I see that piece of logic as key to addressing the problem. |
|
@pbking I did mention it a bit earlier, but it might have gotten lost in the conversation - #30852 (comment) I think the best option would be to implement the rendering as part of the block. Hopefully that shouldn't be too much of a task—I think it'd be perfectly fine to duplicate the code here for the It doesn't have to be perfect for a first attempt, as long as it works and is no longer depending on the navigation screen. |
|
However THE MOST IMPORTANT THING WE ARE ATTEMPTING here is to use the Navigation block in a block-based theme WITHOUT the Full Site Editor. If a user doesn't have access to the navigation block AT ALL then managing it via the customizer (or kin) is the only way they can. To me this comment reads as if you are expecting the navigation block to only be available inside the site editor? I don't think that is the case? |
That's not quite my expectation, just that there will be times when the navigation block isn't available to the user to manage directly. There will be lots of scenarios where the navigation block is useful in locations that a user can manage themselves in the block editor and I don't argue that that workflow needs to be any different. However the navigation block isn't useful in its current state UNLESS a user can manage it directly. That means that it's not a solution that we can use to create assets a user doesn't have access to (such as a header template-part in a classic theme). That's the specific scenario this change is intended for. |
@talldan if we did migrate that code to be a part of the block instead is that could be shared/leveraged by the navigation screen? I would certainly be keen to see that refactored I would just be weary to ensure that we don't have two places trying to achieve the same goal (in potentially divergent ways). |
@pbking I don't think that's something to worry about. The biggest issue would be if Duplicating the code so that it's private code of the block isn't so much of an issue because it wouldn't be exposed as an API (the only thing known externally is the new block attribute). It could then be refactored at a later date to reduce the duplication if that becomes possible. |
|
Thank you all for the discussion so far. I'm going to try to untangle some string of thoughts as there's a lot going on.
Then also a few clarifications on where we want to go, since as @carolinan mentions early in the thread, the way menus work is convoluted and a big source of confusion which we are aiming to leave behind. In this sense, we don't want the old menu system to remain the canonical source of truth and we want to move towards the block representation in the smoothest possible way. In this sense, we want to encourage themes to start using the block and its customization pieces as soon as possible, and we want to update all available interfaces for menu editing to use blocks, at least for the UX. In reality, we cannot use blocks for menus in classic themes without some sort of opt-in because the chance of breakage is high, but we can use the block interface to streamline creation. That's the goal of the navigation screen project, so I won't touch further on it here. On the other hand, for new themes being built we want to leave that old system behind and embrace blocks. However, there has to be a smooth path for "migrating". That's what I see most appealing about some of the ideas proposed here. Not related to editing menus per se (which is a separate conversation) but about how themes can play a part in the transition. For that purpose I like the idea of using
In this case the presence of Regarding the editing experience, there are a few more things that need to be sorted out, namely:
|
There was a problem hiding this comment.
IMO, there is one thing which this PR should change: it should not have anything to do with the navigation editor. That means no mention of block-nav-menus or edits to the updates in the navigation rendering. It should only work to make the block render itself server-side differently (from source).
This attribute is something that makes the block always render from a location and whatever that location offers. It is an invisible setting to the user. It needs to be marked as unstable __unstable__location or something so that we take every caution to be able to change it in the future, depending on how the attribute will be supported for users.
It seems to me we are still unclear about what happens to a navigation block that has a location set when shown in a block editor: is it editable, is the location preserved on edit, can the location be changed by the user etc.? So, we cannot commit to this hence my idea to mark it unstable and only touching the block, not the navigation editor.
Is this feasible? Is it enough so that universal themes can move forward?
|
I'm keen for a prefix, I think that sounds like a good idea @draganescu and communicates the state of the feature. I also have the same questions as you regarding "what should it do when editing" and have the preference of "nothing different" right now. That gets us to a place where we can move forward building universal themes with an opportunity to flesh out the user experience in the future (if indeed there is any user experience). |
|
This PR was getting old and large so I created a simpler and smaller one here: #32491 |
|
Let's close this one in favor of #32491? |
Description
Background
With FSE coming in 5.8, we'd like to build production themes that leverage aspects of block-based theming that will be available in core — incorporating them with existing customization behaviors.
This is related to the concept of universal themes came up in this comment. There are a few uses cases for universal themes outlined in the comment.
The use case this PR explores is switching between navigation editing modes — classic / customizer-based to FSE.
Problem
The new way to create + edit a navigation is different and not synchronized to the old way. Users will continue to edit their menus using the customizer. Themes don't have a way to keep the contents of those "classic" menus in sync with a navigation block.
There may also be circumstances where sites need to remain on a classic saved menu for some time even if the rest of the site is FSE (because they might have some plugins interacting with nav, or because they want to restrict what users can add to the navigation).
Use Case
As a user, I expect the ability to edit my menu using the way I'm familiar with (the customizer) indefinitely.
As a theme developer, I'd like to be able to render those menus using the navigation block. Bonus if updating my theme to be fully block-based in the future won't break or cause the nav to disappear for my users.
Proposal
Add an attribute to the navigation block that specifies a theme nav location — pulling its menu items from this location and rendering them using the block (leveraging
add_theme_support( 'block-nav-menus' )).The FSE side of this proposal is not present in this PR, I think it could benefit from some design exploration.The PR also adds some functionality to the navigation block — when the nav block sets its location attribute, it locks the block's features to only allow the menu location to be changed. The user would be presented with an option to "Convert to Blocks" which would do what the existing "import from existing menu" functionality does today, and a link to edit the classic nav in the respective UI (customizer or nav editor). Once you "Convert to Blocks", the connection between classic and nav block is broken, and the menu is edited / saved using the new navigation modality.This was edited Apr 21, here is the original PR description
This PR makes it possible to create a navigation block that pulls its menu items from a navigation area defined by the theme, by setting an attribute — "source" — on the navigation block.
This would be of great help to transition from classic to block templates — a simple way to render the content of a classic menu using the navigation block.
How has this been tested?
Tested in Automattic/themes#3658
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).