contentOnly pattern experiment: add inspector 'fields' support for Pattern Overrides and Block Bindings

[talldan]

What problem does this address?

The inspector fields currently doesn't support Pattern Overrides or any other attributes that have block bindings.

To reproduce this:

1. Insert a synced pattern that has overrides
2. Note that the inspector fields are displayed and can be edited, however any changes aren't persisted.

https://github.com/user-attachments/assets/f30f7684-deb1-493a-8008-6adff4788eb3

What is your proposed solution?

The reason behind the issue is that block bindings hooks into (or monkey patches) a block's setAttributes calls.

The inspector fields instead dispatch the updateBlockAttributes action, which doesn't have any handling of block bindings.

[talldan]

I've been looking into options for fixing this. I think it's high priority issue that needs some attention. The only short term option is to remove fields support for bound attributes. Not having that support would be a problem IMO (no pattern overrides support, no navigation links or other bound fields).

As mentioned in the issue, the core of the problem is that block bindings only hooks into a block's setAttributes calls to perform its processing (the same setAttributes function that's passed as a prop to a block's edit component), while the ContentOnlyControls (inspector fields) use the updateBlockAttributes action, a lower level API that doesn't process block bindings.

This is the code that runs whenever a block calls setAttributes, setBoundAttributes essentially monkey patches the original setAttributes implementation: https://github.com/WordPress/gutenberg/blob/81978f806b2e879a844e8da5b891b3b016dab340/packages/block-editor/src/components/block-edit/edit.js#L188-L268

The same is also true of the attributes prop passed into a block, this is augmented with bindings values and called computedAttributes in the code. This is where the computedAttributes and setBoundAttributes are passed to a block: https://github.com/WordPress/gutenberg/blob/81978f806b2e879a844e8da5b891b3b016dab340/packages/block-editor/src/components/block-edit/edit.js#L296-L302

Options for fixing this:

Option 1: Refactor block bindings so that the updateBlockAttributes action and getBlockAttributes selector also processes bindings

This would be the ideal solution as it solves the root of the problem, but it's a deep rabbithole unfortunately.

The reason block bindings are implemented in a React component and not within the block editor store, is because the bindings depends on and needs access to block context. Block context is implemented as a React context provider, so there's no way to access a block context value from the block editor store. This is what prevents bindings from being integrated into store selectors and actions, and why it has been implemented like it is today.

So a first step to solving the problem would be refactoring Block Context to the block editor store. After that, block bindings can be integrated with the updateBlockAttributes action and the getBlockAttributes selector.

Option 2: Move/Refactor ContentOnlyControls so that it renders somewhere that it has access to setAttributes and attributes props

This is an easier option, but with trade-offs. The idea is that each block would render an individual DataForm for its ContentOnlyControls somewhere where we can pass in the setAttributes and attributes props (the versions of those props that have the binding integration). Rendering the form somewhere in packages/block-editor/src/components/block-edit/edit.js file makes sense to me, or alternatively via the 'editor.BlockEdit' filter. I see no issue with adding it straight to BlockEdit though, it's a block editor feature, not an extension.

A SlotFill would need to be used to make the forms portal into the block inspector.

There is a drawback. The ContentOnlyControls component does some 'intelligent' flattening of the block hierarchy if a block only has a single inner block, and uses a drilldown where blocks have multiple inner blocks: https://github.com/WordPress/gutenberg/blob/a4de953b608164a20889b3a5090b049b967988e0/packages/block-editor/src/components/content-only-controls/index.js#L492-L502

I don't think this will be as easy to achieve with the SlotFill idea. The implementation becomes tied to however blocks render. Though it might be good to go back to something simpler.

[ramonjd]

Thanks for laying out the issues @talldan

Forgive me if these questions aren't relevant or accurate, I'm trying to understand the options.

Firstly, which one do you think would provide longer term stability and flexibility? Is there a way to do it iteratively?

For what it's worth I got Claude to vibe code something that doesn't work really 😄

The ContentOnlyControls component does some 'intelligent' flattening of the block hierarchy if a block only has a single inner block, and uses a drilldown where blocks have multiple inner blocks:

For Option 2, is the drilldown a hard requirement? Is there another way these blocks could be rendered, e.g., expandable section?

If it ends up being a rabbit hole, I suspect it's another good reason to separate the inspector fields functionality from a development (and UX) perspective

• https://github.com/WordPress/gutenberg/issues/73532

[ramonjd]

Another question. Is it assumed that we want the exact experience for editing the original synced pattern when in "spotlight mode"? What is it we're trying to build?

Looking at these designs, I'm not sure they cover overrides:

• https://github.com/WordPress/gutenberg/issues/73466

[talldan]

Firstly, which one do you think would provide longer term stability and flexibility? Is there a way to do it iteratively?

The first option solves the root cause, so it's the better longer term option.

There are probably ways to ship parts iteratively. Depends which option is taken. I think I already wrote enough hard to understand text above.

For Option 2, is the drilldown a hard requirement? Is there another way these blocks could be rendered, e.g., expandable section?

Anything like that where blocks need to be grouped becomes harder to solve, because it entails giving up some control about how the controls for each block render, it becomes part of the normal block rendering process.

Another question. Is it assumed that we want the exact experience for editing the original synced pattern when in "spotlight mode"? What is it we're trying to build?

I mentioned it already above, it isn't only about pattern overrides, adding support ensures the Navigation Link block and other blocks that might have bindings work properly.

[talldan]

I had a go at Option 2 in this PR - #73863. Unfortunately it doesn't work (I mentioned the problems on the PR), and I'm not sure the issues are solvable. 🤔

[talldan]

I had a go at Option 2 in this PR - https://github.com/WordPress/gutenberg/pull/73863. Unfortunately it doesn't work (I mentioned the problems on the PR), and I'm not sure the issues are solvable. 🤔

The problem I mentioned on the issue is that the AsyncModeProvider that's used to optimize updates to blocks results in fields not displaying in the expected order. I think I could also look at updating the AsyncModeProvider to fix that. It's this logic for the value that would need to change: https://github.com/WordPress/gutenberg/blob/d3734f5e4839d0cf5ada56fedb253fae26c55116/packages/block-editor/src/components/block-list/index.js#L257-L265

Some extra boolean logic would need to be added to exempt contentOnly blocks that are in patterns. It might have performance implications, but I won't know how severe until I try it.

The proposal in #73845 does make my PR more feasible, as it simplifies how the fields are displayed.

[ockham]

Is there any way we could go with option 1 without requiring access to block context from the block editor store -- instead providing it to whatever action or selector upon invocation? Here's a recent example of a selector that does that.

I agree that the dependency on block context makes working with block bindings harder than it should be, but it might still be viable to have inspector controls rely on block context from a principled POV (i.e. not limited to block bindings).

[talldan]

Is there any way we could go with option 1 without requiring access to block context from the block editor store -- instead providing it to whatever action or selector upon invocation? Here's a recent example of a selector that does that.

At the moment where the block fields are rendered has no way to access the context for each block (the fields are all rendered from the point of view of the parent pattern), so I think it would entail Option 2 anyway (and at that point it becomes possible to use setBoundAttributes and the computed attributes so it's no longer an issue).

I did also consider if there might be a way to augment the selectors/actions with the block context without refactoring it, but I couldn't see a way to do that without some very messy code.