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

Backport defaultDuotone option #12

Conversation

ajlende
Copy link

@ajlende ajlende commented Feb 16, 2022

@Mamaduka I have the patch for things included in WordPress/gutenberg#38681 in the first commit (40418ea). And the second commit (f2f1a7d) is a suggestion for how to make sure the filters are rendering.

Alex Lende added 2 commits February 16, 2022 16:17
commit 9e87c24d3be87dffd7dd396e93b865c871f413c6
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 16 14:14:39 2022 -0600

    Add example for get_metadata_boolean

commit 0481c25c779551657fbb4d12d62ad82c8d73d66d
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 16 14:04:49 2022 -0600

    Fix conditional hook

commit 18b6f62213631e89662c2f057df0e0931ee2d836
Author: Alex Lende <alex@lende.xyz>
Date:   Thu Feb 10 12:58:44 2022 -0600

    Prevent generating other default presets

commit 7729c2a8f3b864296049dd76eb18b47bf294f3eb
Author: Alex Lende <alex@lende.xyz>
Date:   Thu Feb 10 12:28:41 2022 -0600

    Try refactor should_override_preset

commit 23c5ed34e833957b9d0e56bf82a614d8db9bcb88
Author: Alex Lende <alex@lende.xyz>
Date:   Thu Feb 10 11:24:17 2022 -0600

    Use multi-origin duotone presets

commit e90fe4c6cfd508eed67ba048592e6bf298d55a8a
Author: Alex Lende <alex@lende.xyz>
Date:   Thu Feb 10 10:54:49 2022 -0600

    Try refactoring override path and bool to not be inverse

commit b1b9fe39da4b0fc04026514fe79725c4c617823d
Author: Alex Lende <alex@lende.xyz>
Date:   Thu Feb 10 10:41:19 2022 -0600

    Add defaultDuotone to prevent override

commit 6713f8725320af0c074727326fe9b7aa219bb76d
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 9 15:55:18 2022 -0600

    Update docs

commit fb0d9a6975d316e79acd8cb198032c1739245dba
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 9 15:46:34 2022 -0600

    Fix phpcs

commit ab51b12808d728b08ee76923dbdf22b35f572308
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 9 15:34:06 2022 -0600

    Add defaultDuotone to JSON schema

commit 07076eb55a2e7f43c217755e6284214bbabe6254
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 9 14:54:04 2022 -0600

    Disable duotone css vars too

commit 50598cf7f96a8ab9c754c4ccfb65d11f78a0c3f7
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 9 13:44:03 2022 -0600

    Add defaultDuotone to disable default SVGs

commit 5a45d0d447a1ac45d5d472d854b528ac1c0509dc
Author: Alex Lende <alex@lende.xyz>
Date:   Wed Feb 9 13:43:24 2022 -0600

    Fix spacing
@@ -574,6 +574,10 @@
add_action( 'wp_enqueue_scripts', 'wp_enqueue_global_styles' );
add_action( 'wp_footer', 'wp_enqueue_global_styles', 1 );

// SVG filters like duotone have to be loaded at the beginning of the body in both admin and the front-end.
add_action( 'wp_body_open', 'wp_global_styles_render_svg_filters' );
add_action( 'in_admin_header', 'wp_global_styles_render_svg_filters' );
Copy link
Owner

Choose a reason for hiding this comment

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

@ajlende This will load SVG filters on every admin page. We only need them on editor pages.

Copy link
Author

Choose a reason for hiding this comment

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

Updated in 41080f4

Copy link
Author

@ajlende ajlende Feb 17, 2022

Choose a reason for hiding this comment

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

I couldn't find a better place for it than in_admin_header. The only other hooks called in the body before the editor are the *_admin_notices hooks which don't make sense.

At least with get_current_screen()->is_block_editor() it should be rendered everywhere the block editor is used by default. In cases like the site editor where the filters have to be loaded differently (see #12 (comment)) it can be overridden on a case-by-case basis.

private static function should_override_preset( $theme_json, $path, $override ) {
if ( is_bool( $override ) ) {
return $override;
private static function get_metadata_boolean( $data, $path, $default = false ) {
Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible to use old logic from should_override_preset instead of introducing a new method and removing the old one?

This method won't be private after WordPress#2322.

Copy link
Author

Choose a reason for hiding this comment

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

It is possible. I would strongly advise against using the old one.

The old method is confusing because it returns either the boolean or the inverse of the boolean when a path is passed. Making the return consistent—the boolean or the boolean at the end of a path—is much easier to understand. And it doesn't require checking if the path argument is an array at both the call site and the method.

global $pagenow;
if (
is_admin() &&
( 'post.php' !== $pagenow || 'edit' !== $_GET['action'] )
Copy link
Owner

Choose a reason for hiding this comment

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

We can also use is_admin() && get_current_screen()->is_block_editor() it should work for the site editor as well.

Copy link
Author

Choose a reason for hiding this comment

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

The site editor doesn't need the SVGs because it's rendering the editor in an iframe that doesn't have access to the filters rendered in the page.

WordPress/gutenberg#37727 is open to render the SVGs via JavaScript into that iframe.

Copy link
Author

@ajlende ajlende Feb 17, 2022

Choose a reason for hiding this comment

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

Even so, I think get_current_screen()->is_block_editor() is still a better way of checking, so I've updated it.

There are a bunch of other checks that are happening with that method, so it's probably safer to use.

@ajlende
Copy link
Author

ajlende commented Feb 17, 2022

The duotone rendering fix was fixed in WordPress#2331. I'll open up a new one to trunk for the rest now that the dependencies are merged for 5.9.1.

@ajlende ajlende closed this Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants