Duotone: Limit SVG filter output to used filters #48995

jeryj · 2023-03-10T15:52:44Z

Rework of #48374
Fixes #38299.

What?

This PR only adds the needed duotone code to the frontend output if that duotone definition is being used.

Why?

There are currently eight globally defined duotone styles, and all of their CSS and SVG definitions are output on the frontend of a WordPress website, even if no images are using the duotone filters. The base raw HTML from TT3 on the Sherbert theme variation results in a 76.4kb request. Without any SVG or duotone-related code, the same page HTML is 67.4kb, which means there is up to 9kb of unnecessary duotone-related HTML on every request.

How?

There's quite a bit of changes required to make this happen. We've gone with a route that adds a new WP_Duotone class that handles what filters need to be added to the page based on what duotone filters, if any, are in use. Here's a general overview of the high-level changes being made, and why:

1. Remove all duotone output from global styles

These are handled by removing the actions related to SVG filters:

remove_action( 'wp_body_open', 'wp_global_styles_render_svg_filters' );
remove_action( 'in_admin_header', 'wp_global_styles_render_svg_filters' );

2. Register all duotone filters from global styles and store in the WP_Duotone class

WP_Duotone::$global_styles_presets is an array that scrapes all of the global styles duotone filters. These could be defined from theme.json globally, per theme, or in custom global styles. We need to do this to have quick access to the duotone filter definitions if we come across a block that needs to access these styles.

3. Register all block-level duotone filters from global styles and store in the WP_Duotone class

WP_Duotone::$global_styles_block_names is an array that scrapes all of the blocks that use a duotone filter from global styles, and stores the slug of the duotone filter it uses, such as default-filter, blue-orange, etc. We need to do this in order to decide if a block being rendered has a duotone filter we need to apply to it.

4. Process all blocks to determine the correct duotone output needed

This is the same as before, but we're only outputting the duotone code if needed. On the 'render_block' filter, we process every block and determine:

Is there a duotone filter we need to apply on this block
If not, return early
If we do, decide which duotone filter we need and how to build it
Add the duotone info we need to WP_Duotone::$output

5. Output the CSS

On the 'wp_enqueue_scripts' action, output all the necessary CSS for duotone filters collected from WP_Duotone::$output`.

6. Output the SVG

On the 'wp_footer' action, output all necessary SVG for duotone filters from WP_Duotone::$output`.

Testing Instructions

These are the different types of Duotone that need testing:

Set the duotone in Global Styles to all blocks of a certain type.
Set the duotone on a per blocks basis.

For each of these it possible to set the duotone to a preset value, a custom value, or unset.
For presets, it is possible to set them using two different formats: var:preset|duotone|blue-orange or var(--wp--preset--duotone--blue-orange).

It is also important to test that only the SVG filters and CSS that is needed on the page is output - rather than outputting all the presets even if they aren't used.

To test an individual block:

Add an image to a post
Click on the "filter" option in the block toolbar
Select a Duotone option
Save the post
Open the post in the frontend and check that the correct Duotone filter has been applied

To test Global Styles

Open the Global Styles interface in the Site Editor
Open "Blocks"
Find the image block
Click on "filter"
Select a Duotone filter
Save the Global Styles settings
Open the site in the frontend and check that all image blocks have the filter applied (unless they specify their own filters)

Testing Instructions for Keyboard

Screenshots or screencast

… duotone presets

This doesn't do anything at the moment. On block render, it checks if the block has duotone filters set via theme.json/global styles/custom styles and returns early if not.

Co-authored-by: scruffian <ben@escruffian.com>

…tone preset

…utput. Name could still be better.

github-actions · 2023-03-10T16:51:59Z

Flaky tests detected in f5499b4.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4420262538
📝 Reported issues:

[Flaky Test] should switch to block tab if we select a block, when Template is selected #48878 in specs/site-editor/settings-sidebar.test.js

Refactored out duotone specific code from theme json gutenberg globla styles. Switched theme_json_register_declarations from action to filter.

Doesn't seem like a legitimately useful filter at the moment. Only for us to be able to use global styles in our duotone-specific output.

We were already getting the presets via the global settings, so the filter was unnecessary.

The filter isn't necesary for us to access what we need. I've removed the refactor as it would be a larger change that can be done in a different PR if needed.

ajlende · 2023-03-14T03:00:26Z

lib/class-wp-theme-json-gutenberg.php

