Update state support to use config instead of block supports#78088
Conversation
|
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. |
|
Size Change: -5 B (0%) Total Size: 7.95 MB 📦 View Changed
ℹ️ View Unchanged
|
| * @return WP_Block_Type | ||
| */ | ||
| private function register_block_with_states( $block_name, $selectors = array() ) { | ||
| private function ensure_block_registered( $block_name, $selectors = array() ) { |
There was a problem hiding this comment.
I don't know why codex changed all this 🤔
There was a problem hiding this comment.
Do the tests still pass if you change it back? 😄
There was a problem hiding this comment.
Actually, I do know why it changed the block names - because the support is now hard-coded to specific blocks, the tests need to use those blocks too.
I guess this function name change was to match the added check for whether the block is registered. I'm not too fussy about it with it being test code.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
tellthemachines
left a comment
There was a problem hiding this comment.
I agree that we shouldn't expose states as a public block support API yet. As mentioned in the comment below, we probably need to curate the options we enable now a little more.
| } | ||
|
|
||
| $supported_states = $block_type->supports['states'] ?? null; | ||
| $supported_states = WP_Theme_JSON_Gutenberg::VALID_BLOCK_PSEUDO_SELECTORS[ $block_name ] ?? null; |
There was a problem hiding this comment.
Hmm, might be better to hardcode the button block here if we want to preserve the exact feature set of #76491. That PR only adds state support to button.
I'm not sure what @MaggieCabrera's intention is with the Nav link block, but right now there aren't that many viable styling options for pseudo states available in the UI:
The other thing is that the Button block.json states in #76491 specifies only hover, focus and active, whereas here we're getting the full set including focus-visible:
I'm not sure why we'd want to style both focus and focus-visible because they seem to apply in pretty much the same circumstances. #68521 that added it isn't very clear on the matter.
We might want to hardcode supported pseudo states in here too.
There was a problem hiding this comment.
:focus-visible was added to be consistent with the global styles. Probably the same with the nav link block, I didn't notice it was missing in the original PR.
Happy to change both, though I think it might be good to be consistent across global styles / block instances, else it becomes a job to track what's missing.
Maybe nav link support not being added is related to the @current state 🤔
There was a problem hiding this comment.
Nav link support is being added in the PR that adds @current UI: #77271
It feels weird in that PR too. About the only style I can imagine wanting to change in that context is text-decoration; all the others would be very awkward.
The thing about focus-visible is it just overrides focus, so it's pretty useless having both, and makes for confusing UI (which should I use? what's the difference between them?). But I don't know why we added support for styling focus-visible in the first place, so I'm probably missing something.
| * @return WP_Block_Type | ||
| */ | ||
| private function register_block_with_states( $block_name, $selectors = array() ) { | ||
| private function ensure_block_registered( $block_name, $selectors = array() ) { |
There was a problem hiding this comment.
Do the tests still pass if you change it back? 😄
|
Question: Could we not leverage the gutenberg experiments option for this instead? That way we can iterate and explore alternative API's whilst we are working on this all. And then when we want to ship it in core it becomes available for all to use? 🤔 |
The disadvantage of hiding this behind an experiment flag is that we don't get any real testing or feedback on it. I think that if the behaviour is stable and we're not exposing any public APIs prematurely it's usually preferable to make features progressively available to users without the experiment flag. Having said that, is there a specific reason you're wary of exposing this functionality too soon? Or would you like to be able to leverage the experiment to add pseudo states to custom blocks, so a public API would be useful? |
|
Even for the plugin there's a back compat policy of two releases: So it'll just slow things down to keep this as a public api. Also, as mentioned in the PR description, I think global styles needs to be considered at the same time. There's no point having global styles support hard-coded to particular blocks, but block instance support opened up as a public API. I think if anything the other way around would be an option (initially). |
tellthemachines
left a comment
There was a problem hiding this comment.
Let's merge this into #76491 and continue working on that one!
* Block Editor: Use config for block pseudo states * Block supports: Render pseudo states from config * Block Editor: Support nested block style state paths
* Block Editor: Use config for block pseudo states * Block supports: Render pseudo states from config * Block Editor: Support nested block style state paths
* first try to create states supports * added tests and fixed css specificity and selectors * refactor to use selectors instead of a new experimental attribute * added unit tests * add toggle to show the changes on canvas * fix linting * fixed dependencies * add the rest of the panels to the states * use the proper panels for the non default states * fix reset filter and added tests * add spacing to toggle * Simplify HTML processor logic in states block support Replace the $found flag and double WP_HTML_Tag_Processor instantiation with a single processor using if/elseif branching. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactored props and BlocksStatesControl * extract shared hook for block states * Remove internal SlotFills from private-apis * Add useBlockStyle( path, state ) hook for block instance style reads/writes * Use useBlockStyle in DimensionsPanel, replacing useBlockStateProps * Use useBlockStyle in BackgroundImagePanel, replacing useBlockStateProps * Use useBlockStyle in BorderPanel and TypographyPanel, replacing useBlockStateProps * Fix useBlockStyle to use updateBlockAttributes instead of setAttributes from context * Remove useBlockStateProps now that all panel hooks use useBlockStyle * Use Symbol key for BlockInspectorPreTabs slot/fill to keep it internal * Rename __experimentalStates support key to states * fix the other slotFill * Use buildStateResetAllFilter in BackgroundInspectorControl * Add tests for useBlockStyle hook * fix test failures * added block.json schema * Use style engine store for states block support CSS output, matching layout.php and position.php * use important only in the cases that need it * Update state support to use config instead of block supports (#78088) * Block Editor: Use config for block pseudo states * Block supports: Render pseudo states from config * Block Editor: Support nested block style state paths * Scope block support reset all to selected style state and remove useBlockStyle hook. (#78141) Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> * Show block instance state badges (#78142) * Show block state badges in inspector * Move state control badges to separate component * Render state badges from global styles header * Derive active states in state badges * Use useToolsPanelDropdownMenuProps for block instance state control menu * Fix duplicate import * Show pseudo state global styles on canvas when "Show state on canvas" is active (#78190) * Show global styles on the canvas for pseudo states when Show state on canvas is active * Use existing merge helper * Improve accuracy of useMemo deps * Fix border styles for block instance pseudo states (#78195) * Fix border styles for block instance pseudo states * Try adding !important to authored border-style rules * Revise/simplify PHP border fallback to inject declarations * Revert editor fallback border style changes for pseudo states * Compile border style fallbacks seperately to avoid setting !important * Add missing !important properties * Use existing cleanEmptyObject util * Update outdated comment * Avoid using symbol key for BlockCardControls since it is never exported * add core changelog * Revert "Avoid using symbol key for BlockCardControls since it is never exported" This reverts commit 9f4e1fe. --------- Unlinked contributors: onetrev, davewhitley, itsdavidmorgan, le-sgs. Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org> Co-authored-by: scruffian <scruffian@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: paaljoachim <paaljoachim@git.wordpress.org> Co-authored-by: HILAYTRIVEDI <hilayt24@git.wordpress.org> Co-authored-by: javierarce <javiarce@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: mtias <matveb@git.wordpress.org> Co-authored-by: carolinan <poena@git.wordpress.org> Co-authored-by: eric-michel <ytfeldrawkcab@git.wordpress.org> Co-authored-by: cbirdsong <cbirdsong@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: shaunandrews <shaunandrews@git.wordpress.org> Co-authored-by: nathanrodrigues2111 <nathanrbsf@git.wordpress.org> Co-authored-by: masteradhoc <masteradhoc@git.wordpress.org> Co-authored-by: deryckoe <deryck@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jarekmorawski <jarekmorawski@git.wordpress.org> Co-authored-by: simonwammerfors <simonwammerfors@git.wordpress.org> Co-authored-by: mateuswetah <tainacan@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: colinduwe <colind@git.wordpress.org> Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org> Co-authored-by: hanneslsm <hanneslsm@git.wordpress.org> Co-authored-by: JiveDig <jivedig@git.wordpress.org> Co-authored-by: Bovelett <annebovelett@git.wordpress.org> Co-authored-by: hadamlenz <adrock42@git.wordpress.org> Co-authored-by: ethanclevenger91 <eclev91@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org>
What?
Based on #76491 and proposes to merge into that branch.
I think it might be too early to consider using a public api like block supports for the states feature. Some reasons:
This PR switches to using an internal config driven approach. There's already quite a lot of config for states (e.g. for the labels, for global styles rendering), so this matches the existing approaches.
The repercussion is right now third-party blocks won't be able to enable pseudo states, but I think this can be re-introduced once state-based features (responsive, pseudo and nav link current) are more finalized.
Testing Instructions
It should work the same as in trunk: