Add Hooks to Image Editor UI

[ericmann]

While attempting to add new components to the media editor modal, our team discovered there are no hooks (actions or filters) in the wp_image_editor function of WordPress core.

We'd like to abstract the function from its current state to make things more extensible.

Concretely, we'd like to pull out various image edit groups into separate callback functions and add them to the UI similar to widgets or scripts (with a discrete registration mechanism). This presents developers with the ability to add new groups, manipulate the order of all groups, dynamically remove unused groups, or even register/avail unused groups for other developers.

Breaking the function out into discrete components also allows for better test coverage for this component of the application.

Ultimately, our goal was to add art direction control the media editor so site administrators have more control over responsive images. Unfortunately the lack of hooks prevents any direct manipulation of the media editor.

Patch forthcoming ...

[tomauger]

I submitted a ptach for this some time ago, that got lost in the shuffle. it does exactly what you're talking about: creates "image editor groups" that are modeled after meta boxes on the post editor.

NOTE: That patch was against 3.8 and will not apply to trunk. I'll submit a refresh momentarily.

https://core.trac.wordpress.org/attachment/ticket/21811/trac-21811-extensible-image-editor.patch

This plugin demonstrates how you would hook into these editor groups from your plugin to add a new editor group to the Image Editor.

https://core.trac.wordpress.org/attachment/ticket/21811/EnhancedImageEditorDemo_plugin.zip
https://core.trac.wordpress.org/attachment/ticket/21811/21811-enhanced-editor-demo.zip

In this case I believe it adds an editor group that allows the user to select which image_size he wants to crop for.

[tomauger]

Refactors wp-admin.includes/image-edit.php to move all Editor Groups into modular callbacks using add_image_edit_group()

[ericmann]

Original proposed patch to address the issues presented by the ticket. Props due to @bswatson @jonbellah @ednoles @kylereicks @sabreuse @ericmann

[ericmann]

We used this concept as a team hackathon - our original goal was to add <picture> style art direction support to the image editor, but the lack of hooks within the wp_image_editor() UI prevented any manipulation.

Instead, we focused on making the UI more modular. attachment:30062.diff​ introduces several new functions - we used the add/remove register/unregister paradigm frequently implemented within WordPress to manage a list of editor groups for within the UI. The wp_image_editor() function has also been refactored to use the same mechanism to register/load/print existing core sections.

We've added unit tests for the new functions, and also added escaping for variable output within existing code (i.e. moved the markup generation routines to discrete callbacks and ensured that passed args like $post_id are properly escaped).

The project in all was a collaborative effort by:
@bswatson
@ednoles
@jonbellah
@sabreuse
@kylereicks
@ericmann

[ericmann]

In our haste to push out code, we forgot to merge in a few unit tests. 30062.1 replaces 30062 as the canonical diff.

[johnbillion]

I've asked ericmann if he can provide an overview of what this patch does in terms of its API, so reviewers can get their head around it before looking at that big ol' diff file.

[ericmann]

Wanted to take a second to provide some overview for reviewers.

In short, every image edit group in the right rail of the image editor is now registered by way of a callback. These callbacks can be registered with a simple API:

• add_imgedit_group() - Registers and loads a specific callback
• register_imgedit_group() - Registers a callback
• remove_imgedit_group() - Unloads a specific callback but leaves it registered
• unregister_imgedit_group() - Unregisters a callback
• get_imgedit_groups() - Gets an array/list of all loaded image edit groups

Core will now register four default image groups:

1. Scale Image
2. Restore Image
3. Apply to
4. Crop Image

(These are the same groups registered pre-patch in the same order. Registering them as discrete elements here allows developers to filter and reorder/remove where necessary.)

In addition, several new hooks are defined so developers (themes/plugins) can hook in wherever needed. Whenever an image edit group is added/removed or registered/unregistered we trigger an action call passing the unique name of the edit group.

The get_imgedit_groups() function internally calls a filter, passing the array of all loaded image groups. This allows developers to dynamically filter which groups show up or don't show up based on whatever conditions they need.

Internally, the image edit groups are stored in a global array ($_imgedit_groups) in two buckets - > registered and loaded. Registered edit groups are available for use, but must be loaded (by calling add_imgedit_group()) in order to appear in the UI. This allows plugin editors to specify callbacks and register edit groups, but leave the decision to load them up to theme developers instead.

