Tweaks for #42452 #42691

aristath · 2022-07-26T07:11:50Z

No description provided.

aristath · 2022-07-26T07:13:28Z

packages/style-engine/class-wp-style-engine-css-rules-store.php

+	 *
+	 * @return WP_Style_Engine_CSS_Rules_Store[]
+	 */
+	public static function get_stores() {


A get_stores in the Store class makes sense in order to get all registered/available stores. In all the other PRs I made, this one was always a requirement so I'm adding it here too as it makes it easier to abstract the logic for $stores in the WP_Style_Engine object as well.

aristath · 2022-07-26T07:15:09Z

packages/style-engine/class-wp-style-engine.php

-	/**
-	 * Instance of WP_Style_Engine_CSS_Rules_Store to hold block supports CSS rules.
-	 *
-	 * @var array<string, WP_Style_Engine_CSS_Rules_Store|null>
-	 */
-	private static $stores = array(
-		'layout-block-supports' => null,
-		'block-supports'        => null,
-	);
-


We should not have a hardcoded array of stores. Instead, we can use the WP_Style_Engine_CSS_Rules_Store::get_stores() method to get all available/registered stores. This will make it easier to extend the class and it will work for all future iterations regardless of the store's name.

Makes total sense. 🙇

Just to leave a note on my thinking in relation to these hardcoded stores:

I was trying to find a way to control the number and the name of stores registered in Gutenberg

By having a hard coded list somewhere we can then control the order in which we print each store's styles onto the page. This is important for inheritance and also, later, for CSS layers.

I agree defining them here is not the right place, and we don't need to do right now anyway.

Maybe later we can think about how to tell the style engine/store class to restrict stores and the order in which we store them for the purposes of rendering to the page. 🤔

cc @andrewserong just for a FYI

cc @andrewserong just for a FYI

Thanks for the heads-up, and great points! Yes, it could be a good idea for us to keep an allow-list of stores (somewhere?) to catch accidental typos / ensure that Gutenberg's styles are being added to the right place. We could even use a _doing_it_wrong call to flag when it's being used incorrectly?

and we don't need to do right now anyway. Maybe later we can think about how to tell the style engine/store class to restrict stores and the order in which we store them for the purposes of rendering to the page.

Agreed.

aristath · 2022-07-26T07:16:03Z

packages/style-engine/class-wp-style-engine.php

-		foreach ( static::$stores as $store_key => $store_instance ) {
-			static::$stores[ $store_key ] = WP_Style_Engine_CSS_Rules_Store::get_store( $store_key );
-		}


No longer needed since the $stores was removed. Instead of this, we'll be getting a store directly when needed.

aristath · 2022-07-26T07:18:01Z

packages/style-engine/class-wp-style-engine.php

-		$css_declarations = new WP_Style_Engine_CSS_Declarations( $css_declarations );
-		$stored_css_rule  = static::$stores[ $store_key ]->add_rule( $css_selector );
-		$stored_css_rule->add_declarations( $css_declarations );
+		static::get_store( $store_key )->add_rule( $css_selector )->add_declarations( $css_declarations );


Simplifies the implementation by using the methods other objects in the engine already have

aristath · 2022-07-26T07:19:15Z

packages/style-engine/class-wp-style-engine.php

-			return null;
-		}
-		return static::$stores[ $store_key ];
+		return WP_Style_Engine_CSS_Rules_Store::get_store( $store_key );


Always gets a store. Previously we only allowed 2 store names, this one will get the store if it existed, otherwise it will create a new store and return it.

aristath · 2022-07-26T07:20:10Z

packages/style-engine/class-wp-style-engine.php

+		$stores = WP_Style_Engine_CSS_Rules_Store::get_stores();
+
+		foreach ( $stores as $key => $store ) {
+			$styles = static::compile_stylesheet_from_store( $key );
+
+			if ( ! empty( $styles ) ) {
+				wp_register_style( $key, false, array(), true, true );
+				wp_add_inline_style( $key, $styles );
+				wp_enqueue_style( $key );
+			}
 		}


Gets an array of all stores and enqueued them each one separately.

aristath · 2022-07-26T07:22:02Z

packages/style-engine/class-wp-style-engine.php

-	 *
-	 * @return string A string of classnames separate by a space.
-	 */
-	public function compile_classnames( $classnames ) {


The compile_classnames method was only used in 1 spot, and it just does an implode( ' ', array_unique( $classnames ) ); so nothing complicated that would warrant a new method. The implode & array_unique were simply added to the place where this method was being called.

aristath · 2022-07-26T07:22:35Z

packages/style-engine/class-wp-style-engine.php

@@ -594,40 +578,21 @@ public function compile_css( $css_declarations, $css_selector ) {
 		if ( $css_selector ) {
 			$css_rule = new WP_Style_Engine_CSS_Rule( $css_selector, $css_declarations );
 			return $css_rule->get_css();
-		} else {


else here was not needed since the line above is a `return.

aristath · 2022-07-26T07:23:16Z

packages/style-engine/class-wp-style-engine.php

-		$store = static::get_store( $store_key );
-		if ( $store ) {
-			$processor = new WP_Style_Engine_Processor( $store );
-			return $processor->get_css();
-		}
-		return '';
+		$processor = new WP_Style_Engine_Processor( static::get_store( $store_key ) );
+		return $processor->get_css();


Since the $store now always exists, this method was simplified.

aristath · 2022-07-26T07:23:39Z

packages/style-engine/class-wp-style-engine.php

@@ -678,7 +643,7 @@ function wp_style_engine_get_block_supports_styles( $block_styles, $options = ar
 		}

 		if ( ! empty( $parsed_styles['classnames'] ) ) {
-			$styles_output['classnames'] = $style_engine->compile_classnames( $parsed_styles['classnames'] );
+			$styles_output['classnames'] = implode( ' ', array_unique( $parsed_styles['classnames'] ) );


The compile_classnames method was removed.

aristath · 2022-07-26T07:55:41Z

packages/style-engine/class-wp-style-engine.php

- * @return WP_Style_Engine_CSS_Rules_Store|null The store, if found, otherwise `null`.
+ * @return WP_Style_Engine_CSS_Rules_Store.
 */
 function wp_style_engine_add_to_store( $store_key, $css_rules = array() ) {
-	if ( empty( $store_key ) || empty( $css_rules ) ) {
-		return null;
+	$style_engine = WP_Style_Engine::get_instance();
+	if ( empty( $css_rules ) ) {
+		return $style_engine::get_store( $store_key );
 	}
-	if ( class_exists( 'WP_Style_Engine' ) ) {
-		$style_engine = WP_Style_Engine::get_instance();
-		foreach ( $css_rules as $selector => $css_declarations ) {
-			$style_engine::store_css_rule( $selector, $css_declarations, $store_key );
+
+	foreach ( $css_rules as $selector => $css_declarations ) {
+		if ( empty( $selector ) || empty( $css_declarations ) ) {
+			continue;
 		}
-		return $style_engine::get_store( $store_key );
+		$style_engine::store_css_rule( $selector, $css_declarations, $store_key );
 	}
+	return $style_engine::get_store( $store_key );


Tweaked the wp_style_engine_add_to_store function to always return a store instead of store|null.
This will make the API more robust.

Added test for new method.

ramonjd · 2022-07-27T05:07:06Z

packages/style-engine/class-wp-style-engine-css-rules-store.php

+	 *
+	 * @return void
+	 */
+	public static function remove_all_stores() {


Hope you don't mind, I added this to make testing easier.

ramonjd

Thanks a lot for this @aristath 🙇

I merged the latest change from the base branch and added a couple of tests. Hope that's okay! If you think the remove_all_stores is a bit off or needs renaming, go for it. I mainly added it for the tests.

…y/style-engine-enqueue-block-supports-styles-tweaks

* abstract stores * "else" not needed * compile_classnames method not needed * we have a method to get the store here * Make the wp_style_engine_add_to_store function always return a store * wp_style_engine_get_stylesheet - always return string * Merged with base branch. Added test for new method. Co-authored-by: ramonjd <ramonjd@gmail.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com>

* Enqueuing block support styles version 1. * Linter, this one's for you. * Post trunk merge cleanup and update tests. * Removed spacing around curly braces in CSS rules. Updated tests. We could maybe add a prettify option down the road. Juggling methods around to cater for adding styles to (any) store. Also making return values consistent. * Splitting `wp_style_engine_enqueue_block_supports_styles` and `wp_style_engine_get_block_supports_styles` so we can enqueue styles that don't need parsing, e.g., layout * Integrate the processor class * Migrate layout styles to style engine store. * Update packages/style-engine/class-wp-style-engine.php Co-authored-by: Ari Stathopoulos <aristath@gmail.com> * Tweaks for #42452 (#42691) * abstract stores * "else" not needed * compile_classnames method not needed * we have a method to get the store here * Make the wp_style_engine_add_to_store function always return a store * wp_style_engine_get_stylesheet - always return string * Merged with base branch. Added test for new method. Co-authored-by: ramonjd <ramonjd@gmail.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com> * Adding check for the context argument. Adding tests. * Updating the processor so that it's ignorant of stores. Why? So that it can be used to process any CSS and not just stored CSS. Updating layout for backwards compatibility in gutenberg_get_layout_style (returning the styles that are collected in the function body only) Created a new mode for incoming $css_rules to be a collection of selector + css_declaration keys. Removed wp_style_engine_get_stylesheet (from store) since we don't use it yet Added a new function wp_style_engine_get_stylesheet_from_css_rules() that will process and compile a collection of CSS rules, and not store them. * dump var_dump() * Improve the processor * remove trailing commas - compatibility with PHP < 7.2 * rename css_declarations to declarations * remove unused variable Removing unused function wp_style_engine_get_stylesheet_from_store Updating tests to check for merging and deduping * Switch parse_block_styles from public to protected static * Now that all methods are static, there's no need to call `get_instance()` * Revert get_instance() in wp_style_engine_add_to_store because we want to ensure the hook is enqueued here. * Adding a test for the 'enqueue' flag. * Update lib/block-supports/layout.php Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> * Adding a test for the 'enqueue' flag. * Add named stores to the processor * avoid setting var for something that only gets used once * Only use "else" if absolutely necessary * Add a set_name method * combine & simplify conditions * use empty() instead of isset() checks here * shorten it Co-authored-by: Ari Stathopoulos <aristath@gmail.com> Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>

aristath added 3 commits July 26, 2022 09:56

abstract stores

c2faabe

"else" not needed

c42efdb

compile_classnames method not needed

4782919

aristath commented Jul 26, 2022

View reviewed changes

we have a method to get the store here

68e5806

aristath commented Jul 26, 2022

View reviewed changes

aristath requested a review from ramonjd July 26, 2022 07:23

aristath added 2 commits July 26, 2022 10:44

Make the wp_style_engine_add_to_store function always return a store

3400a9e

wp_style_engine_get_stylesheet - always return string

5e1e444

aristath commented Jul 26, 2022

View reviewed changes

aristath marked this pull request as ready for review July 26, 2022 09:26

Merged with base branch.

79147b9

Added test for new method.

ramonjd reviewed Jul 27, 2022

View reviewed changes

ramonjd approved these changes Jul 27, 2022

View reviewed changes

ramonjd and others added 2 commits July 27, 2022 15:15

Whoops!

b51bbaa

Merge branch 'try/style-engine-enqueue-block-supports-styles' into tr…

bf8f73a

…y/style-engine-enqueue-block-supports-styles-tweaks

ramonjd added [Type] Code Quality Issues or PRs that relate to code quality [Package] Style Engine /packages/style-engine labels Jul 27, 2022

ramonjd assigned aristath Jul 27, 2022

ramonjd merged commit 707b8d5 into try/style-engine-enqueue-block-supports-styles Jul 27, 2022

ramonjd deleted the try/style-engine-enqueue-block-supports-styles-tweaks branch July 27, 2022 05:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweaks for #42452 #42691

Tweaks for #42452 #42691

aristath commented Jul 26, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

ramonjd Jul 27, 2022

ramonjd Jul 28, 2022

andrewserong Jul 28, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

ramonjd Jul 27, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

aristath Jul 26, 2022

ramonjd Jul 27, 2022

ramonjd left a comment

Tweaks for #42452 #42691

Tweaks for #42452 #42691

Conversation

aristath commented Jul 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramonjd left a comment

Choose a reason for hiding this comment