-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Create classic menus empty #43190
Conversation
Co-authored-by: Dave Smith <[email protected]>
Co-authored-by: Dave Smith <[email protected]>
Size Change: +159 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
@@ -247,6 +247,7 @@ function Navigation( { | |||
status: classicMenuConversionStatus, | |||
error: classicMenuConversionError, | |||
value: classicMenuConversionResult, | |||
classicInnerBlocks: classicMenuConversionInnerBlocks, |
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.
This is used to pass the innerBlocks to the block from the hook.
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.
@@ -359,6 +360,8 @@ function Navigation( { | |||
showClassicMenuConversionNotice( | |||
__( 'Classic menu imported successfully.' ) | |||
); | |||
|
|||
replaceInnerBlocks( clientId, classicMenuConversionInnerBlocks ); |
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.
This does work, but other changes get triggered which means they get wiped.
@@ -65,13 +66,11 @@ function useConvertClassicToBlockMenu( clientId ) { | |||
|
|||
// 2. Convert the classic items into blocks. | |||
const { innerBlocks } = menuItemsToBlocks( classicMenuItems ); | |||
setClassicInnerBlocks( innerBlocks ); |
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.
So that they can be passed from the hook back to the component,
menuName, | ||
innerBlocks | ||
); | ||
navigationMenu = await createNavigationMenu( menuName, [] ); |
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.
We don't need this change.
@@ -100,7 +99,7 @@ function useConvertClassicToBlockMenu( clientId ) { | |||
setValue( null ); | |||
setError( null ); | |||
|
|||
convertClassicMenuToBlockMenu( menuId, menuName ) | |||
return convertClassicMenuToBlockMenu( menuId, menuName ) |
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.
We don't need this change.
efcdb1e
to
6072b40
Compare
We did this in a different way |
What?
This is an attempt to fix #42850
Why?
When you create a new navigation menu from a classic menu then a new wp_navigation CPT is created, which is already populated with the content of the classic navigation. The upshot of this is that as soon as the classic navigation is created the fallback behaviour of the block is changed and the classic menu is used instead of page list (as described in #42799)
How?
When we create a new "classic" wp_navigation CPT we do it without any innerblocks, so its empty. This means that the fallback behaviour is unchanged. We then populate the inner blocks with the navigation items that were returned from the API. This should mark the block as "dirty" for the entity saving flow, so it feels like it should be a simpler approach.
Testing Instructions
To replicate the issue
With this PR enabled
6. The fallback is unchanged
7. (not working from this point) You should see the classic menu inner blocks in the editor
8. When you go to save you have the option to save the new CPT
9. When you have saved the frontend now displays the classic navigation menu items.