-
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
Ensure imported Classic Menu dirty state to include in Editor entity changes list #50318
Conversation
As this bug exists in WP 6.2 should I include the |
Size Change: -1 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Flaky tests detected in ebee8b4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4881795694
|
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.
I tested this and it looks good. It does not reintroduce the issue that #43580 was meant to fix. Let's merge!
What?
Fixes a bug whereby importing a Classic Menu would not trigger a dirty state on the resulting
wp_navigation
entity and thus the new Navigation could not be saved.Change causes newly imported Navigation Menu to show in the sidebar when you click "Save" in the editor.
Fixes #49718
Why?
It looks like #43580 introduce a regression.
The bug is that when the classic menu is converted into a (block based) Navigation Menu the call to
editEntityRecord
does not set the status topublish
. Rather it simply sets it to whatever the initialstatus
argument was that was passed to the "convert" function:gutenberg/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js
Line 100 in 44308e7
This means that the entity never becomes "dirty" and thus never gets included in the "saved" changes when you click "Save" in the editor.
The clue is in the comment where it references
publish
as the expected state change:gutenberg/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js
Lines 88 to 103 in 44308e7
How?
The fix is to change the status argument back to a hard coded string
publish
in theeditEntityRecord
call.Testing Instructions
Follow instructions in #49718.
On this branch once you you complete this step...
...the newly imported Classic Menu should be included in the sidebar when you click "Save" in the editor. Clicking and confirming the "Save" should cause the Navigation to update on the front of the site.
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2023-05-04.at.11-36-30.mp4