-
Notifications
You must be signed in to change notification settings - Fork 4.6k
contentOnly patterns: Hide pattern internals in the list view #72574
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
|
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. |
|
Size Change: +114 B (0%) Total Size: 2.42 MB
ℹ️ View Unchanged
|
| return; | ||
| } | ||
| const blockAttributes = getBlockAttributes( block.clientId ); | ||
| const isPattern = !! blockAttributes?.metadata?.patternName; |
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.
Could possibly use the isSectionBlock / getParentSectionBlock selectors for some of this logic. That would expand support to template parts, synced patterns and blocks with templateLock: 'contentOnly'.
|
Couple with whatever #72044 becomes, I see a lot of positives in this simplified interface, thanks for getting it up.
I think it reinforces patterns as unique entities and, at least for me, allows my brain to more quickly identify patterns. The only thing I'm not sure about is whether the keyboard navigation still makes sense. It seems to work find for me, especially when using the arrow key to navigate through pattern blocks. I suppose there are fewer DOM elements to render in the list view too! |
Not in any way urgent, but I just noticed if I switch the contentOnly experiment off, the patterns are stuck in this reduced mode in the list view, so we might want to add a check somewhere for the contentOnly experiment? |
|
Flaky tests detected in 821bafb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19170541158
|
| window?.__experimentalContentOnlyPatternInsertion | ||
| ) { | ||
| return null; | ||
| } |
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 wonder if (as an alternative) it might be possible to avoid rendering the entire ListViewBranch? So instead of return null here, the conditional on line 300 could be changed to something like:
{ ! isSection && hasNestedBlocks && shouldExpand && ! isDragged && (
<ListViewBranch
Then whenever the block is a section we could also use the hasSelectedInnerBlock selector to make it have the selected style, which would reduce some of the logic below.
|
Sorry to be the nay-sayer, but I'm worried about this change since there are people who rely heavily on the list view for selecting blocks and navigating the overall block tree. This change would make it harder for those people to access the content. We'd be taking a place that you can always select the block you want to edit, and make it so you now need to learn two separate modes for block selection in two different areas of the tree. |
|
@jeryj I understand your concern. The counter point is that not all of the actions that we can usually do in the list view will work with this editing mode, so things like drag and drop won't work, which still creates a different interaction pattern. The aim of removing the children in the list view is that the pattern is a "single block" rather than a block with inner blocks. |
But we still offer affordances in the other sidebar to allow block selection. I'm in favor of pushing towards a mental model that patterns are a single block, but I don't want to lose the very real benefit of block selection from the list view. Could we differentiate between them somehow? To make it clearer that it's a pattern but not lose the selection model? |
|
Interesting discussion! I reckon a lot of content only will be a balance between abstraction and transparency. Seems like we're committed to making patterns feel like atomic, indivisible units. However, I get the concern about people relying on list view for block selection. We're removing what some folks see as a utility Trying to think of compromises... What about hiding pattern internals in the list view only when unselected? And when shown use visual styling (e.g., lighter opacity) to show they're "part of a pattern unit"? Maybe there could also be a preference setting to always expand patterns as well. 🤷🏻 |
They're already collapsed until you "move into" them by clicking the expand arrow or a right arrow keypress to expand that item. Maybe once we have the full sidebar editing experience for content only this change will make more sense? Until there's a way to edit all this content directly from the inspector sidebar, this change feels like a step backwards for keyboard users. |
Fair point 👍🏻 @talldan is away for a bit, but we hope to push #71730 forward as best we can. Any testing/reviews would be appreciated! 🙇🏻 |
|
Just retested, and things are working pretty well! I get there might be a few outstanding things to work out, only wanted to mention that this change matches conceptually — at least to me — what @jasmussen was explaining somewhere I can't find right now, that it helps to think of patterns as blocks. |
|
I removed the label and icon changes. I think this is good for another review. |
|
Thanks for updating this! I think it works well. It'd be good to get sign off/consideration from @jasmussen Heads up, the following was merged yesterday: This excludes template parts from the content only effect, so I expect the internals should be exposed again? |
|
A quick GIF for ease of conversation, showing a contentOnly locked pattern showing up as a single block in the list view, until "Edit section" is clicked, at whic point the full list view tree is shown. I like it! The List View is meant to be a 1:1 representation of the block list, and the guiding instinct for this work in general has been to treat a pattern as if it was a block itself. There are editable fields inside, but they are arguably not to be thought of as "blocks", and so showing just a single pattern item in the list view feels accurate. That said, the list view is specifically something I know that @fcoveram has had thoughts on, going in the same direction of ensuring the List View always speaks truthfully. To that end, I'd love to wait for him to just briefly chime in to validate agreement. |
Hol up 🤣 |
+1 to this. I like how it works in the video above. I'm not 100% sure as ListView and Canvas should show the same elements, but embracing the pattern concept is better to address the interaction from an entity type. |
|
Apologies for coming and contradict my previous comment. The general approach I think we need to follow is to keep the current heuristic, therefore, show the content state of blocks in both ListView and Canvas. With that in mind, and after taking a look at #73419, I think it's more clear to show internal blocks rather than hiding it. In cases where non-sync pattern live inside sync patterns that also live inside template parts, I envision obscure flows of interaction when trying to select and edit a block or a content. The above is more clearly outlined in a recently shared design proposal in #73466 |
|
Looks like the current understanding is that items in the list view will be shown for "spotlight" mode |
Thanks for working on this @scruffian and for the clarification @fcoveram I'll close this and the accompanying issue. We can reopen if there's new information! |
|
May have closed this prematurely. Once we have a solid plan on this I'll return and rectify. Cheers |



What?
Closes #72019
Hides the inner blocks for pattern blocks. This obscures the internals of the parent block, making it act like one single block. We also remove the ListViewExpander from these blocks
Why?
These changes attempt to tie the contentOnly changes more firmly to the concept of patterns, to make it clear to users why things are working differently for these blocks.
How?
useContentOnlySectionEdithook to share with the two "Edit pattern" buttonsUnlockDesignMenuItemcomponentTesting Instructions
Screenshots or screencast