Allow opt-out of pattern overrides #60066
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevin940726
added
[Type] Enhancement
|
Size Change: +330 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
I still hold the opinion that the default state of whether something is overwritable should be false. Even when named. The main interface for renaming a block is the list view. Not the advanced inspector controls. To me, this does not really solve the full UX issue. From my point of view, it is a bad experience to have a side-effect on a feature as common as renaming. Like I've said before. Users are so used to the fact that they can rename files, layers, elements, etc. in other pieces of software / operating systems. And that action should not have a side-effect. I'm fine with using the name as the underlying API for us to determine what overrides are connected to instead of a unique ID (even though I still think that fixing a typo in a name should not mean that all instances lose the connection because it is really hard for users to understand why that happened) |
|
@fabiankaegy I actually agree with everything you said! I still decided to create this draft PR so that people can test the flow themselves and decide which option is better. Now that the original tracking issue is getting too big and closed, I recommend sharing your opinions on the new epic issue #59819, which more people would see! My intention is to create more working examples (like this PR) and potentially guide the feature towards an acceptable state for everyone. |
This was referenced Mar 26, 2024
One aspect of this that might need further consideration is support for mixing and matching different types of bindings. An option could be to mark individual attributes as Pinging some block bindings contributors (cc @gziolo @SantosGuillamot @artemiomorales @michalczaplinski) |
|
Maybe I'm missing something but, if what you want is not to use pattern overrides, why don't you directly delete the binding from the block attributes instead of setting it to |
|
There's a seperate proposal to not have any binding attributes - Use metadata.name only as the hint for pattern overrides. |
22 tasks
In the current implementation, once a |
|
So, let me ensure I understand it correctly. What you want is:
Is that right? If that's the case, I want to consider it better because we are relying on the existence of the |
|
Would having a "default" option like discussed in this issue help? We could potentially have:
"metadata": {
"name": "custom-name",
"bindings": {
"__default": "pattern/overrides"
}
}
"metadata": {
"name": "custom-name",
"bindings": {
"__default": "core/pattern-overrides",
"content": "core/post-meta",
}
}
"metadata": {
"name": "custom-name",
"bindings": {
"content": "core/pattern-overrides",
}
}
|
|
Overall, block bindings are opt-in. In effect, developers or users must explicitly set the connection for the individual attribute. It's a contract that ensures that there is full control over what is bound. The presence of the |
|
@gziolo do your concerns still apply if pattern overrides were opt-in only as outlined here #59812 (comment) ? |
|
It does seem to be moving towards it being opt-in now, so potentially the solution outlined above by @SantosGuillamot adding a |
No concerns regarding opt-in. It's been opt-in from the start for Pattern Overrides and works out of the box on the server with |
|
Closing as since #60234 this feature requires opt-in by default. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Supersedes #59583. Attempts to close #59812.
Add an opt-out toggle for pattern overrides.
Why?
The current implementation of pattern overrides relies on the block's name (Block Renaming API) to enable the feature, but it lacks an opt-out mechanism to just name the block without enabling the feature.
How?
This PR adds a toggle box next to the block name control, which defaults to true, and disables the feature when it's toggled off.
The design is similar to what @SaxonF shared in #59813 (comment) (the last one).
Technically, we set the
bindingstofalseto disable the feature. This is not "officially supported" but it seems to work 🤷.Testing Instructions
heading,paragraph,image, andbutton) to the patterns and give them block names (in the editor settings -> Advanced panel)bindingsattribute tofalse.Screenshots or screencast
Kapture.2024-03-21.at.16.30.13.mp4