@@ -144,7 +144,7 @@ class WP_Theme_JSON_Gutenberg {
 			'path'              => array( 'color', 'duotone' ),
 			'prevent_override'  => array( 'color', 'defaultDuotone' ),
 			'use_default_names' => false,
-			'value_func'        => 'gutenberg_get_duotone_filter_property',
+			'value_func'        => null, // Don't output CSS Custom Properties for duotone.
 			'css_vars'          => '--wp--preset--duotone--$slug',
 			'classes'           => array(),
 			'properties'        => array( 'filter' ),


I'm pretty sure the entire duotone section of PRESETS_METADATA can be removed since generating the preset is done by duotone.php now.

Actually, it looks like the constructor still uses it for keying presets by origin which we need later. That being said, I still think we can remove the css_vars key and have that solely defined in duotone.php as part of the cleanup.

Also, I noticed that the theme.json styles.blocks['core/block'].filter.duotone filters are being applied on the frontend, but not the editor, so there are also likely changes that need to be done in the block-editor duotone.js hook and use-global-styles-output.js as well.

The properties key is also probably redundant now since we're no longer generating properties here.

…tches

ajlende · 2023-03-14T16:53:42Z

phpunit/block-supports/duotone-test.php

+		return array(
+			'pipe-slug'   => array( 'var:preset|duotone|blue-orange', 'blue-orange' ),
+			'css-var'   => array( 'var(--wp--preset--duotone--blue-orange)', 'blue-orange' ),
+			'css-var-weird-chars'   => array( 'var(--wp--preset--duotone--.)', '.' ),


. isn't a valid identifier token according to the CSS spec.

CSS custom properties always take the first -- branch of this railroad diagram and then allow for [a-zA-Z0-9_-] and a set of non-ascii code points.

It looks like slugs in WordPress are handled by sanitize_title_with_dashes() according to this Stack Exchange answer.

So I would be inclined to further restrict the non-ascii section to characters that get output from that function as well.

And I think somewhere in the global styles code, we also restrict slugs to not contain consecutive hyphens (--) since those would get converted to pipes (|) in the var: syntax.

For some reason, the render_block filter runs even on blocks that are not included on the page. So, if you have a duotone filter defined in your theme.json, such as for the core/post-featured-image block, it will run the block filter even if no featured image is set. As a result, the duotonen defined in theme.json per block would get output even if that block wasn't in use. The only reliable check I could find to determine if the block was in use or not was to check the block content. By bailing early if no block content, we avoid adding unused SVG to the output.

scruffian

I think this is looking great, and in my testing its working well.

IMO we should merge this and deal with any more improvements in followups, as this PR is already very large.

This reverts commit 0bb7a9c.

ajlende · 2023-03-15T15:42:08Z

lib/class-wp-duotone.php

+ *
+ * @access public
+ */
+class WP_Duotone {


Did we figure out if files in the lib folder get the _Gutenberg suffix. If not, this should have that suffix because we want to be able to make changes in Gutenberg without affecting core by referencing the _Gutenberg version.

ajlende · 2023-03-15T16:01:09Z

lib/class-wp-theme-json-gutenberg.php

+	public static function get_preset_css_var( $path, $slug ) {
+		$duotone_preset_metadata = static::get_preset_metadata_from_path( $path );
+		return static::replace_slug_in_string( $duotone_preset_metadata['css_vars'], $slug );
+	}


I think we can get this working without adding this to the public API. The only place the css_vars for frontend duotone is being used is within the block supports now, and we already have it hardcoded a couple times there.

That'd be lovely. For the record, I saw that this PR was reverted at #49102

This version of the PR did get reverted, and then was cleaned up and merged in this PR: #49103

We were able to get the PR merged without adding anything to lib/class-wp-theme-json-gutenberg.php

jeryj and others added 8 commits March 7, 2023 15:13

Adds WP_Duotone class to set-up preset and theme-defined blocks using…

7aa4c4a

… duotone presets

Fix incorrect filter name

23b631b

Check if duotone filter is applied on a block in theme.json

2f63ab2

This doesn't do anything at the moment. On block render, it checks if the block has duotone filters set via theme.json/global styles/custom styles and returns early if not.

Rename duotone class static variables and functions

70581a1

Set WP_Duotone:: array and only output those via wp_footer

a024ddb

Only output used duotone presets

0256548

Co-authored-by: scruffian <ben@escruffian.com>

Cleanup, removal, and only output used SVG filters and CSS

2c283cc

Support var:preset|duotone|default-filter syntax for block name styles

460a413

jeryj self-assigned this Mar 10, 2023

jeryj added 2 commits March 10, 2023 10:11

Refactor to handle both duotone pipe and css var style syntax for duo…

2d0e235

…tone preset

Renamed output_presets to output, as it also includes custom dutone o…

5f47de7

…utput. Name could still be better.

jeryj added 2 commits March 10, 2023 14:40

Fix theme json vs block styles duotone order of application

c39504b

Refactored out duotone specific code from theme json gutenberg globla styles. Switched theme_json_register_declarations from action to filter.

Renamed filter to be more accurate, but should refactor out

1f13855

Doesn't seem like a legitimately useful filter at the moment. Only for us to be able to use global styles in our duotone-specific output.

scruffian mentioned this pull request Mar 10, 2023

Duotone: Output duotone presets per block #48374

Closed

2 tasks

jeryj added 3 commits March 13, 2023 16:53

Simplify getting global styles presets

0c67759

We were already getting the presets via the global settings, so the filter was unnecessary.

Undo theme json class duotone refactoring and remove filter

e41b1e6

The filter isn't necesary for us to access what we need. I've removed the refactor as it would be a larger change that can be done in a different PR if needed.

Remove unused code

45426d2

ajlende reviewed Mar 14, 2023

View reviewed changes

jeryj added 7 commits March 14, 2023 09:41

Rename save_ to set_ class method names

3d224bf

Fix duotone test to use real global preset value

476fe04

Tests for WP_Duotone::gutenberg_get_slug_from_attr()

c23a4a4

Rework gutenberg_get_slug_from_attr to use regex for more accurate ma…

47d86d4

…tches

Tests for WP_Duotone::is_preset

a7fab17

Rework is_preset to also check if the preset exists

af3a236

Remove array_key_exists checks for presets within the block filter

005baa1

ajlende reviewed Mar 14, 2023

View reviewed changes

jeryj added 2 commits March 14, 2023 15:35

Make linter happy

f5499b4

scruffian marked this pull request as ready for review March 15, 2023 10:39

scruffian requested a review from spacedmonkey as a code owner March 15, 2023 10:39

scruffian changed the title ~~WIP: Update/svg filter frontend output~~ Duotone: Limit SVG filter output to used filters Mar 15, 2023

scruffian added the [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") label Mar 15, 2023

scruffian approved these changes Mar 15, 2023

View reviewed changes

jeryj merged commit 0bb7a9c into trunk Mar 15, 2023

jeryj deleted the update/svg-filter-frontend-output branch March 15, 2023 14:25

github-actions bot added this to the Gutenberg 15.4 milestone Mar 15, 2023

ajlende added a commit that referenced this pull request Mar 15, 2023

Revert "Duotone: Limit SVG filter output to used filters (#48995)"

10e18d5

This reverts commit 0bb7a9c.

ajlende mentioned this pull request Mar 15, 2023

Revert "Duotone: Limit SVG filter output to used filters" #49102

Merged

ajlende reviewed Mar 15, 2023

View reviewed changes

jeryj restored the update/svg-filter-frontend-output branch March 15, 2023 16:10

jeryj mentioned this pull request Mar 15, 2023

Duotone: Limit SVG filter output to used filters #49103

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Duotone: Limit SVG filter output to used filters #48995

Duotone: Limit SVG filter output to used filters #48995

jeryj commented Mar 10, 2023 •

edited by scruffian

Loading

github-actions bot commented Mar 10, 2023 •

edited

Loading

ajlende Mar 14, 2023

ajlende Mar 14, 2023

ajlende Mar 14, 2023

ajlende Mar 14, 2023 •

edited

Loading

scruffian left a comment

ajlende Mar 15, 2023

ajlende Mar 15, 2023

oandregal Mar 20, 2023

jeryj Mar 20, 2023 •

edited

Loading

Duotone: Limit SVG filter output to used filters #48995

Duotone: Limit SVG filter output to used filters #48995

Conversation

jeryj commented Mar 10, 2023 • edited by scruffian Loading

What?

Why?

How?

1. Remove all duotone output from global styles

2. Register all duotone filters from global styles and store in the WP_Duotone class

3. Register all block-level duotone filters from global styles and store in the WP_Duotone class

4. Process all blocks to determine the correct duotone output needed

5. Output the CSS

6. Output the SVG

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

github-actions bot commented Mar 10, 2023 • edited Loading

ajlende Mar 14, 2023

Choose a reason for hiding this comment

ajlende Mar 14, 2023

Choose a reason for hiding this comment

ajlende Mar 14, 2023

Choose a reason for hiding this comment

ajlende Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

scruffian left a comment

Choose a reason for hiding this comment

ajlende Mar 15, 2023

Choose a reason for hiding this comment

ajlende Mar 15, 2023

Choose a reason for hiding this comment

oandregal Mar 20, 2023

Choose a reason for hiding this comment

jeryj Mar 20, 2023 • edited Loading

Choose a reason for hiding this comment

jeryj commented Mar 10, 2023 •

edited by scruffian

Loading

github-actions bot commented Mar 10, 2023 •

edited

Loading

ajlende Mar 14, 2023 •

edited

Loading

jeryj Mar 20, 2023 •

edited

Loading