-
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
Locked Templates: blocks with contentOnly locking should not be transformable #64917
Conversation
Size Change: +665 B (+0.04%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Flaky tests detected in 7293615. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10738765593
|
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.
Left a few comments, but it seems pretty close.
@@ -252,7 +264,8 @@ export const BlockSwitcher = ( { clientIds, disabled, isUsingBindings } ) => { | |||
clientId: clientIds?.[ 0 ], | |||
maximumLength: 35, | |||
} ); | |||
if ( invalidBlocks ) { | |||
|
|||
if ( invalidBlocks || selectedHasTemplateLock ) { |
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 think this results in no block icon, which doesn't seem like the right thing to do:
Instead there's this block indicator component that's the block icon without a dropdown that could be rendered:
gutenberg/packages/block-editor/src/components/block-switcher/index.js
Lines 266 to 283 in 84b48f7
if ( hideDropdown ) { | |
return ( | |
<ToolbarGroup> | |
<ToolbarButton | |
disabled | |
className="block-editor-block-switcher__no-switcher-icon" | |
title={ blockSwitcherLabel } | |
icon={ | |
<BlockIndicator | |
icon={ icon } | |
showTitle={ isReusable || isTemplate } | |
blockTitle={ blockTitle } | |
/> | |
} | |
/> | |
</ToolbarGroup> | |
); | |
} |
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.
Whoops, I missed that. Good spotting. I'll check out your suggestion. 🙇🏻
@@ -120,6 +126,7 @@ export function PrivateBlockToolbar( { | |||
_isDefaultEditingMode, | |||
isUsingBindings: _isUsingBindings, | |||
canRemove: canRemoveBlock( selectedBlockClientId ), | |||
selectedHasTemplateLock: _hasTemplateLock, |
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.
Nit: Naming thing (also applies to other files) - selected
isn't needed as the toolbar only shows for selected blocks. hasTemplateLock
implies any kind of template lock (that it could also be 'all'), but that isn't the case.
Maybe it can be simplified to isContentOnly
or hasContentOnlyLocking
6edcde7
to
c8ec66f
Compare
…-switcher, only hide the drop down if a selected block is content locked.
c8ec66f
to
7293615
Compare
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 my testing this worked as expected 👍
Thanks a lot @getdave! Yes, all good. I appreciate you keeping it up to date. |
Co-authored-by: Dave Smith <[email protected]>
Hey @ramonjd 👋 Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note. We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1. All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :) Please let us know if you can assist with that. Thanks in advance :) |
Dev Note: blocks with "contentOnly" locking are no longer transformableIn 6.7, the block transformation menu and block variations selector are not available for blocks with "contentOnly" content locking. For example, a Group Block that has "contentOnly" locking cannot be transformed to a Columns Block, or switched to a Group variation such as Grid, Row or Stack. Transforming to other blocks or block variations has the potential to mutate the entire block structure, including its internal content. The purpose of "contentOnly" is to allow editing of a block's content, for example, its internal headings, text or images. |
Resolves #64661
What?
Where a block has content locking, do not show the block transformation/variations picker in the block tools popover or the block tools inspector.
Why?
Switching variations or to other block variations transforms the block and, therefore, potentially the content inside.
Blocks that a locked with
contentOnly
should theoretically only allow editing of content (text, images etc).How?
Checking for
contentOnly
status and not rendering the variation tools.Testing Instructions
Example HTML
Click on a locked group. Check that the transformation/variation options in the toolbar and inspector are not available.
Unlock the block group by right clicking and selecting "Modify" - you should see the regular transformation/variation options in the toolbar and inspector.
Select several locked blocks. The toolbar/inspector should not display any block transform options.
Screenshots or screencast
Kapture.2024-08-30.at.15.05.27.mp4