Skip to content
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

Group duotone outputs and refactor rendering #49705

Merged
merged 4 commits into from
Apr 18, 2023

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Apr 11, 2023

What?

Part 3 in a set of duotone php refactoring to fix small issues in duotone rendering.

This part is the refactoring that changes the most duotone rendering logic. It moves the stylesheet genration (style engine) code into its own action that happens after all blocks have rendered.

Also included is a small fix for classic themes that would always try outputting a stylesheet even if no duotone filters were present.

Why?

  • Grouping the outputs will reduce the number of calls to the style engine that we have.
  • In the future, SVG output can be optimized to wrap all definitions in a single <defs> group instead of having nested SVGs.
  • Refactoring should help make the code more understandable, especially with the updates in part 4.
  • Classic themes shouldn't include an empty duotone stylesheet if no duotone filters are present.

How?

  • Creates a new output_block_styles as a companion to output_global_styles that handles the style engine rendering for all blocks in a single go.
  • Refactors the code to reduce duplication. This is most apparent for SVG generation between editor.
  • Adds a check for duotone filters before outputting anything.

Testing Instructions

  1. Ensure that duotone filters in all of the example content render the same in the post editor, site editor, and frontend.
  2. Test that the custom filters render in classic themes. You can use the same example content which includes some.
Example content for TT3 block-out, canary, and pilgrimage variations
<!-- wp:heading -->
<h2 class="wp-block-heading">Image core preset</h2>
<!-- /wp:heading -->

<!-- wp:image {"style":{"color":{"duotone":"var:preset|duotone|midnight"}}} -->
<figure class="wp-block-image"><img src="https://loremflickr.com/640/360" alt="placeholder image" class=""/><figcaption class="wp-element-caption">Preset slug: midnight</figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Image theme preset</h2>
<!-- /wp:heading -->

<!-- wp:image {"style":{"color":{"duotone":"var:preset|duotone|default-filter"}}} -->
<figure class="wp-block-image"><img src="https://loremflickr.com/640/360" alt="placeholder image" class=""/><figcaption class="wp-element-caption">Preset slug: default-filter</figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Image custom filter</h2>
<!-- /wp:heading -->

<!-- wp:image {"style":{"color":{"duotone":["rgb(92, 51, 10)","#fcf2e8"]}}} -->
<figure class="wp-block-image"><img src="https://loremflickr.com/640/360" alt="placeholder image" class=""/><figcaption class="wp-element-caption">Custom sepia filter</figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Image unset</h2>
<!-- /wp:heading -->

<!-- wp:image {"style":{"color":{"duotone":"unset"}}} -->
<figure class="wp-block-image"><img src="https://loremflickr.com/640/360" alt="placeholder image" class=""/><figcaption class="wp-element-caption">This should never have a filter applied</figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Image theme.json styles (plain block)</h2>
<!-- /wp:heading -->

<!-- wp:image {"style":{"color":{}}} -->
<figure class="wp-block-image"><img src="https://loremflickr.com/640/360" alt="placeholder image" class=""/><figcaption class="wp-element-caption"><code>settings.styles.blocks['core/image'].filter.duotone</code></figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Cover core preset</h2>
<!-- /wp:heading -->

<!-- wp:cover {"url":"https://loremflickr.com/640/360","dimRatio":0,"style":{"color":{"duotone":"var:preset|duotone|midnight"}}} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim"></span><img class="wp-block-cover__image-background" alt="placeholder image" src="https://loremflickr.com/640/360" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Preset slug: midnight</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Cover theme preset</h2>
<!-- /wp:heading -->

<!-- wp:cover {"url":"https://loremflickr.com/640/360","dimRatio":0,"style":{"color":{"duotone":"var:preset|duotone|default-filter"}}} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim"></span><img class="wp-block-cover__image-background" alt="placeholder image" src="https://loremflickr.com/640/360" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Preset slug: default-filter</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Cover custom filter</h2>
<!-- /wp:heading -->

<!-- wp:cover {"url":"https://loremflickr.com/640/360","dimRatio":0,"style":{"color":{"duotone":["rgb(92, 51, 10)","#fcf2e8"]}}} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim"></span><img class="wp-block-cover__image-background" alt="placeholder image" src="https://loremflickr.com/640/360" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Custom sepia filter</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Cover unset</h2>
<!-- /wp:heading -->

<!-- wp:cover {"url":"https://loremflickr.com/640/360","dimRatio":0,"style":{"color":{"duotone":"unset"}}} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim"></span><img class="wp-block-cover__image-background" alt="placeholder image" src="https://loremflickr.com/640/360" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">This should never have a filter applied.</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Cover theme.json styles (plain block)</h2>
<!-- /wp:heading -->

<!-- wp:cover {"url":"https://loremflickr.com/640/360","dimRatio":0,"style":{"color":{}}} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim"></span><img class="wp-block-cover__image-background" alt="placeholder image" src="https://loremflickr.com/640/360" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…"} -->
<p class="has-text-align-center">This depends on <code>settings.styles.blocks['core/cover'].filter.duotone being set</code> to the CSS custom property for a preset.</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

@ajlende ajlende added [Type] Bug An existing feature does not function as intended [Type] Code Quality Issues or PRs that relate to code quality labels Apr 11, 2023
@ajlende ajlende self-assigned this Apr 11, 2023
@github-actions
Copy link

github-actions bot commented Apr 11, 2023

Flaky tests detected in ad8f98c.
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/4734482802
📝 Reported issues:

@scruffian scruffian force-pushed the fix/deprecate-remaining-global-duotone-functions branch from 0e7119c to 6adec60 Compare April 18, 2023 11:49
Base automatically changed from fix/deprecate-remaining-global-duotone-functions to trunk April 18, 2023 15:41
@scruffian scruffian force-pushed the fix/refactor-group-duotone-output branch from 10fd25d to ad8f98c Compare April 18, 2023 16:02
Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +77 to 90
* being output on the page. Organized by an id of the preset/color group and the information needed
* to generate the SVG and CSS at render.
*
* Example:
* [
* 'blue-orange' => [
* 'wp-duotone-blue-orange' => [
* 'slug' => 'blue-orange',
* 'colors' => [ '#0000ff', '#ffcc00' ],
* ],
* 'wp-duotone-000000-ffffff-2' => [
* 'slug' => 'wp-duotone-000000-ffffff-2',
* 'slug' => '000000-ffffff-2',
* 'colors' => [ '#000000', '#ffffff' ],
* ],
* ]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changed from being keyed by slug to being keyed by id so that the id didn't have to get recomputed again later. The key in the second part of the example (wp-duotone-000000-ffffff-2) was wrong before, but it's right now which is why it didn't change.

@ajlende ajlende merged commit eb71952 into trunk Apr 18, 2023
@ajlende ajlende deleted the fix/refactor-group-duotone-output branch April 18, 2023 19:45
@github-actions github-actions bot added this to the Gutenberg 15.7 milestone Apr 18, 2023
@ramonjd ramonjd added the Needs PHP backport Needs PHP backport to Core label Jun 5, 2023
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants