[wp-trac] [WordPress Trac] #30062: Add Hooks to Image Editor UI

[WordPress Trac]

#30062: Add Hooks to Image Editor UI
---------------------------------+---------------------------------
 Reporter:  ericmann             |       Owner:
     Type:  enhancement          |      Status:  new
 Priority:  normal               |   Milestone:  Awaiting Review
Component:  Media                |     Version:  4.0
 Severity:  normal               |  Resolution:
 Keywords:  has-patch has-tests  |     Focuses:  ui, administration
---------------------------------+---------------------------------

Comment (by ericmann):

 Replying to [comment:6 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).

 > 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.)

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