-
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
Fix query block duplicate group block rename controls for WP 6.4 #55604
Fix query block duplicate group block rename controls for WP 6.4 #55604
Conversation
Size Change: +23 B (0%) Total Size: 1.63 MB
ℹ️ View Unchanged
|
Flaky tests detected in c8c44cf. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6638634898
|
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.
One nit, this looks good to me.
packages/block-editor/src/components/block-rename/empty-string.js
Outdated
Show resolved
Hide resolved
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.
Thank you for this! Also looks good to me, I've left one inline question.
packages/block-editor/src/components/block-settings-menu-controls/index.js
Outdated
Show resolved
Hide resolved
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.
Just noticed that this test failure may be a genuine failure related to these changes.
And this is why I wrote good tests for this feature 🎉 Yes we need to reinstate the inspector control to allow renaming. Probably requires re-adding the hook. |
Note that when this PR merges we will want to create a PR which merges the changes here with those in This could be a relatively complex merge operation due to how |
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.
Changes look good to me and tests are happy! 🚢
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.
LGTM
I've gone ahead to remove the |
What?
Fixes the repeated
Rename
menu item in the block options menu when the Group block is within a Query Loop block.Fixes #55517
Why?
Because you only want to rename once, rename once, rename once, rename once...
How?
Removes the reliance on
Block.Edit
hook and instead moves the menu item directly into theBlockSettingsMenuControlsSlot
.Why is this ok?
For 6.4 this feature exists only for the Group block. However in Gutenberg
trunk
the feature is enabled for (nearly) all blocks.This means the feature is an integral part of the Editor and so using a hook no longer makes sense. Indeed, we agreed on such an approach in #55250 (comment).
Testing Instructions
lots of31 Posts (I used Fakerpress)Rename
option.wp/6.4
branch.Rename
is only available for GroupAdvanced -> Rename
.Testing Instructions for Keyboard
Screenshots or screencast