-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Block API: Convert block alias into canonical block (PoC) #61481
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +53 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
...item.initialAttributes, | ||
...variation.attributes, | ||
...( supportsAlias && { | ||
className: getBlockDefaultClassName( variation.name ), |
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.
Maybe it would be better to move this to what generates the markup rather than a pre-set attribute?
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.
Potential problem with this is block validation
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.
Additional custom class names are ignored during validations, so that should be good. That's also a design question whether we want to have two class names printed:
- computed from the original block type
- computer from the alias for the block variation
This looks very promising. Noting that it would potentially address #43743. |
@tjcafferkey and I just had a call where he walked me through this PR and WordPress/wordpress-develop#6523. Thanks Tom! This is looking very promising 😊 I'll need a bit more time to fully catch up, but as a quick brain dump, I was wondering if we could integrate the aliases a bit more tightly with the existing concept of block variations (and potentially avoid setting a <!-- wp:block-alias/separator-variant {"metadata":{"alias":"core/separator"},"className":"wp-block-block-alias-separator-variant"} -->
<hr class="wp-block-separator has-alpha-channel-opacity"/>
<!-- /wp:block-alias/separator-variant --> The way I'm thinking about this is largely as follows: The existing concept of block variations works based on attributes. My go-to example is Core's Social Link block. The markup for the above example is as follows: <!-- wp:social-links -->
<ul class="wp-block-social-links">
<!-- wp:social-link {"url":"wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"tumblr.com","service":"tumblr"} /-->
</ul>
<!-- /wp:social-links --> Note that the editor distinguishes variations of this block (as evidenced by the different icons it uses) based on their Per the code in this PR, if we were to introduce aliases for our block as its author, we would need to add code to set However, this puts an extra burden on block authors, and it feels a bit redundant, since there's a 1:1 mapping from the "distinguishing" attribute ( AFAICS, the existing client-side registration API already contains all the information we need:
So I think it should be possible to rewrite the client-side transforms to use information gathered from whatever registry is used for This might still leave us with the problem of how to handle the server side. I think that we can treat that as a separate problem, and potentially tackle it in a similar way that block registration has evolved over time: Separate mechanisms on the client and server that were eventually bridged by a somewhat shared JSON format (i.e. |
I talked with @gziolo today, and he pointed out one reason why we might actually want to store the reference to the canonical block explicitly: If a plugin defines a block variation, and we start persisting that block in our markup using that variation's alias, then it'd be good to preserve the information what the underlying block is in case that plugin is disabled. E.g. let's say there's a Custom Gallery plugin that ships a <!-- wp:cg/custom-gallery {"someAttribute":"value",...} /--> Now if that plugin is disabled, any code that would include the mapping from alias to attribute value is no longer available, and as a result, WP won't know any longer what to show in the editor. If OTOH the canonical block name is preserved in the markup, WP can degrade more gracefully by replacing <!-- wp:cg/custom-gallery {"metadata":{"canonical":"core/gallery"},"someAttribute":"value",...} /--> by <!-- wp:gallery {"someAttribute":"value",...} /--> In practice, I'm not yet 100% how compelling a reason this is for storing the reference to the canonical name in the markup. While it's nice to degrade gracefully, one could argue that
|
wordpress-develop counterpart: WordPress/wordpress-develop#6523
To do (that are known):
Related to: #43743
What?
Why?
How?
Testing Instructions
core/separator
block with a new alias ofblock-variant/separartor-variant
.Register variant
Hook block
Add variant to template
Testing Instructions for Keyboard
Screenshots or screencast