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.

Sign up for GitHub

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

Jump to bottom

duotone.php code cleanup #48607

Merged
merged 19 commits into from
Mar 1, 2023
Merged

duotone.php code cleanup #48607

merged 19 commits into from
Mar 1, 2023

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Mar 1, 2023 •
edited
Loading

What?

Just a bit of cleanup in duotone.php.

Why?

I hope the code is easier to understand.

How?

  • Use WP_Theme_JSON_Gutenberg::scope_selector to scope the selector.
  • Treat non-variable strings as plain CSS.
  • Update comments.
  • Update variable names.
  • Restructure duotone generation into custom preset, CSS, and custom code branches.

Testing Instructions

Duotone rendering should work as before except unset filters will use unset instead of none.

none is equivalent to unset for non-inherited CSS properties, like filter, with an initial value of none.

Testing Instructions for Keyboard

N/A

Screenshots or screencast

N/A

@ajlende ajlende added the [Type] Code Quality Issues or PRs that relate to code quality label Mar 1, 2023
@ajlende ajlende requested a review from getdave March 1, 2023 01:05
@ajlende ajlende self-assigned this Mar 1, 2023
@getdave getdave marked this pull request as ready for review March 1, 2023 09:15
@getdave getdave requested a review from spacedmonkey as a code owner March 1, 2023 09:15
scruffian
scruffian reviewed Mar 1, 2023
View reviewed changes
lib/block-supports/duotone.php

// Build the CSS selectors to which the filter will be applied.
// Todo - encapsulate this in a function.
Copy link
Contributor

Choose a reason for hiding this comment

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

Whooo!

scruffian
scruffian approved these changes Mar 1, 2023
View reviewed changes
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 enabled auto-merge (squash) March 1, 2023 23:01
@scruffian scruffian merged commit 9189c3a into trunk Mar 1, 2023
@scruffian scruffian deleted the update/duotone-cleanup branch March 1, 2023 23:23
@github-actions github-actions bot added this to the Gutenberg 15.4 milestone Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scruffian scruffian scruffian approved these changes

@getdave getdave Awaiting requested review from getdave

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey spacedmonkey is a code owner

Assignees

@ajlende ajlende

Labels
[Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Milestone
Gutenberg 15.4
Development

Successfully merging this pull request may close these issues.
2 participants
@ajlende @scruffian