-
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
Extract pattern-overrides code from the block-editor package #60887
Conversation
Size Change: +351 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
export default function RenameBlockModalControl() { | ||
const { renamingBlockClientId } = useSelect( | ||
( select ) => ( { | ||
renamingBlockClientId: unlock( | ||
select( patternsStore ) | ||
).getRenamingBlockClientId(), | ||
} ), | ||
[] | ||
); | ||
if ( ! renamingBlockClientId ) return null; | ||
|
||
return <BlockRenameModal clientId={ renamingBlockClientId } />; | ||
} |
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.
Wondering if you'd considered the BlockSettingsMenuControls
slot to add the pattern specific rename action? It looks like the fills handily get rendered right after the rename menu item:
gutenberg/packages/block-editor/src/components/block-settings-menu-controls/index.js
Lines 86 to 91 in 8deb8f9
{ showRenameButton && ( | |
<BlockRenameControl | |
clientId={ selectedClientIds[ 0 ] } | |
/> | |
) } | |
{ fills } |
Then it might just require a allowBlockRenaming: false
editor setting then to remove the normal rename action.
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.
My idea is to avoid customizing through the controls so that we can reuse the same renameBlock
function for commands and other renaming flows as well. We can consider supporting both if needed though, but to me it's not necessary for now. Does it make sene?
Flaky tests detected in 0004087. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8751715773
|
0004087
to
16fd393
Compare
} } | ||
aria-expanded={ renamingBlock } |
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.
Since we're opening a dialog and we won't be able to access this element when the dialog is open, I don't see why we need this 🤔. I could be wrong though so could use a sanity check from @Mamaduka. 🙏
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.
You're right. The content behind the dialog become "inert" and browser should ignore them. Adding them to few menu items with modals was my mistake.
Closing this one due to the comment here - #60619 (comment). Thanks for working on this though and sorry for the u-turn! |
What?
Close #60619. Extract pattern-overrides code from the block-editor package.
I'm not entirely sure that the code duplication is worth the effort 😅, so I'll leave this as a draft for now.
Why?
Pattern overrides is a WP-only feature, which shouldn't be inside the
block-editor
package.How?
Use a private
renameBlock
editor setting to register different flow. This can be further extended when we support renaming blocks via other UI actions (shortcut, command, etc).Testing Instructions
The original renaming flow should still work as expected.
Follow the same testing instructions in #60234 and #60769.
Screenshots or screencast
N/A