List View block support: Hide list tab when allowedBlocks is empty, with no children#78932
Conversation
|
Size Change: +85 B (0%) Total Size: 8.6 MB 📦 View Changed
|
|
Flaky tests detected in cdd94c4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27656969731
|
|
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. |
|
Switching back to draft while I try out some other ideas. One thing I wasn't quite happy with is that this PR is an imperative call to disable the support. @talldan has suggested looking at a property on I'll be AFK soon, so I'll likely play around with this more sometime next week. |
f37d516 to
adebc97
Compare
adebc97 to
281ce35
Compare
|
Just switching this over to ready for review now. The main change is that this moves the It also means that combined with gutenberg/packages/block-library/src/gallery/edit.js Lines 654 to 668 in 39f3ed7 So essentially the proposal here is: add a small flag within useInnerBlocksProps that consumers can use as one part of how they control display and interaction with inner blocks. (As always, I'm not wedded to any one approach for this, so happy for feedback if this looks off as an overall approach!) |
tyxla
left a comment
There was a problem hiding this comment.
Thanks for this @andrewserong!
I'm a bit torn if we really need a new listView API - see my comment below.
I think the rest is just mechanics and quite straightforward to finish once we agree on that.
Curious to hear what others think too.
| useDispatch( blockEditorStore ) | ||
| ); | ||
|
|
||
| const isEnabled = hasListViewSupport( name ); |
There was a problem hiding this comment.
Now that isEnabled comes from hasBlockListViewSupport( clientId ) instead of hasListViewSupport( name ), is name still used anywhere in ListViewPanel?
| blockType, | ||
| parentLock, | ||
| defaultLayout, | ||
| listView, |
There was a problem hiding this comment.
Picking up the thread from my comment on the gallery PR (#78796 (comment)), since this is the better place for it.
I'm not convinced we need a dedicated listView opt-out API, and I'd like to question it before we commit to a permanent public surface. The thing that nags me: the flag is only ever subtractive, hasBlockListViewSupport shows it can only turn participation off, and only for a block that already has type support. So it's not really a new capability; it's a refinement of "should this instance's List View show." That makes me wonder whether the answer can be derived from the state we already have rather than declared.
The "managed inner blocks" state that motivates this is templateLock: 'all' + listView: false. A fully locked container that renders a preview has, by definition, nothing insertable and nothing navigable, which feels like enough signal on its own. So the question I'd like us to answer explicitly: can List View participation fall out of existing semantics, e.g. templateLock: 'all' with no navigable children, or a block editing mode like disabled/contentOnly, instead of a new flag a consumer has to remember to set alongside the lock?
Two concrete costs push me to ask: it's a permanent public API on useInnerBlocksProps, born from a single experimental consumer; and the BlockEdit change adds a store subscription to a per-every-block hot path, where the value was previously computed synchronously, and for the motivating case (gallery with no descendants) that propagation looks inert anyway.
To be clear, I think your composition instinct in #1229 makes sense, pairing templateLock: 'all' with a list-view signal beats inventing a bespoke "managed" mode. My doubt is narrower: whether the list-view half needs to be its own declared flag, or whether it should be a consequence of the lock/editing-mode the consumer already sets. If we try deriving it and it turns out gnarly (the navigation special case, controlled-blocks cases like Page List, etc.), then a minimal explicit flag like this is a fine outcome, but I'd like to see that we tried, so we're adding the API because the derivation doesn't hold, not by default.
There was a problem hiding this comment.
I'm sold! 😄
It seems to be working quite nicely for me without the listView flag, so I've reworked the PR to go for the inferred approach. Let's see how it holds up!
281ce35 to
d6906ea
Compare
|
Thanks so much for the thorough review @tyxla! I think you might have mentioned the idea of inferring the managed inner blocks state elsewhere, but it didn't quite click for me until you explained it in detail above. I've reworked this PR to try that out so that it now becomes This feels much better (and simpler to me) and gets to the heart of the use case for the dynamic gallery block. It also resolves a couple of your other (valid) comments about the PR. I'll update the dynamic gallery block PR shortly to match the new behaviour of this one, but right now this is feeling much better to me! This should be ready for another look. I'll be AFK tomorrow, but will continue on with this work next week. Thanks again! |
ramonjd
left a comment
There was a problem hiding this comment.
Just catching up here, thanks for all the helpful explanations folks!
| const hasManagedInnerBlocks = | ||
| getTemplateLock( state, clientId ) === 'all' && | ||
| getBlockOrder( state, clientId ).length === 0; |
There was a problem hiding this comment.
Kudos for trying out the suggested approach! in my testing when I replace the diff example with something like
templateLock: randomOrder ? 'all' : undefined,and I see
<!-- wp:gallery {"randomOrder":true,"linkTo":"none"} -->I don't see the list view's status changing, though the innerblocks are locked 🤔 I'm probably doing something wrong, or is that expected?
Curious, what's the intended usage for the dynamic gallery? It would set templateLock by default?
There was a problem hiding this comment.
Maybe because gallery has N image inner blocks regardless of randomOrder, so getBlockOrder( clientId ).length === 0 never holds 🤔
That's probably it
There was a problem hiding this comment.
I don't see the list view's status changing, though the innerblocks are locked 🤔 I'm probably doing something wrong, or is that expected?
The testing steps in this PR are quite artificial — it'll only work if the Gallery block has no inner blocks. It's much more natural in the dynamic gallery PR, but for this branch, you can use the following markup to get to that state:
<!-- wp:gallery {"randomOrder":true,"linkTo":"none"} -->
<figure class="wp-block-gallery has-nested-images columns-default is-cropped"></figure>
<!-- /wp:gallery -->There was a problem hiding this comment.
I eventually got there when I saw getBlockOrder( clientId ).length === 0 - i inferred my testing from the code. shoulda read the pr desc 😄
There was a problem hiding this comment.
Curious, what's the intended usage for the dynamic gallery? It would would set templateLock by default?
You can see it in the dynamic gallery PR, but it'll set the logic like this:
const innerBlocksProps = useInnerBlocksProps( blockProps, {
defaultBlock: DEFAULT_BLOCK,
directInsert: true,
orientation: 'horizontal',
renderAppender: false,
// In dynamic mode the inner blocks are managed by the source, so lock
// the container with `templateLock: 'all'`. This prevents inserting or
// dragging blocks into it (via `getTemplateLock` -> `canInsertBlockType`),
// and — because a locked container with no inner blocks has nothing to
// navigate — also opts the gallery out of the List View.
templateLock: isDynamic ? 'all' : undefined,
} );
| */ | ||
| export function hasBlockListViewSupport( state, clientId ) { | ||
| const blockName = getBlockName( state, clientId ); | ||
| const hasTypeSupport = |
There was a problem hiding this comment.
Is this the same check as typeHasListViewSupport above? Would hasListViewSupport work for both? Or with an added type if you think that's clearer, though I think the context makes it so.
There was a problem hiding this comment.
Oh, typeHasListViewSupport is gone now 😄
|
I guess template locked + no blocks makes sense in terms of hiding the tab, a user wouldn't be able to insert blocks into a template locked empty block anyway and there are no blocks to view or rearrange. I wonder though, why does a dynamic gallery have no blocks? Is it because a BlockPreview or similar is being used to show the dynamic gallery? The BlockPreview feels like an implementation detail. Using a block preview makes the gallery images
|
fc4e94e to
68a0570
Compare
|
Tiny update: I've made it so this also respects |
ramonjd
left a comment
There was a problem hiding this comment.
I tested this with a gallery block and the code snippet as is, and inside a pattern, synced pattern and template and the block is behaving as expected 🎉
It's looking consistent to me so far. I only had a nitpick comment to offer 😄
ramonjd
left a comment
There was a problem hiding this comment.
This is testing well for me ✅
allowedBlocks + listView combo is a good signal I think. Happy to defer to other reviewers, but LGTM
Just left a non-blocking question around "parent": [ "some/block" ]
94674ec to
dc9e494
Compare
Thanks for the approving review! 🙇 This feels like it's been quite a collaborative PR this one as we've tried out a few different approaches, and I'm indebted to the all the good feedback from you all (@ramonjd, @tyxla, and @talldan). As such I'll leave this PR open overnight just in case there's any last minute feedback or concerns. In my view, I think we've landed on a good shape and heuristic, so if there aren't any objections I'll merge it first thing tomorrow and then rebase the Dynamic Gallery prototype accordingly. |
dc9e494 to
faa9834
Compare
tyxla
left a comment
There was a problem hiding this comment.
Not much more to add, just a few non-blocking comments - this looks pretty solid! Thanks @andrewserong!
| * blocks and its `allowedBlocks` (`[]` or `false`) permits no block: the nested | ||
| * List View panel would render no rows and no appender, so it is hidden rather | ||
| * than shown empty. This is a signal, not a guarantee — a child naming this | ||
| * block as its `parent` stays insertable regardless (see `canInsertBlockType`); |
There was a problem hiding this comment.
This remains something to potentially follow up on as it might occur to some folks as unintended behavior.
A block that supports `listView` is excluded from the List View when it has no inner blocks and disallows insertion (`allowedBlocks: []`) — there is nothing to select, navigate, or add. `hasBlockListViewSupport` reads `allowedBlocks` from block list settings rather than inferring a managed state from `templateLock`, because a content-locked (`contentOnly`) ancestor relaxes a child's own `templateLock: 'all'` to `contentOnly`, whereas `allowedBlocks` is carried through unchanged — so the exclusion holds in every context. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Address review feedback from talldan: - Express the exclusion as a single combined condition — empty inner blocks AND `allowedBlocks: []` — so the "both must hold" intent reads directly. An empty block that still allows insertion keeps its List View (and its appender). Behavior is unchanged. - Drop the "managed inner blocks" framing (it was vague and collided with `controlledInnerBlocks`). Rename the local to `isListViewUnusable` and describe the concrete situation: the List View has nothing to show or add, so it's hidden to avoid bloating the UI. Co-Authored-By: Claude Opus 4.8 <[email protected]>
faa9834 to
cdd94c4
Compare
|
Thanks Marin! I've renamed the private selector to |
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t post Add an optional dynamic mode to the Gallery block where displayed images are resolved at render time from a source instead of hand-picked `core/image` inner blocks. The v1 source is "images attached to the current post". The mode is toggled by the presence of a `dynamicContent` attribute, so existing static galleries are untouched and no deprecation is needed. - block.json: `dynamicContent` attribute + `usesContext` post id/type. - index.php: resolver + dynamic render branch building real `core/image` blocks. - edit.js: preview/InnerBlocks switch, empty-state and toggle entry points, and `allowedBlocks: []` in dynamic mode so the gallery opts out of the List View via core's `shouldRenderBlockListView` (merged separately in #78932). - dynamic-gallery.js / use-dynamic-gallery.js / dynamic-source.js: client-side preview via `useBlockPreview`, source query helpers, and image attr building. - variations.js: a "Dynamic Gallery" inserter variation. - Tests: JS unit tests for the source query and PHP render tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>


What?
Add a per-instance way for a block to disable the List View block support when it's template locked and has no children. The idea is that consumers may want/need to be able to disable the list view tab programmatically rather than just the boolean in
block.json.Why?
It's possible to have blocks that only want to allow the list tab or view to be available based on a certain set of conditions (or a mode). The primary use case I have in mind is that of a Dynamic Gallery block that has two modes: a dynamic one where the inner images are fetched via a query, and a static mode where individual blocks are set within the block. I.e what I'm exploring in #78796
That's just one use case, though. There could be many cases where a block wants or needs to hide its internals based on a condition. So this PR is proposing a general way to do that.
How?
listViewflag, but it turns out we can infer a desired state for hiding the list view tab viaallowedBlock: []orallowedBlocks: false(and the absence of any inner blocks). This state can effectively become a "managed inner blocks" mode.shouldRenderBlockListViewselector to return whether or not the list view support is available — this way we're not just checking if the block type allows it, we're also checking if the block currently supports itFor an example of how a consumer might use this in practice, take a look at #78796 which adds a dynamic gallery block variation (effectively a mode).
Testing Instructions
We can simulate the behaviour in #78796 by updating a block locally to set
allowedBlocks: []. This is quite artificial, the real use case is in the other PR. Here's a diff to apply it to the Gallery block in this branch.In this case, to demo the behaviour, I've wired up switching
allowedBlocks: []to therandomOrdervalue so that you can see it hiding the list tab in the editor:With the above diff applied, if you add the following block markup, you'll see the Gallery block with no list tab:
Note again: this is a very artificial example to help test this branch. The above state is effectively useless on this branch, but it will become useful in the dynamic gallery PR (#78796)
Screenshots or screencast
With the above diff applied for example, the List tab would be hidden if you have randomOrder toggled on with a gallery block that has no children:
Use of AI Tools
Claude Code and Codex