Skip to content
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

Pattern Overrides: individual binding attributes are saved in the pattern markup and don't auto-update #58601

Closed
talldan opened this issue Feb 2, 2024 · 6 comments · Fixed by #60694
Assignees
Labels
[Feature] Block bindings [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@talldan
Copy link
Contributor

talldan commented Feb 2, 2024

What problem does this address?

When creating a pattern and toggling 'Allow instance overrides' for a block, the block receives some attribute data that looks like this:

"bindings":{"url":{"source":"core/pattern-overrides"},"title":{"source":"core/pattern-overrides"},"alt":{"source":"core/pattern-overrides"}}}

This is the block bindings declaration, that declares that image block url, title and alt can be overriden within patterns.

A problem arises when the block bindings feature supports more attributes in the future. Because this data is saved in a post type, it isn't easy to update after the fact.

What is your proposed solution?

Don't use the bindings property for pattern overrides, and instead rely on the presence of a name

As mentioned in #59268 (comment), an option is to consider any named block as bound, and not use the bindings attributes at all for pattern overrides.

This may require some modifications to the block bindings API

Wildcard binding

Because patterns generally mark entire blocks as supporting overrides, if there were a way to declare that all supported attributes should be overridden, that might solve the issue ... perhaps using a wildcard?

"bindings":{"*":{"source":"core/pattern-overrides"}}}

Something that may be important to consider is that users may want to be able to mix bindings in the future, so inside a pattern they could also declare a binding to post meta, potentially even on a block that already supports overrides. I think that could still work here:

"bindings":{"*":{"source":"core/pattern-overrides"}, "url":"core/post-meta"}}

Though use cases are possibly unclear.

@talldan talldan added [Type] Enhancement A suggestion for improvement. [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Feature] Block bindings labels Feb 2, 2024
@gziolo
Copy link
Member

gziolo commented Feb 2, 2024

What would be your proposal to mark individual attributes as bindable? Is it enough to detect that they are sourced from HTML? What if the block author doesn't want to allow binding custom values?

@talldan
Copy link
Contributor Author

talldan commented Feb 2, 2024

What would be your proposal to mark individual attributes as bindable? Is it enough to detect that they are sourced from HTML? What if the block author doesn't want to allow binding custom values?

It sounds like a separate issue to me. First it seems like the server-side replacement needs to be reliable enough for a general rollout, but at that stage, the ability to opt-in or out via configuration could be an option (block supports?)

@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Feb 2, 2024

Do you know if there are any reserved words for block attributes? I was thinking of using "default", but I guess someone could name an attribute "default", right?

"bindings":{"default":{"source":"core/pattern-overrides"}}

@gziolo
Copy link
Member

gziolo commented Feb 2, 2024

Is it the issue with Block Bindings or the very special way Pattern Overrides enable connection where it's all or nothing? My question still holds, but let me rephrase it – what if the pattern author doesn't want to allow overrides to all attributes for a given block?

@talldan
Copy link
Contributor Author

talldan commented Feb 5, 2024

Do you know if there are any reserved words for block attributes? I was thinking of using "default", but I guess someone could name an attribute "default", right?

I don't think so, though it hasn't really been a concern in the past. Attributes like metadata, styles have been introduced. I would possibly consider calling it something like __default to reduce the risk.

Is it the issue with Block Bindings or the very special way Pattern Overrides enable connection where it's all or nothing? My question still holds, but let me rephrase it – what if the pattern author doesn't want to allow overrides to all attributes for a given block?

Thanks for clarifying the question.

Lots of this comes down to the desired user experience, so maybe @SaxonF has some ideas around what the experience for selecting overrides might be in the future. I do recall some past conversation about it being opt-out, so all blocks in the pattern a user creates would be automatically bound, but then the user can deselect individual blocks.

I don't know if we'd also have the ability to deselect or prevent individual attributes from being editable. If we do, that would suggest we need a way to specify 'all' attributes, but then also de-list particular ones that the user selects.

@talldan
Copy link
Contributor Author

talldan commented Mar 1, 2024

Surfacing this comment - #59268 (comment).

Seems like a good suggestion that would also fix this issue.

@talldan talldan changed the title Pattern override: individual binding attributes are saved in the pattern markup and don't auto-update Pattern Override: individual binding attributes are saved in the pattern markup and don't auto-update Mar 13, 2024
@talldan talldan changed the title Pattern Override: individual binding attributes are saved in the pattern markup and don't auto-update Pattern Overrides: individual binding attributes are saved in the pattern markup and don't auto-update Mar 13, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants