Add a new public method for getting the selector for a block #45505
Conversation
|
|
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @ingeniumed! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
github-actions
bot
added
the
First-time Contributor
|
@oandregal Something that struck us while making this: There is some overlap in the Technically, the purpose of the two functions is different as the former is for getting the selector of one block while the latter gathers all the blocks, along with some other metadata properties for each block. |
Agree, it seems like it would make sense to implement a 6.2 version of |
| * | ||
| * @return string|null the CSS selector for the block. | ||
| */ | ||
| function wp_theme_get_css_selector_for_block( $block_name ) { |
There was a problem hiding this comment.
The theme and the block are unrelated, so it seems better to rename this to something along the lines of wp_block_get_css_selector.
Note this conversation. A block has more than one selector. This method should be able to retrieve any of the selectors provided by the block.
There was a problem hiding this comment.
It'd be good to update get_blocks_metadata to use this method as well. It'd give us feedback whether this is good in practice or not.
For example, I haven't really dug into this, but just thinking about reusing this method at get_blocks_metadata it struck me that an alternative to this PR could be adding a get_css_selector method to the WP_Block_Type class instead.
There was a problem hiding this comment.
The theme and the block are unrelated, so it seems better to rename this to something along the lines of wp_block_get_css_selector.
Naive question, should we be prefixing this with gutenberg_ prior to it landing in core?
Note this conversation. A block has more than one selector. This method should be able to retrieve any of the selectors provided by the block.
+1 on this comment. Not only can blocks have selectors on a root and per-block-support basis, in the near future we plan on allowing custom selectors for individual styles for example on the color's background.
The point above probably illustrates we'll get even more benefit in future from reusing the results of this PR.
an alternative to this PR could be adding a get_css_selector method to the WP_Block_Type class instead.
Sounds good to me 👍
| */ | ||
| function wp_theme_get_css_selector_for_block( $block_name ) { | ||
| $registry = WP_Block_Type_Registry::get_instance(); | ||
| $blocks = $registry->get_all_registered(); |
There was a problem hiding this comment.
If the method is about a single block, we can use get_registered( $name ) instead.
|
@aaronrobertshaw given your interest and thoughts on #45194 would you available to give feedback on this PR? I'm out next week, but I think we need to connect the work here with the work we talked about over there. |
There was a problem hiding this comment.
Thanks for working on this @ingeniumed 👍
I think @oandregal makes a lot of great points in his feedback to date. I'll add my two cents to those comments inline.
My main thoughts are:
- We'll definitely need to extend this to allow specification which block support (
color,typography) or which individual style (background,border-radius) we want a selector for. - Making this a method on
WP_Block_Typeseems like a great fit to me. - Reusing whatever the result of this PR is in the
get_blocks_metadatamethod is a win in my books.
You can find further history on the feature level selectors in #42087. That PR added the ability to specify different selectors on a per block support basis. A use case for this was to allow border support styles to target the inner img element for an Image block instead of the outer figure wrapper.
| * | ||
| * @return string|null the CSS selector for the block. | ||
| */ | ||
| function wp_theme_get_css_selector_for_block( $block_name ) { |
There was a problem hiding this comment.
The theme and the block are unrelated, so it seems better to rename this to something along the lines of wp_block_get_css_selector.
Naive question, should we be prefixing this with gutenberg_ prior to it landing in core?
Note this conversation. A block has more than one selector. This method should be able to retrieve any of the selectors provided by the block.
+1 on this comment. Not only can blocks have selectors on a root and per-block-support basis, in the near future we plan on allowing custom selectors for individual styles for example on the color's background.
The point above probably illustrates we'll get even more benefit in future from reusing the results of this PR.
an alternative to this PR could be adding a get_css_selector method to the WP_Block_Type class instead.
Sounds good to me 👍
|
Mentioning this PR that @aaronrobertshaw mentioned to me as it relates to what @alecgeatches and I are proposing here. |
Thanks for linking the above PR 👍 It is a work in progress but a first pass at achieving the goals set out in #45194 that was linked earlier in this PR discussion. It would eventually make the block's Unfortunately, I'll be AFK until early January. So #46496 is unlikely to land before then unless someone else would like to pick that one up. |
|
Closing my PR given the changes that we wanted for this PR have already gone in, in #46496 |
What?
This PR came about from the conversation that was had here between @oandregal, @alecgeatches and I about having a public method to get the selector for a block instead of exposing the `get_blocks_metadata function.
Why?
Given the recent drive to do a code quality check for theme.json APIs, it was mentioned to not expose the
get_blocks_metadatafunction like we proposed in this PR. Instead, it's better to make a simpler public method that when given the block name would return the selector for that block.This is using existing cached methods to do its job so the lookup time is minimal, and it ensure it only returns the basic information needed which is the selectors.
How?
It's using a simpler version of the
get_blocks_metadatafunction by:Testing Instructions
Make a test plugin that essentially calls the following:
$looked_up_selector = wp_theme_get_css_selector_for_block( $block_name );for a block. Ensure the right value is given back.