Add interactivity states UI support via Styles Engine#41708
Closed
getdave wants to merge 15 commits into
Closed
Conversation
Do not merge! This commit has highlighted the need to possibly allow certain CSS props via safecss_filter_attr
|
Size Change: +598 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
getdave
commented
Jun 14, 2022
Comment on lines
+122
to
+128
| $style_defs = gutenberg_style_engine_generate( | ||
| $block_styles, | ||
| array( | ||
| 'selector' => ".$class_name", | ||
| 'css_vars' => true, | ||
| ) | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
This will now include any psuedo selectors
getdave
commented
Jun 14, 2022
| ); | ||
|
|
||
| if ( ! empty( $style_defs['css'] ) ) { | ||
| gutenberg_enqueue_block_support_styles( $style_defs['css'] ); |
Contributor
Author
There was a problem hiding this comment.
This enqueues the <style> tag within the editor which appears directly below the block in the DOM.
getdave
commented
Jun 14, 2022
| const newStyle = cleanEmptyObject( | ||
| immutableSet( | ||
| localAttributes.current?.style, | ||
| [ 'elements', 'link', 'states', 'hover', 'color', 'text' ], |
Contributor
Author
There was a problem hiding this comment.
This structure is intended to marry up the the nesting within the states key of Theme JSON. This allows the Style Engine to pick out the correct color for processing on server and client side.
It's all very hard coded at this point.
getdave
commented
Jun 14, 2022
| @@ -98,28 +99,67 @@ export function getInlineStyles( styles = {} ) { | |||
| // The goal is to move everything to server side generated engine styles | |||
| // This is temporary as we absorb more and more styles into the engine. | |||
| const extraRules = getCSSRules( styles ); | |||
Contributor
Author
There was a problem hiding this comment.
getCSSRules will now generate the rules so that any states rules will have the psuedo selector as the selector prop of the rule.
getdave
commented
Jun 14, 2022
| // Primitive check for pseudo selector. In future | ||
| // we should make this a formal prop of the style rule from getCSSRules. | ||
| pseudoSelector = rule.selector.replace( ':', '' ); | ||
| set( output, [ 'states', pseudoSelector, rule.key ], rule.value ); |
Contributor
Author
There was a problem hiding this comment.
Doing this changes the contract implied by getInlineStyles because now there is a random states key in it whereas before it was just rule -> value.
Not sure how we avoid this. We need to convey the information about pseudo selectors but the data structure isn't conducive to this. Ideas welcomed.
Contributor
Author
There was a problem hiding this comment.
@ramonjd The key / value data structure here isn't conducive to handling pseudo selectors, but we need it to in order to output the correct rules within the editor.
What do you think of my approach? Are we abusing things too heavily here? Or should we change the returned data structure and update all instances within Gutenberg?
This function isn't a public API so we should be ok in that regard.
Your expertise would be greatly appreciated.
Member
There was a problem hiding this comment.
Hi @getdave
Thanks for getting things going here.
It's a tough one. I remember hooks/style.js threw some chilli into the soup when I first look at porting elements over to the JS style engine.
The key / value data structure here isn't conducive to handling pseudo selectors
Totally.
I can't help but recommend a more iterative approach here, that is, first let's get basic support for elements in the style engine in, then deal with new functionality. Similar to what we've been looking at over in #41619
I'm not saying it's the panacea, but it has helped to smooth out progress, and gives us time to consider holistic approaches.
Along those lines I've started on the JS part for elements over here: #41732
I'll also test this out with the new :hover object as well, which might necessitate a new ruleset or constant to inform the style engine about eligible elements.
getdave
commented
Jun 14, 2022
| selector, | ||
| ELEMENTS[ element ], | ||
| elementStyles?.states[ stateKey ], | ||
| `:${ stateKey }` |
Contributor
Author
There was a problem hiding this comment.
I think a util function here which converts from stateKey (hover) to pseudo selector (:hover) would help communicate the intent of the code better.
Contributor
Author
|
Given the changes to the data structure in #41619 this PR will need updated to match the |
Contributor
Author
Absolutely. Happy with that. All these PRs are marked as draft for a reason 😆 It's basically a case of me understanding the limits of the current system and getting familiar with the style engine. Let's take baby steps towards our goal. Happy to close this our as the experiment it was. |
Member
Thanks a lot for taking the time and effort to dive into these questions @getdave The more minds we have on Gutenberg styles the better the outcome. I really appreciate it! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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?
Exploratory PR to add basic interactivity states to link elements via the block UI. Currently only
:hoveris supported onlinkelements.Closes #27075 (or at least address some part of it) and ultimately also closes #38277.
Supersedes #41383.
Builds on #41619 will need rebase once that PR merges.
Why?
Users have been asking for this feature for a long time.
How?
This PR explores
The Style Engine is not setup to cater for pseudo selectors so this PR is refactoring towards making this possible.
Testing Instructions
Screenshots or screencast