-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Duotone: Limit SVG filter output to used filters #49103
Conversation
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>
…utput. Name could still be better.
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.
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.
There was some duotone specific code within the theme json class that was only used to build a css declaration for duotone css output. It is now handled manually for speed and simplicity.
Flaky tests detected in 1457d58. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4449389586
|
…_var to be more accurately named Also used these functions throughout duotone.php to refactor for simplicity/consistency. Renamed some declaration variables as well to be more accurately named.
Added a few comments for accuracy
lib/block-supports/duotone.php
Outdated
/** | ||
* Safari renders elements incorrectly on first paint when the SVG filter comes after the content that it is filtering, | ||
* so we force a repaint with a WebKit hack which solves the issue. | ||
* | ||
* @param string $selector The selector to apply the hack for. | ||
*/ | ||
function duotone_safari_rerender_hack( $selector ) { | ||
/* | ||
* Simply accessing el.offsetHeight flushes layout and style | ||
* changes in WebKit without having to wait for setTimeout. | ||
*/ | ||
printf( | ||
'<script>( function() { var el = document.querySelector( %s ); var display = el.style.display; el.style.display = "none"; el.offsetHeight; el.style.display = display; } )();</script>', | ||
wp_json_encode( $selector ) | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to move into WP_Duotone_Gutenberg
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow-up, I think it'd be good to run this all at once by passing everything from WP_Duotone_Gutenberg::$output and having one <script> tag that handles rerendering all of them.
It isn't supported in PHP 7.0
*/ | ||
public static function set_global_style_block_names() { | ||
// Get the per block settings from the theme.json. | ||
$tree = WP_Theme_JSON_Resolver::get_merged_data(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeryj @ajlende @aaronrobertshaw @andrewserong We need to use the class bundled by Gutenberg instead of the core class. Fix for this at #49199
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Thanks for getting that fix out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oandregal @ajlende @jeryj it appears that the fix in #49199 surfaced an issue with the duotone action added in this PR, that causes theme.json block node data to be cached too soon preventing the editor-only styles introduced by #46496.
While debugging on trunk, I found that global styles duotone filters aren't being applied in either the post editor (#49239) or the frontend. With regards to #49239, that branch works until you rebase it on trunk where it then fails again to apply the duotone filter.
Update: Found that the image/block.json
file needs the filters.duotone
in the selectors now for the global styles and probably some other tweaks to allow moving away from support.color.__experimentalDutone
.
I wasn't able to find another way to detect we're in the editor for editor-only selectors than via get_current_screen
and the duotone action here appears to be getting the block node data before that function exists.
I'll create an issue for this but also wanted to flag it within the context of this PR.
Edit: New issue - #49324
Rework of #48374
Fixes #38299.
Original PR that was reverted: #48995
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:
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 asdefault-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:
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:
For each of these it possible to set the duotone to a
preset
value, a custom value, orunset
.For presets, it is possible to set them using two different formats:
var:preset|duotone|blue-orange
orvar(--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:
To test Global Styles
Testing Instructions for Keyboard
Screenshots or screencast