The wp_image_editor() function remains largely unchanged, but instead of building out its own markup for each existing image edit group (see the numbered list above) it merely calls a foreach across the array returned by get_imgedit_groups(). This means, by default, WordPress will still iterate over the four default groups - developers can, however, inject as many additional groups as needed into the markup by either adding/registering a new group through the API or filtering imgedit_groups to manually specify the groups they want printed and their order.

[tomauger]

@ericmann I'm surprised you don't mention or acknowledge my patch which does exactly the same thing, with some notable differences which I think are worth exploring.

I think the loaded/registered idea is very interesting. It doesn't map 1:1 to the way we handle meta boxes, which may prompt some to consider it a bit confusing. But it's a cool approach.

I do like the add_default_imgedit_groups() acttion.

I do prefer my approach that makes the register/add_imgedit_group function a little more robust on the front-end rather than deferring everything to the back-end. See my version:

function add_image_edit_group( $id, $title, $callback, $tab = "default", $help_text = "", $class = "", $callback_args = null ){ //... }

I feel that this gives developers a little more flexibility, and in particular, allowing the $args through makes things a lot easier.

Furthermore, my do_imgedit_groups() function makes things a little more DRY and keeps the Image Editor Group boxes more consistent. In your approach, you're forcing the theme/plugin dev to grok the complete structure of the Image Editor box, including the show/hide help javascript, the correct classes, etc.

That stuff can (and likely will) change in core at some point, and we want ALL the boxes to look and behave as much the same as possible. So take that piece out of the developer's hands.

In your version I'm not seeing a hook, other than 'add_default_image_groups' where devs could officially hook their own Image Edit Groups onto, the way we have 'add_meta_boxes' for Meta Boxes. See my patch for a way you can do this, right before looping through the Editor Groups.

[ericmann]

Updated diff with erroneous reference to a global variable.

[ericmann]

Replying to tomauger:

I'm surprised you don't mention or acknowledge my patch which does exactly the same thing

There was never any offense intended. We were working on a team hackathon where we were exploring adding art direction options to the media editor. While exploring wireframes and specing out the project, we realized it was a non-starter due to the lack of hooks in core on the edit screen.

We looked through Trac and saw no open tickets referencing or discussing the issue, so I opened this one to a) create a discussion and b) give us a ticket number we could reference in unit tests (you can run the tests for just our patch with a --group 30062 filter).

Your first reference to your patch pointed to an old (already "completed") ticket and also stated it was out of date. We had already started coding at that point, so we kept moving forward. I also haven't had time to review your patch until today in the first place - it's a great effort and, while different than our approach, definitely makes good headway on the same issue.

I think the loaded/registered idea is very interesting. It doesn't map 1:1 to the way we handle meta boxes, which may prompt some to consider it a bit confusing. But it's a cool approach.

We weren't trying to map 1:1 to meta boxes. There are several registration/loaded paradigms within WordPress already: meta boxes, widgets, scripts, hooks. We were looking at all of them and trying to find the lowest common denominator (a global list of registered/loaded elements) and keep things as flexible as possible. This is why our registration functions use simple callbacks for firing the render methods.

I do prefer my approach that makes the register/add_imgedit_group function a little more robust on the front-end rather than deferring everything to the back-end. See my version:

