-
Notifications
You must be signed in to change notification settings - Fork 4.6k
contentOnly patterns: Allow re-entry into contentOnly mode after 'Edit contents' #72044
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
Conversation
|
Size Change: +824 B (+0.03%) Total Size: 2.45 MB
ℹ️ View Unchanged
|
|
Looks good, nice work. "Edit design" is a good option. "Edit blocks" is another option. I don't know that I have a preference, it seems like a button that can also be renamed at a later time if need be. Behaviorwise this also feels like a good experience that, through the spotlight usage, feels very coherent. Nice work. I'm waffling a bit on the "Finish editing" button, mostly because that button sits on every single innerblock you select. That itches in my brain, insofar as it might suggest finish editing the Paragraph, not finish editing the pattern. There's also something about the phrasing, which is not bad, but perhaps something to just marinate on. I wonder if we could do a breadcrumb instead. Something along the lines of what a navigation menu item does with a back arrow next to the main block name? We can potentially make that more explicit, happy to do some mockups if this resonates. But the loose/unformed idea would be to maintain the IA of the innerblocks, and provide a "back" affordance that sits in context of the parent. |
|
@talldan So I didn't check how you implemented this but one thing that I'd like to note is that I see this behavior expanding to more than just these unsynced patterns. I actually see this replacing the "focus mode" of some of the other blocks (synced patterns, template parts, navigation blocks...). I know we still need to discuss this design wise cc @jasmussen but for me, as a user, It feels like I'm "focusing" on specific area. I know that for the use-cases I mentioned there's a subtle difference in the fact that these cases save to different entities (so we can in this case navigate to a new "page") but I do wonder if the default should be similar to this (a block focused mode) with the possibility to "navigate" to a full page to edit the entity and in that case, I wouldn't even consider that later option as a "focused mode". Anyway, I guess all what I'm saying is that ideally this mode is not tied strongly to the "pattern" behavior. I see this as a state in "block-editor" package that says Anyway, just random thoughts, probably nothing actionable for now, but wanted to share this thought. |
|
I agree using spotlight scaffolding for this is working extremely well to focus actions, and we should be adopting this more broadly. I also agree this can take over a fair amount of similar contexts (it will be used for block comments also). I'm not convinced it's a complete replacement for isolated editing, however, because that mode also has the benefit of reducing the block tree, simplifying the amount of nesting layers you have to engage with. I don't know where to draw the line, most simple template parts feel fine in this mode, but extremely complex ones could really benefit from the isolation. And we still have concepts like navigation overlays that may end up being something that isn't rendered in the editor at all, only previewed, and then edited in isolation in a go-in and back-out kind of way. No strong opinions shared in the above, happy to try anything, mostly wanting to ensure some potential hidden benefits so we don't forget them. |
|
Noting that #45264 may be able to have a context shift based on outcomes here, the most basic being to close it. |
|
I agree that what I'm proposing is a solution to #45264 I'll add that every-time we fork the experience for the user we confuse them. What I'm saying is that ideally, for users, there should be just a single treatment to "focus" on a block. This doesn't mean you can't edit "template parts", "synced patterns", "navigation" in total isolation. But this, is not "focused mode", this is just navigating to the editor of these separate entities. |
|
Agreed, and that resonates. It's mostly the reduced block tree in the focused mode, and of course exiting the mode being intuitive, that's on my mind. If a breadcrumbs/back button can handle the latter, I wonder, is there any way we can also handle the former? Can we/should we show a reduced block tree in the spotlight-focused mode? |
|
This is great! The flow feels so much simpler and tighter 👏🏻 Some thoughts while trying the PR (haven't reviewed code yet): "Done" in the toolbarI'm torn on this. One one hand, I like the exit hatch, but I think it would be fine to leave it off for now as it's quite easy to exit this mode at the moment. If we do leave this button in the toolbar, it should match whatever text is used on the main button to exit the mode (currently "Finish editing"). I think we can remove this "Done" button from the group toolbar though, since it doesn't really semantically match the context IMO. Keyboard users accidentally exit the mode too easilyCan focus mode become a modal/focus trap for the interaction? Pressing Escape on the canvas could exit the mode. Keyboard users navigate the block canvas with the arrow keys, and it's hard to know where your block boundary is. If you press the arrow key one too many times, you've now unintentionally exited the design editing and need to return to the sidebar to press the button then back to the canvas, etc etc. Detach in group block options menuThis will need some naming help. Detach from what? The reason to click this would be if you want to remove contentOnly editing for this instance of the pattern. I think a name that communicates that would be better. We can leave it "detach" for now, but I wanted to get the wheels turning on what this could be named. Note about "What should we call X button?"I scaffolded the usability test in a way that it can hopefully be run easily with a wider audience. We could collect a few top contenders for names and run the test and see if one wins out.
@jasmussen I like it sitting at that level (above all the inner blocks), as otherwise, you'd need to know to select the parent group block to find the button again.
I agree that "Finish editing" could be better communicated for the same reason you mention. Let's test out a few different names once we get this overall flow down. I think we're narrowing in on the right experience and that will help us decide what language to use.
@youknowriad I agree. When starting in the full site view, editing template parts would benefit from this focused editing view instead of being transferred to an isolated view. The focused/spotlight feels much more seamless. |
@jasmussen This may be another way to handle the keyboard selection issue I mentioned. If the block tree is limited to just those within the pattern, then the keyboard shouldn't accidentally arrow outside of the focus boundaries, since that would be the entire block tree. |
|
Thanks for all the feedback!
This option is from the existing code in
I'll have a look at how we could implement something like this. Also worth considering whether we want click outside to be the main way to exit, or if it should be something like double-click or not hooked at all around block selection (at the moment it works via this component). There might a chance to align both mouse and keyboard behavior if we make the blocks outside the pattern completely non-selectable.
At the moment this is fixing some issues with and replacing the 'Modify' contentOnly feature that Jorge implemented some time ago. I think it should be possible to make most of the other modes use it. The one that's more difficult is Synced Patterns, as I think we'll have to temporarily disable bindings (pattern overrides) for the user to truly be editing the source pattern. I'm not sure if it's possible currently, so it might take a bit more exploration and collaboration with folks working on bindings. We could also try it without disabling the bindings and see what it's like, though it might then be confusing that it's different to 'Edit Original'. |
9f89941 to
cd0d5de
Compare
@jasmussen I pushed a commit that tries this to see what it's like. I realize there are some conflicting opinions about this part, but just trying things out at the moment. The mention of breadcrumbs also made me realize the breadcrumb bar at the bottom needs some love! |
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.
Very cool, I really like how this helps orient you when working on the pattern, and with the way it highlights working on a particular section, it's feeling pretty good to me.
The main question I had was to do with the spotlight mode and how it feels when going to make changes to the pattern that the user wants to see in relation to blocks that sit next to it. For example, if I have a template that's using particular colors and I want to see how making a big change to the design of the pattern will look in relation to other blocks, I might need to click out of the pattern in order to see that:
2025-10-06.16.39.59.mp4
Then, clicking back in to get back to where I was takes a couple of extra steps. So, for folks making design changes there's potentially some friction here. I almost wanted a toggle to be able to highlight the pattern or not — but I don't want to suggest adding additional controls 😅
On the other hand, what I'm really liking about how it all feels in this PR is that it's hard to accidentally make changes to the design, and it's easy to get back to content mode 👍
(These are just some impressions from using it — overall I like the direction here!)
I actually see this replacing the "focus mode" of some of the other blocks (synced patterns, template parts, navigation blocks...).
That thought struck me, too. The behaviour in this PR feels quite 'solid', so it'd be fun to see how the spotlight approach could work for editing headers and footers, too (separately from this PR, of course) 🤔
@jasmussen By this do you mean hiding the other blocks in List View, or literally hiding all the other blocks (both in canvas and list view)? I tried something akin to focus mode within List view, which I think works ok (though there might be accessibility issues with the contrast level): Kapture.2025-10-06.at.14.48.55.mp4I also tried completely removing the other blocks, not sure it works as well: Kapture.2025-10-06.at.15.05.22.mp4 |
|
Lots of incredible discussion. It's likely I missed bits, apologies in advance if that's the case.
Not a strong opinion on my part, except the wording, and fair point. I do wonder if this is one of those cases where it might make sense to in fact show the button above the block title, just to maintain the IA? I think that's actually what Ben is proposing here. It's perhaps a bit clunky, something we can ponder refining, but just to finish the thought, this:
Still not sure about the wording. "Exit editing" does not seem better than "Finish editing". Note, I'm compiling the branch as I sketch this, I look forward to testing the back button too, that may also work. I'll write another comment in a minute. |
|
Taking a look at the branch: Nice, that's not bad. I'll defer to you all on whether it's something like this branch, or this. Any preferences? One note if we go with the back button, the indentation for the description is looking a bit big. I think it might be good to generally unindent the paragraph, like I showed in the mockup above. That's not a blocker in any way, just something we can change in the blockcard component itself, as a bit of polish. |
faff3cd to
b4bcc74
Compare
|
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. |
|
#72959 is now merged and I've trimmed some other parts from this PR. It should be ready for review. |
Try hooking Edit Contents button up to modifyContentLockBlock action Fix block editing modes Fix focus mode restoration Fix docblock incorrect param Remove existing editing tools Show edit design button for templateLock contentOnly blocks Consider temporarily edited block not to be a section Show finish editing button for any child block of the temporarily edited block Revert "Remove existing editing tools" This reverts commit a43b0fa. Ensure done button is visible Hide modify and done button when experiment is active Implement back arrow when child of temporarily edited blocks is selected Only show Edit design button on main section Try limiting list view to edited blocks Try focus mode in List View Try disabling blocks other than the temporarily edited blocks Re-instate click outside behavior, but keep other blocks disabled to constrain keyboard behavior Disallow editing of title when temporarily editing blocks Add escape key for exiting mode Show parent/child relationship on block card when editing contents Fix error destructuring object caused by early return Remove focus mode revert code Make parent selectable in block card Remove lint disable Try different copy Improve button styling Rename APIs Rename classname Simplify EditContents render condition Fix navigation block back button showing for block cards with parent/child relationship Improve nested sections - only consider top level section as THE section Avoid showing nav back arrow when in a content only section Add comment Revert changes to content preview file Try ts-ignore on every line for import Fix show template mode by making editedContentOnlySection take priority Only disable blocks outside edited section when experiment is active Fix issue where template lock is persisted even after unlock Fix spotlight in list view Fix conflict resolution issue Fix merge conflict issue
…cks and selecting an inner block
650e3d3 to
7cf99a6
Compare
| // For the content only pattern experiment, disable blocks that are outside of the edited section. | ||
| if ( window?.__experimentalContentOnlyPatternInsertion ) { | ||
| derivedBlockEditingModes.set( clientId, 'disabled' ); | ||
| return; | ||
| } |
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.
It looks like this results in a lock icon when the root section block is selected after unlocking the design. Is that expected? I think it might be because the section block now returns false for canMove and canRemove in useBlockLock?
This then also means the Unlock option is shown in the block settings menu, which could be confusing:
Then, I notice that if I lock the pattern again, the Unlock icon shows up in the toolbar (I think because if it's been shown once, it'll continue to be shown?):
Not a blocker if this one's tricky, though, as this part of the behaviour is guarded behind the experiment.
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.
Yep, it was discussed a little bit earlier as well in this PR, it's an existing bug in trunk that's surfaced by this change. I've made an issue for it - #72159.
I'll plan to tackle it separately seeing as it wasn't introduced (and this feature is still experimental).
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.
Sounds good, thanks for confirming, and sorry for the double-up!
andrewserong
left a comment
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.
This is testing great for me, and the code changes looks good. Just noted a couple of other drive-by comments as I thought of them, apologies if it's covering ground we've already covered earlier.
In addition to testing these changes, I smoke tested unsynced patterns, synced patterns, template locked patterns, template parts, etc, with the contentOnly experiment disabled, and everything appears to be working as on trunk as far as I can tell.
There's quite a few changes here, though, so do feel free to get another approving review if you're unsure of landing the change, but this LGTM!
Great work here 🚀
| parentNavBlockClientId | ||
| ? __( 'Go to parent Navigation block' ) | ||
| : // TODO - improve copy, not sure that we should use the term 'section' | ||
| __( 'Go to parent section' ) |
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 flattening the structure, could "go to parent" be enough? (Not a blocker for now, of course)
| __( 'Go to parent section' ) | |
| __( 'Go to parent' ) |
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.
Ended up removing this, it's from an earlier iteration!
| isTemporarilyEditedBlock: | ||
| getTemporarilyEditingAsBlocks() === clientId, | ||
| isContentOnlyTemplateLocked: | ||
| getTemplateLock( clientId ) === 'contentOnly', | ||
| }; | ||
| }, | ||
| [ clientId ] | ||
| ); | ||
|
|
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 guess a problem with doing that would be that if we close the sidebar, then we'd stop editing blocks, which mightn't be desired?
| getBlockSelectionStart, | ||
| } = select( blockEditorStore ); | ||
| return ( | ||
| ! getBlockSelectionStart() || |
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.
Tiny nit: feel free to ignore:
Just trying to wrap my head around this change. Is this so that if a user deselects all blocks (i.e. by hitting Escape within the list view), we don't accidentally stop editing the section? If so, it might be worth adding an inline comment as the negation of getBlockSelectionStart() is a little different to the name of the isBlockOrDescendantSelected const.
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.
It was probably something like that. I can't remember now, so wish I'd left a comment!
I think I'll remove the code that was added, from testing I can't reproduce any issues that it might resolve.
| node.addEventListener( 'click', onClick ); | ||
|
|
||
| return () => { | ||
| node.removeEventListener( 'click', onClick ); | ||
| }; |
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.
Also optional: should these be guarded behind the experiment, or does it not matter?
ramonjd
left a comment
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.
Thanks a lot for all the hard work here.
I checked the following:
✅ That I can unlock and relock patterns and template parts using the button and clicking outside the pattern
✅ That I can still "lock" and "detach" the pattern
✅ Group blocks with templateLock set (work as expected)
Also turned off the experiment and compared with trunk behaviour, and couldn't see any regressions.
Thanks for splitting up the PR 🙇🏻
Folks have already noted most of these but I think follow ups could include:
- how to separate pattern unlocking from the concepts of lock/detach in the minds of users. Might be a rename of this feature or some other hints. This will come out in the wash later I reckon.
- Should we be able to insert patterns into other unlocked patterns? I couldn't do this
- How to reconcile edit button in toolbar of template parts. Do they do the same thing, or are we duplicating functionality?
I think it's ready to go!
Did you have Zoom Out active? That would prevent inserting patterns into other patterns. If you exit Zoom Out it works ok. |
Oh thanks for checking. Yes I did 🤦🏻 I was trying to add patterns from the sidebar inserter - that switches zoom on straight away. |
|
Thanks for all your work on this, its looking great! |







What?
Fixes #72017
I think this might need more work, but surfacing early to get some feedback.
Changes the 'Edit contents' button behavior for unsynced patterns:
How?
A lot of this is achieved by using and improving the existing 'Modify' option that
contentOnlylocking has. Unsynced patterns now use the exact same system.As part of this I've fixed #49048, when editing a templateLock: 'contentOnly' block, the
templateLockattribute is no longer temporarily removed. Instead the blockEditingModes are set todefaultin the reducer, a change which won't persist when saving.Testing Instructions
Also try the same with:
"templateLock":"contentOnly"attribute set and wraps some content blocks.They should work the same way.
To do
getTemporarilyEditingAsBlocks) to be more related to content lockScreenshots or screencast
Kapture.2025-10-03.at.15.29.32.mp4