-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Preserve nav block data on theme switch #36002
Conversation
Size Change: +429 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
de62943
to
5e1a894
Compare
The current problem with this PR is no longer technical. It's UX. Unfortunately we seem to require separate workflows for
As a Theme developer I want to:
As a site owner, I want to:
The currrent UX of having a single dropdown on the Nav block itself for "Navigation area" does allow us to handle these permutations. For example, if the Nav block is already assigned as
Confused? Try out this PR - you'll soon see what I mean. At this point I'm stumped as to how to proceed. |
I think changing a navigation block's area is a pretty big operation. It changes the whole meaning of the block, and the user is probably also moving the block to a different place and wants it to be completely different. Or they've removed and are re-adding a block. So I think reassigning to different area is probably the right thing. It shouldn't cause any change in the posts that are assigned to areas, IMO. In classic menu terms, this would be updating the PHP code in the theme to render a different menu location. It might be that there's a menu assigned to that location already, and so that now shows. I realise these assignments are going to be a challenging thing to convey in the block. I think we should also support a nav block that doesn't have an area and just uses an ID, so removing an assigned area should be a thing too. |
Thanks for providing your input here Dan. It's much appreciated.
I think you're saying toggling the That concept is fine, but we're still stuck in that there is now no way to assign/reassign/unassign a |
// const updateNavEntityArea = async () => { | ||
// const noTerms = []; | ||
// // navigationAreaId is of type "number". Therefore | ||
// // ID: 0 is used to represent a value of "none". | ||
// const maybeNewAreaTermId = | ||
// navigationAreaId > 0 ? [ navigationAreaId ] : noTerms; | ||
// // Toggle the active navigaiton area term. | ||
// await editEntityRecord( | ||
// 'postType', | ||
// 'wp_navigation', | ||
// navigationMenuId, | ||
// { | ||
// wp_navigation_area: maybeNewAreaTermId, | ||
// } | ||
// ); | ||
// // Persist the change. | ||
// await saveEditedEntityRecord( | ||
// 'postType', | ||
// 'wp_navigation', | ||
// navigationMenuId | ||
// ); | ||
// }; | ||
// if ( isEntityAvailable ) { | ||
// updateNavEntityArea( navigationAreaId ); | ||
// } | ||
}, [ navigationAreaId ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI folks this code is designed to automatically assign the wp_navigation_area
term (that which is currently set on the Navigation block) to the wp_navigation
Post (the "menu items") currently active on the block.
I had to disable it because we cannot have the dropdown that controls both:
- Assigning the term to the
wp_navigation
Post, AND... - Designating the Nav block as being a placeholder for a given "area".
Took this for a quick spin, but I probably bungled a few steps as I couldn't quite get the whole thing to work as described: Nevertheless, it's an impressive PR, and an impressive goal to solve: preserving the contents of your menu as you switch to a new theme. Thank you for working on it. As I read the PR and tested the various steps, I felt that the goal being sought was one that could benefit every block area, not just the contents of the navigation. As I switch themes, I might want to keep the content I added to my footer or my sidebar. Or I might want to keep just the contents of the footer, but not that of the sidebar. In fact I might really like the font and colors of my previous theme and want to bring that along. Perhaps I want to keep the 404 page from the old one, but really want the front page of the new one. Maybe I'm inspired by the beautiful patterns and demo content from the new theme, so I want a fresh start for some templates. All of that suggests me that regardless of how the navigation block is persisted, there are some opportunities around how we persist, transfer, and give choice during the theme switch process, and potentially even surface checklists of things to transfer mid-switch. This is something that I know @critterverse has been exploring, with some recent explorations surfaced in her post, Adventures in Block Theme Switching. |
// await editEntityRecord( | ||
// 'postType', | ||
// 'wp_navigation', | ||
// navigationMenuId, | ||
// { | ||
// wp_navigation_area: maybeNewAreaTermId, | ||
// } | ||
// ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the concept of assigning an area to a menu support a menu being assigned to more than one area?
If not, it might be that a taxonomy isn't the right concept. I think the idea I had in mind is that a menu would be assigned to an area—so the area is able to store a reference to the menu it renders. Not sure what WordPress data types could support that idea.
The other reason for doing it that way is that there's no situation where two menus are assigned to one area.
When an area is assigned, the 'Select Menu' dropdown could change the association. |
Thanks for linking to my post @jasmussen! The Navigation block is something I wondered about as a part of as a possible pre-activation checklist when switching themes — but I kept coming back around to the sense that it should “just work.” I like these users stories outlined by @getdave, where it sounds like there could be a couple of different contingencies to get a nav in place. If the user would prefer a different nav, it's also easy to swap or start fresh at the block level. But I'm curious whether y'all can imagine a scenario in which the user does not want to bring their navigation along to a new theme after switching? This didn’t seem like a common use case but I may be missing some of the nuances. Kudos on all the awesome Nav block work so far, I love the direction it's heading in :) |
We can close this one. |
Description
WIP PR. Description to follow.
Closes #35750
How has this been tested?
This is very rough.
Before applying this PR
trunk
After applying this PR
wp-admin/edit-tags.php?taxonomy=wp_navigation_area&post_type=wp_navigation
and create two areas:Primary
Secondary
wp-admin/edit.php?post_type=wp_navigation
and assignMain Menu
toPrimary
andUtility Menu
toSecondary
.Navigation Areas
.Primary
- it should pull inMain Menu
.Secondary
it should pull inUtility
.Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).