Repository navigation
Conversation
|
Size Change: +122 B (0%) Total Size: 8.04 MB 📦 View Changed
ℹ️ View Unchanged
|
richtabor
added
[Type] Feature
richtabor
marked this pull request as ready for review
May 24, 2026 14:06
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new “Lock layout” option to the Block Lock modal for blocks exposing a templateLock attribute, mapping it to templateLock: 'contentOnly' and ensuring existing lock affordances recognize this state.
Changes:
- Extend the Block Lock modal to include a “Lock layout” checkbox and update submission logic to write
templateLock: 'contentOnly'. - Update
isLockedBlockto treattemplateLock: 'contentOnly'on the block as a locked state. - Add unit coverage for
isLockedBlockwhentemplateLockis set tocontentOnly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/block-editor/src/components/block-lock/modal.js | Adds UI + state handling for “Lock layout” and updates how template locking is applied on submit. |
| packages/block-editor/src/store/private-selectors.js | Extends isLockedBlock to consider block-level templateLock: 'contentOnly'. |
| packages/block-editor/src/store/test/private-selectors.js | Adds a unit test ensuring isLockedBlock returns true for templateLock: 'contentOnly'. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Flaky tests detected in b0b045a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26366543389
|
richtabor
marked this pull request as draft
May 24, 2026 14:42
Contributor
|
Nice PR. Noting that I also have #79158 which tackles this same issue. I will say that the reason I went in a different direciton and created an explicit button, so as to maintain the explicit act of creating a pattern separate. This PR just creates the pattern immediately, unnamed it appears: 
This is certainly a more elegant one step experience. The question is whether it's more confusing in that sense. |
Member
|
Not sure if @richtabor already pushed that PR, but I liked the sub-menu variation with simplified options (#78612). I think the first step, which is self-contained, will be to refactor the options dropdown to use a modern menu component. |
Contributor
|
Not bad. I like the grand vision for this, which appears to be making pattern editing even more ephemeral. I guess that may answer my question around how this affects pattern creation; it doesn't, it just locks the layout, literally. Pattern creation is a separate thing, and you choose that from the separate menu item. I like that quite a bit. Rich let me know if I got this instinct right, and how I can best support this work. |
Contributor
|
I'm looking at related PRs hoping to unblock landing a fix for #73599. I believe Rich has his hands full for the moment, is there a way we can help resurrect this effort and land a fix? Alternately, we can resurrect #79158 which I closed in favour of this one. It might be a simpler place to start. What do you think? I know you are busy too, @WordPress/gutenberg-design, so thank you for your time. |
Member
|
@richtabor showed me a nice concept at WCEU for simplifying locking via submenus. Once we land the main Options refactoring, I can look into block options via a new menu component design. This should open the door to simplification. |
Member
Author
Exactly. I don't think we should conflate the two.
Exactly. We can simplify locking into a sub-menu within Block Options and within the List View (#78612). |
🤖 PR meta 🤖🎉 PropsIf 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. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: +196 B (0%) Total Size: 8.24 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
|
Contributor
|
Nice, I dig it, I'll keep my own variant closed. What help can I offer here? LMK! |
richtabor
marked this pull request as ready for review
September 22, 2026 12:58
Member
Author
Final test/review. @Mamaduka can you review from a eng perspective? |
youknowriad
self-requested a review
September 30, 2026 13:26
youknowriad
reviewed
Sep 30, 2026
| attributes?.templateLock === 'contentOnly' || | ||
| ( !! attributes?.metadata?.patternName && | ||
| ( getEditedContentOnlySection( state ) === clientId || | ||
| isSectionBlock( state, clientId ) ) ) |
Contributor
There was a problem hiding this comment.
I think the changes here might have unexpected side effects that we are not accounting for. Need to check deeper though.
youknowriad
reviewed
Sep 30, 2026
| if ( isPatternSection && ! isLayoutChecked ) { | ||
| blockAttributes.metadata = { | ||
| ...metadata, | ||
| patternName: undefined, |
Contributor
There was a problem hiding this comment.
Why are we "detaching" something from the pattern when unchecking "lock layout". That feels like two distinct actions no?
Member
There was a problem hiding this comment.
Unlocking a single pattern seems to be a replacement for Detach, but I think in general there needs to be some refinement between pattern editing and what locking layout is supposed to be.
Detach is a permanent operation. Now there's no distinction between temporarily allowing editing and permanently unlocking - it's unlocked or it's not.
And if locking is turned off, e.g., if I run wp.data.dispatch( 'core/editor' ).updateEditorSettings( { canLockBlocks: false } );, I can no longer detach the pattern.
Specifically in relation to deleting patternName, if you check Lock layout, apply, then uncheck the box again it does not restore patternName.
Should unlocking discard pattern identity?
The conflation becomes obvious for me in two place:
- when you toggle "Enable editing all patterns", which works patterns with
patternName. Maybe we need to rethink that option too. - locking an ordinary Group block for example shows the existing Edit pattern button, although no pattern was created. Do we need the surrounding interface to support the distinction?
Contributor
|
Overall my feeling is that this PR is doing too much, I initially thought it's about surfacing "content-only" but it's not, it does a lot more things (opinionated behavior about detaching patterns...). It's not clear whether that was intended but I feel like it creates complexity and confusion. |
ramonjd
reviewed
Oct 1, 2026
ramonjd
left a comment
ramonjd
left a comment
Member
There was a problem hiding this comment.
Overall my feeling is that this PR is doing too much, I initially thought it's about surfacing "content-only" but it's not, it does a lot more things (opinionated behavior about detaching patterns...). It's not clear whether that was intended but I feel like it creates complexity and confusion.
+1
I think, at its narrowest, there's definitely a plausible convenience angle here, which is "I'd like to allow content edits while preserving design on this specific block, but don't make me create a pattern or name anything or add something to my pattern library."
It would help to understand the use case driver. Have there been reports where the existing "Create unsynced pattern" workflow needs improvement?
Maybe we just need a quick way to make a block a pattern? Like one click. I don't believe a saved pattern record is technically required (need fact check).
I’m also not 100% clear on the relationship with patterns, and why it all requires changing pattern detachment.
Wondering if they can be separated, but I guess it needs more planning and/or a wider review of pattern editing's function and UI.
Granted, it's a fairly complex area, and I don't have a complete overview, so I appreciate the effort to get the idea up for consideration.
| if ( isPatternSection && ! isLayoutChecked ) { | ||
| blockAttributes.metadata = { | ||
| ...metadata, | ||
| patternName: undefined, |
Member
There was a problem hiding this comment.
Unlocking a single pattern seems to be a replacement for Detach, but I think in general there needs to be some refinement between pattern editing and what locking layout is supposed to be.
Detach is a permanent operation. Now there's no distinction between temporarily allowing editing and permanently unlocking - it's unlocked or it's not.
And if locking is turned off, e.g., if I run wp.data.dispatch( 'core/editor' ).updateEditorSettings( { canLockBlocks: false } );, I can no longer detach the pattern.
Specifically in relation to deleting patternName, if you check Lock layout, apply, then uncheck the box again it does not restore patternName.
Should unlocking discard pattern identity?
The conflation becomes obvious for me in two place:
- when you toggle "Enable editing all patterns", which works patterns with
patternName. Maybe we need to rethink that option too. - locking an ordinary Group block for example shows the existing Edit pattern button, although no pattern was created. Do we need the surrounding interface to support the distinction?
| }; | ||
| let templateLockValue; | ||
| if ( isLayoutChecked ) { | ||
| templateLockValue = 'contentOnly'; |
Member
There was a problem hiding this comment.
I think it's here, it looks like it adds "templateLock":"contentOnly" to patterns too:
- Select an existing pattern
- Open "Unlock" modal. "Lock layout" is already checked.
- Check "Lock movement" and click "Apply".
- In the Code editor, the pattern's comment now contains "templateLock":"contentOnly"
That templateLock keeps the pattern content only, even when toggling the "Enable editing all patterns" command.
As with below, this says to me that there's too much cross over between unlock layout and patterns going on. One needs to give ground I believe, which would lead to greater changes than just in this PR, including the UI
Comment on lines
+286
to
+287
| isLayoutChecked || | ||
| ( lock.move && ! lock.remove ) |
Member
There was a problem hiding this comment.
I think this messes with blocks that have "templateLock":"all".
Test code:
<!-- wp:group {"templateLock":"all","metadata":{"patternName":"theme/locked","name":"Locked pattern"},"layout":{"type":"constrained"}} -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>Locked pattern text</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->- click "Locked pattern", open Unlock modal
- "Apply to all blocks inside" shows as off and greyed out, even though the block has templateLock: "all".
- Check only "Lock removal" and click Apply.
- In the Code editor, "templateLock":"all" is gone and "templateLock":"contentOnly" has replaced it.
| Before | After |
|---|---|
 |
 |
What's right here?
Contributor
|
I'm personally not convinced the editor needs this option. It's very similar to creating an unsynced pattern and detaching an unsynced pattern. Maybe locking the layout without creating a named reusable pattern will be useful to some users, but I think it might really be a very small number of users that use this. |
Contributor
|
I appreciate the motivation, but I also have to be frank, I've had to focus intently in other areas of the editor for a while, and have become slightly removed from this. In that vein, rather than give support in favour of, or even suggest this is not a good idea, from my end I'll have to defer to Rich and @fcoveram and @jameskoster on this. |
Contributor
|
I understand where the need comes from, but the modal takes too much attention, and the number of feature types might increase if locking is more granular. Perhaps placing these actions in a submenu is simpler and cleaner. The affordances stay on the same screen, and each option is a single action where "Lock all features" is on top in a single group, and the others and a multi-selection group where each feature type is a check/uncheck. Similarly to what Photoshop does. CleanShot.2026-10-16.at.12.59.03.mp4If layout is among the options, why not style? I could envision users locking the styling and allowing only group movement. On the other hand, I don't think I've seen the "Apply to all blocks inside" as an option in other products. The locking features I've seen default to locking child elements. Likely the most common mental model when selecting a group is that actions applied to the parent affect its internal elements. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Adds a layout lock option to the block lock modal for blocks that expose a
templateLockattribute and for unsynced pattern sections. Selecting the option storestemplateLock: 'contentOnly', while existing movement/removal lock options continue to map tolockand non-content-only template lock values.Why?
Blocks can already preserve their inner layout with
contentOnlytemplate locking, but the lock modal did not expose that state alongside the other lock controls. This makes the layout lock visible and editable from the same UI.Visual
How?
templateLock === 'contentOnly'attribute or unsynced pattern association, preserving the locked state during temporary pattern editing.all/insertbehavior.templateLock: 'contentOnly'as a locked state so existing lock toolbar/menu affordances remain available.Testing Instructions
Run
npm run test:unit -- packages/block-editor/src/store/test/private-selectors.js.Run
npm run lint:js -- packages/block-editor/src/components/block-lock/modal.jsx packages/block-editor/src/store/private-selectors.js packages/block-editor/src/store/test/private-selectors.js.Check
npm run wp-env-test status, runnpm run wp-env-test startonly if stopped, and runnpm run build. Open the editor at http://localhost:8889/wp-admin/post-new.php.Open the editor, insert a Group block with inner content, select the Group, and open the block lock modal.
Confirm
Lock layoutappears, can be checked and applied, and is still checked when reopening the modal.Uncheck
Lock layout, apply, and confirm it clears.Regression check: enable
Lock movement,Lock removal, andApply to all blocks inside; then check and uncheckLock layoutbefore applying. ConfirmApply to all blocks insideis preserved when layout lock is unchecked.Insert an unsynced pattern, enter Edit pattern, and confirm the Unlock dialog still shows Lock layout checked. Uncheck it and apply; the pattern name and content should remain while the pattern association is removed.
Testing
npm run buildnpm run typechecknpm run lint:changelogsnpm run test:e2e -- editor/various/block-locking.spec.js --workers=1 --repeat-each=3(21 passed)npm run test:unit -- packages/block-editor/src/store/test/private-selectors.jsnpm run lint:js -- packages/block-editor/src/components/block-lock/modal.jsx packages/block-editor/src/store/private-selectors.js packages/block-editor/src/store/test/private-selectors.js