Style engine: support nested CSS rules (or CSS containers) #58797
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
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ phpunit/style-engine/class-wp-style-engine-css-rules-container-test.php ❔ lib/load.php ❔ phpunit/style-engine/class-wp-style-engine-css-rules-store-test.php ❔ phpunit/style-engine/class-wp-style-engine-processor-test.php ❔ phpunit/style-engine/style-engine-test.php |
|
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
ramonjd
force-pushed
the
try/style-engine-nested-rules-explosion
branch
2 times, most recently
from
9208e2a
to
975eb35
Compare
# Clean up after rebase Clean up after rebase
Ensuring we can add rule objects to store Tests new container class
ramonjd
force-pushed
the
try/style-engine-nested-rules-explosion
branch
from
b540eb6
to
08211cc
Compare
ramonjd
changed the title
ramonjd
commented
Feb 10, 2024
| @@ -10,7 +10,7 @@ | |||
| if ( ! class_exists( 'WP_Style_Engine_CSS_Rule' ) ) { | |||
|
|
|||
| /** | |||
| * Holds, sanitizes, processes and prints CSS declarations for the Style Engine. | |||
| * Holds, sanitizes, processes and prints CSS rules for the Style Engine. | |||
There was a problem hiding this comment.
Aside from this copy update, these changes just revert those in #58867
| * @package Gutenberg | ||
| */ | ||
|
|
||
| if ( ! class_exists( 'WP_Style_Engine_CSS_Rules_Container' ) ) { |
There was a problem hiding this comment.
New class with a similar interface to WP_Style_Engine_CSS_Rule
This is intentional so that the relevant getter class methods such as get_css/selector can be called where there is a collection of rules + nested rules, e.g., in a single store.
Also so WP_Style_Engine_CSS_Rule doesn't need to care about whether its nested or not, which is the same as the declarations class, and can take care of its own output.
There was a problem hiding this comment.
TODO - WP_Style_Engine_CSS_Rules_Container should extend WP_Style_Engine_CSS_Rule, or even look into adding WP_Style_Engine_CSS_Rules_Container methods to WP_Style_Engine_CSS_Rule itself.
I'd say the former to maintain separation of concerns.
Reason being, with nested CSS, the container rule should also be able to have declarations.
This can be handled by the existing API, indicated by passing a container and not a selector:
wp_style_engine_get_stylesheet_from_css_rules(
array(
'container' => 'fieldset',
'declarations' => array( .... )
),
array(
'container' => 'fieldset',
'container' => 'textarea',
'declarations' => array( .... )
)
)
/*
Output:
fieldset {
...declarations;
textarea {
...declarations;
}
}
*/ |
|
||
| foreach ( $rules as $rule ) { | ||
| if ( ! $rule instanceof WP_Style_Engine_CSS_Rule ) { | ||
| _doing_it_wrong( |
There was a problem hiding this comment.
I can't seem to unit test this using $this->expectNotice();
| */ | ||
| public function get_css( $should_prettify = false, $indent_count = 0 ) { | ||
| $css = ''; | ||
| $indent_count = $should_prettify ? $indent_count + 1 : $indent_count; |
There was a problem hiding this comment.
Bumps the indent count for nested rules
| if ( empty( $this->rules[ $selector ] ) ) { | ||
| $this->rules[ $selector ] = new WP_Style_Engine_CSS_Rule( $selector ); | ||
| $this->rules[ $selector ] = $is_rules_object ? $rule : new WP_Style_Engine_CSS_Rule( $selector ); |
There was a problem hiding this comment.
If the selector is a string create a CSS_Rule. This is legacy behaviour. Containers should be passed as already instantiated.
| @@ -55,7 +63,7 @@ public function add_store( $store ) { | |||
| /** | |||
| * Adds rules to be processed. | |||
| * | |||
| * @param WP_Style_Engine_CSS_Rule|WP_Style_Engine_CSS_Rule[] $css_rules A single, or an array of, WP_Style_Engine_CSS_Rule objects from a store or otherwise. | |||
| * @param WP_Style_Engine_CSS_Rule|WP_Style_Engine_CSS_Rule[]|WP_Style_Engine_CSS_Rules_Container|WP_Style_Engine_CSS_Rules_Container[] $css_rules A single, or an array of, WP_Style_Engine_CSS_Rule objects from a store or otherwise. | |||
ramonjd
added
[Type] New API
Store combines existing rules Store combines
ramonjd
force-pushed
the
try/style-engine-nested-rules-explosion
branch
from
f3e82d7
to
081de82
Compare
|
Flaky tests detected in 1737f72. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7860556773
|
ramonjd
commented
Feb 11, 2024
| } | ||
|
|
||
| if ( $container ) { | ||
| $new_rule = new WP_Style_Engine_CSS_Rules_Container( $container, $new_rule ); |
There was a problem hiding this comment.
A WP_Style_Engine_CSS_Rules_Container also allows us to limit the depth of nested rules to 1
Does the CSS spec have a name for these? |
That's a good question. I haven't found anything really striking so far, other than Maybe "nested-rule" is the closest to the spec I can find. |
In MDN the closest I could find was either "nested" or "conditional group rules" under at-rules... also a bit of a mouthful! |
Edit: or just "parent" |
|
Naming's always hard! I think I lean slightly toward |
5 tasks
|
This was just an experiment. Closing in favour of: |
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.
In progress... this PR will be pared back to satisfy the minimum requirements of #58867. I'm coding "ahead" to validate the approach 😁
What?
Proposed refactor as a follow up to #58867, from which all the ideas flow. Props to @tellthemachines
Provide an API to generate nested CSS rules, e.g.,
Expressed in PHP:
Features:
TODO
@containerqueries) See: Add/try/style engine nested rules explosion/add/grid child spans #58919Why?
To provide a base-line way to add nested rules.
How?
Creating a
WP_Style_Engine_CSS_Rules_Containerthat storesWP_Style_Engine_CSS_Ruleobjects and outputs nested CSS.Accepting a
containervalue in the public API functions of:wp_style_engine_get_stylesheet_from_css_rules()wp_style_engine_get_styles()Testing Instructions
npm run test:unit:php:base -- --group style-engineTesting Instructions for Keyboard
Screenshots or screencast