[wp-trac] [WordPress Trac] #58555: Backport: Duotone changes, refactor, enhancements and fixes

[WordPress Trac]

#58555: Backport: Duotone changes, refactor, enhancements and fixes
-------------------------------------------------+-------------------------
 Reporter:  onemaggie                            |       Owner:
                                                 |  isabel_brison
     Type:  task (blessed)                       |      Status:  reopened
 Priority:  normal                               |   Milestone:  6.3
Component:  Editor                               |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-unit-tests gutenberg-merge       |     Focuses:
  needs-patch                                    |
-------------------------------------------------+-------------------------
Changes (by spacedmonkey):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 I have a number of questions issues with the code as it stands. I hope
 someone can answer.

 1. Why are their three deprecated methods in this new class?
 get_filter_svg_from_preset, get_filter_css_property_value_from_preset and
 get_filter_id_from_preset. It is very strange to me to introduce and then
 deprecated a method in the same release. This should be removed no?
 2. Why were functions like `wp_register_duotone_support` deprecated? As a
 developer API, it seems much cleaner to have a function.
 3. Why does the `WP_Duotone` use all static methods? I feel like having
 all static methods, basically makes it pointless of it being a class. It
 is just a namespaced function at that point. Either go back to functions
 and make this use methods. It should be simple enough to convert to none
 static with no downsides. Example

 {{{#!php

 $duotone = new WP_Duotone();
 // Add classnames to blocks using duotone support.
 add_filter( 'render_block', array( $duotone, 'render_duotone_support' ),
 10, 2 );

 // Enqueue styles.
 add_action( 'wp_enqueue_scripts', array( $duotone, 'output_block_styles'
 ), 9 );
 add_action( 'wp_enqueue_scripts', array( $duotone, 'output_global_styles'
 ), 11 );
 }}}

 Static properties and methods have a number of downsides.
 - There are harder to test, as they are stateful. In tests, simple create
 a new instance.
 - Different from the rest of the code base. I know there are other
 instances of static methods in core, but for the most part these were done
 for a reason. I don't understand the reasoning here.
 - Hard to follow and understand the code.

 I would love the feedback of the committers here. @isabel_brison
 @mikeschroder @peterwilsoncc

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58555#comment:28>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform