[wp-trac] [WordPress Trac] #29211: Customizer: extract a cropped-image control from header images

Tue Jul 14 06:30:23 UTC 2015

#29211: Customizer: extract a cropped-image control from header images
------------------------------+-------------------------------
 Reporter:  celloexpressions  |       Owner:  celloexpressions
     Type:  task (blessed)    |      Status:  assigned
 Priority:  normal            |   Milestone:  4.3
Component:  Customize         |     Version:  3.9
 Severity:  normal            |  Resolution:
 Keywords:  has-patch         |     Focuses:  javascript
------------------------------+-------------------------------
Changes (by celloexpressions):

 * keywords:  needs-patch => has-patch

Comment:

 There are several major issues here still. [attachment:29211.5.diff]
 addresses all of them. This may need some tweaking in terms of the way the
 code is placed/structured, but we need to address all of the problems here
 in a fix before this ticket can be closed, and these fixes need to be in
 4.3.

 ----

 Adding a `WP_Customize_Cropped_Image_Control` currently causes the
 Customizer to throw a JS error, rendering the entire Customizer completely
 unusable. Fortunately this fix is simple: we need to ensure that the
 `customize-views` script is loaded after the media scripts. Fixed in
 [attachment:29211.5.diff].

 ----

 The hook introduced in [33154] in `wp_ajax_crop_image` is not usable
 because insufficient information is passed to the hook to do ''anything''.
 You get only the context (which is presumably known and something you'd
 test against to determine handling) and `0` currently.
 [attachment:29211.5.diff] passes the cropped image URL and the original
 attachment id to this filter, and adds clarification of exactly what the
 filter does. Specifically, you start with the original attachment id but
 need to return an attachment id for an attachment object that has the
 cropped image in it. That means either changing the original attachment or
 creating a new one. Alternatively, a callback on this filter could
 presumably bail out of the ajax call and simply return the cropped image
 URL to the JS client. This is based on the information that is needed for
 the core site icon implementation of attachment handling after the cropped
 image is created.

 But when cropping is done from a Customizer control, the ajax action needs
 to give developers a reasonable result by default regardless. Currently
 there is a vague error that shows up in the media modal after submitting
 the copping step that is hard to debug, requiring a deep traversal through
 the intricacies of the media JS and the Customizer's implementation of it,
 and severely diminishing the developer experience. They're forced to debug
 what is really a core bug. The Customizer does not require developers to
 hook into core ajax functions in order for a basic control to work. You
 add the control and an associated setting and it "just works". Therefore,
 this filter is only used when an image is being cropped in ajax by a non-
 core feature or from a Customizer control.

 ----

 Continuing on from the issue with the filter, we need to implement the
 attachment-handling part for `WP_Customize_Cropped_Image_Control`. One of
 the best features of the Customizer is its API. The developer experience
 with the Customizer is just as important if not more important than the
 user experience. As the API grows, we need to ensure that we stay
 consistent with its established patterns, maximize forwards and backwards
 compatibility, optimize both ease of implementation and extensibility, and
 regularly do QA/QC on our API just like we're trying to do with our UI and
 UX. With UI and UX, it's all about screenshots and user testing. With
 developer experience, it's all about code examples, test plugins, and
 developer testing. When we find issues, we need to fix them, especially if
 we're still in beta and can fix them. A good example of this is the
 decision in WordPress 4.0 to make Panels a distinct object instead of
 being a custom section type (implemented as a subclass), as they had
 initially been developed.

 To ensure that the cropped-image control works as intended, we initially
 included the site icon feature with it in core, as an extension on the
 basic functionality offered via the API. This allowed us to catch many of
 the issues in the initial implementation, but we missed some things. To
 further test our implementation of this new control, this addition to our
 API and marketable improvement to the developer experience -- the power of
 the media modal and a core-provided image cropper that allows images to be
 fine-tuned to countless uses in themes and plugins with minimal developer
 effort -- I implemented `WP_Customize_Cropped_Image_Control` in another
 place where it's been proposed to be used in core: background images.

 The reasons for first implementing background image cropping (#32403) in a
 plugin are explained in [https://wordpress.org/plugins/background-image-
 cropper/ the plugin] and on the
 [https://make.wordpress.org/core/2015/07/10/feature-plugin-chat-on-
 july-14/#comment-26314 plugin proposal], but I've attached the plugin to
 this ticket also so that it can be evaluated as an example and used
 directly for testing. Note that due to some of the oddities of the way
 this core feature is implemented, the previewing part won't work yet, but
 the focus here is on the control and the preview will work with other
 custom cropped-image controls. The code is remarkably minimal for
 implementing such a historically complex (see: header images) feature, but
 considering the Customizer, and the developer experience that it offers,
 it's a reasonable and expected implementation:
 {{{
 add_action( 'customize_register', 'background_image_cropper_register', 11
 ); // after core
 function background_image_cropper_register( $wp_customize ) {
         $wp_customize->remove_control( 'background_image' );
         $wp_customize->add_control( new
 WP_Customize_Cropped_Image_Control( $wp_customize, 'background_image',
 array(
                 'section'     => 'background_image',
                 'priority'    => 0,
                 'flex_width'  => true, // Allow custom aspect ratios and
 dimensions.
                 'flex_height' => true,
                 'width'       => 1920, // "Suggested" size.
                 'height'      => 1080,
         ) ) );
 }
 }}}

 After testing this plugin, the critical script dependency issue mentioned
 above was discovered. Fixing that seemed to allow this control to work
 exactly as expected - the currently selected image was displayed just like
 it is with a regular image control, the change button opened the media
 modal, and after selecting an image, the cropper came up in the modal just
 like it does with header images. I successfully adjusted the crop handles
 to select the part of the image that I wanted to use and hit the crop
 button, expecting the modal to seamlessly close and the cropped image to
 show up in the Customizer controls and the preview. But instead, after a
 brief pause, I got a vague error message that the image could not be
 cropped, leaving closing the media modal as my only option.

 This flow is a failure in both user experience and developer experience.
 For the user, everything almost worked, then hit a major, insurmountable
 roadblock. Similarly, for the developer, the Customizer API offered its
 usual power and flexibility... until it wasn't quite able to do what it
 usually does, crashing and burning in an error message that is difficult
 to parse and assign as the fault of core or the developer's simple
 implementation of the typically-easy Customizer API. Even if the developer
 were familiar with the basics of custom controls in the Customizer, the
 necessity of their understanding the media js would present a significant
 barrier to finding the cause of the error and fixing it.

 We can, and really must, do better. When you add an image control to the
 Customizer, users can access the media modal select an image, and see it
 previewed without any additional work on behalf of the developer. When you
 add a cropped-image control to the Customizer, it should work just the
 same. Sure there are a few more ''optional'' parameters on the control to
 deal with the specifics of the cropping to be done, but these only offer
 more flexibility within the core control before needed to build a custom
 child control. With [attachment:29211.5.diff], the highly backend process
 of cropping an image is coupled with the very necessary step of creating a
 new attachment to represent the cropped image and returning that object to
 the Customizer UI. The `switch` for context is replaced by direct checks
 for core features with custom handling, followed by the default handling
 in the Customizer (complete with filters for use in more customized
 implementations), and finally followed by the above-mentioned filter for
 custom handling when a plugin uses this function outside of the
 Customizer.

 This fix allows the API to remain optimized for simplicity and ease of
 use, while still allowing for more advanced solutions if needed.

 ----

 One final fix here is to make the cropped-image control extend
 `WP_Customize_Media_Control` directly in PHP like it does in JS. The only
 difference between the media control and the image control is that the
 media control saves the attachment id while the image control saves the
 url (and is restricted to images, which the media control does with a
 parameter). The cropped-image control saves the attachment id to the
 setting, so it should extend the media control, for clarity. The distinct
 image control likely wouldn't have existed had the Customizer been
 introduced after the media manager, and is largely technical debt at this
 point. Also fixed in [attachment:29211.5.diff].

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