function add_image_edit_group( $id, $title, $callback, $tab = "default", $help_text = "", $class = "", $callback_args = null ){ //... }

I feel that this gives developers a little more flexibility, and in particular, allowing the $args through makes things a lot easier.

No. This doesn't make things easier, it introduces a very rigid and inflexible API against which each existing edit group must be rewritten. It also means we're keeping our code strictly in the backend and moving away from the ability to do more with a front-end (i.e. Backbone) view.

Our initial approach was to introduce various views and models to keep everything as streamlined and future-proof as possible. However, the original core code used direct references to post IDs and other variables that would be very inflexible in a reusable JS template world. Rather than rewriting everything, we sought to adapt the existing code in as flexible a manner as possible.

This means we keep most of the existing code (which is less of a shift for reviewers/maintainers, etc) while still moving to a far more flexible editor interface that allows developers to add in new sections where necessary.

Using callbacks rather than multiple, hard-coded variables, also paves the way for introducing an OOP-based approach in the future. The callback could easily be the ->render() method on an object that subclasses a WP_ImgEdit_Group class. But that kind of work would be beyond the scope of this ticket.

In other words, we're trying to introduce the smallest possible change to an existing system that both allows us to hook in and accomplish the goals of developer extensibility while still keeping the system open to future changes.

Furthermore, my do_imgedit_groups() function makes things a little more DRY and keeps the Image Editor Group boxes more consistent. In your approach, you're forcing the theme/plugin dev to grok the complete structure of the Image Editor box, including the show/hide help javascript, the correct classes, etc.

Yes. Forcing a developer to grok the complete structure of a UI element they're adding to WordPress, including dynamic interactions and class names, is a good thing and was very intentional. This shouldn't be hidden from the developer as future changes in core will change the way things function at a core level.

That stuff can (and likely will) change in core at some point, and we want ALL the boxes to look and behave as much the same as possible. So take that piece out of the developer's hands.

If WordPress makes fundamental changes to the markup structure and JS interactivity of these edit boxes in the future, it will fundamentally change the way the edit screen works. Even with automated markup generation, there's no guarantee the post-change version of your edit box will in any way resemble your intended UI. Instead, we provide the callback to allow developers to place their own markup in the edit box, which they can then style and program interactivity for outside of the currently-supported core mechanisms.

In your version I'm not seeing a hook, other than 'add_default_image_groups' where devs could officially hook their own Image Edit Groups onto, the way we have 'add_meta_boxes' for Meta Boxes. See my patch for a way you can do this, right before looping through the Editor Groups.

Look a bit more closely. The add_default_imgedit_groups isn't a hook, it's a function hooked to init. Other developers can also register/load their custom groups on init as well. In addition, the invocation of get_imgedit_groups() internally calls a filter (imgedit_groups), which allows developers to modify the loading order of sections, add new ones, or remove them entirely.

(I've udpated the patch with attachment:30062.1.2.diff​ as some old code was pushed earlier that referenced the global variable directly rather than calling its wrapper function. That was a mistake on my part.)

[tomauger]

Sorry I have been absent from this ticket for longer than I had intended. Thanks Eric for the detailed and considered responses.

Replying to ericmann:

No. This doesn't make things easier, it introduces a very rigid and inflexible API against which each existing edit group must be rewritten. It also means we're keeping our code strictly in the backend and moving away from the ability to do more with a front-end (i.e. Backbone) view.

I see your point - your approach provides greater flexibility in the end.

This means we keep most of the existing code (which is less of a shift for reviewers/maintainers, etc) while still moving to a far more flexible editor interface that allows developers to add in new sections where necessary.

I'm completely on-board with this. I think you will find that I approached the problem with the same philosophy and that in both of our patches, the core code is very much the same.

Furthermore, my do_imgedit_groups() function makes things a little more DRY and keeps the Image Editor Group boxes more consistent. In your approach, you're forcing the theme/plugin dev to grok the complete structure of the Image Editor box, including the show/hide help javascript, the correct classes, etc.

Yes. Forcing a developer to grok the complete structure of a UI element they're adding to WordPress, including dynamic interactions and class names, is a good thing and was very intentional. This shouldn't be hidden from the developer as future changes in core will change the way things function at a core level.

I wonder whether the reason we disagree on this point is because we are considering different types of "developer"? is it possible that I'm envisioning the plugin/theme developer and you are considering the more hardened core contributor?

I always feel that helping less involved developers create compliant and well-integrated extensions to WordPress is of benefit to everyone.

Let me put it this way: consider writing the Codex entry for adding editor groups - with your approach, the documentation will need to go into great detail about the HTML structure the developer would need to create in order to match the existing UX; if we actually create the HTML structure (including the Help piece) and allow them to focus on the functionality of their editor group, I think this makes for a better developer experience and will probably lead to more consistent UIs for those that decide to extend the editor in this way.

If WordPress makes fundamental changes to the markup structure and JS interactivity of these edit boxes in the future, it will fundamentally change the way the edit screen works.

Not sure about this - for example, since 3.8 there has been a slight update to the HTML structure that changed the way the Help text was wrapped. Had the Editor Groups been in place at that time, and the structure left up to the developer, then that change would have broken the Help feature for any custom editor groups.

On the other hand, I can appreciate your desire to give the developer maximum flexibility - if they don't want their editor group to look like the rest of the UI, then they should be allowed to over-ride the default HTML structure.

I can envision a before_title and after_title type action that injects the standard WordPress markup & styles, but can be left out by a developer who wishes to over-ride the entire structure.

[helen]

I am wary of doing this at the moment given the Image Flow working group. We could find ourselves in a tough place trying to maintain this going forward when realistically we need to replace what we currently have.

[ericmann]

Replying to helen:

I am wary of doing this at the moment given the Image Flow working group. We could find ourselves in a tough place trying to maintain this going forward when realistically we need to replace what we currently have.

I agree. As I opened the original ticket, I'm comfortable closing it for now. We can revisit later if needed.