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

Port colord to PHP #49700

Merged
merged 4 commits into from
Apr 18, 2023
Merged

Port colord to PHP #49700

merged 4 commits into from
Apr 18, 2023

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Apr 10, 2023

What?

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

This part deprecates tinycolor functions and uses a private port of colord for duotone instead to match the JS.

Why?

How?

  • Switched over to a port of colord instead of the port of tinycolor.
  • Deprecated all of the PHP Gutenberg tinycolor functions. It was a mistake to have them public in the first place.

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 10, 2023
@ajlende ajlende self-assigned this Apr 10, 2023
@ajlende ajlende requested a review from spacedmonkey as a code owner April 10, 2023 22:41
@github-actions
Copy link

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

* @return string The SVG for the filter definition.
*/
public static function get_filter_svg_from_preset( $preset ) {
// TODO: This function will be refactored out in a follow-up PR where it will be deprecated.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@scruffian scruffian merged commit bc39f39 into trunk Apr 18, 2023
@scruffian scruffian deleted the add/port-duotone-php-to-colord branch April 18, 2023 11:34
@scruffian
Copy link
Contributor

This would be a good candidate to backport to core now.

@github-actions github-actions bot added this to the Gutenberg 15.7 milestone Apr 18, 2023
@Mamaduka
Copy link
Member

This would be a good candidate to backport to core now.

That's a good idea! The new code will also need the PHPUnit tests.

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.

4 participants