Edit Image in TinyMCE editor revamp

[eablokker]

First time poster here...

I propose a revamp of the "Edit Image" thickbox window in the TinyMCE editor. I believe #21390 may have intended to fix this, but didn't get around to it.

I believe the edit image dialog should be in the same UI style as the new Insert/Upload Media dialog. We should be able to re-select the image size (thumbnail, medium, large, full, and custom sizes), refresh the alt text from what's stored in the media attachment metadata, replace with a different image from the library, and be able to un-attach the media as an attachment to the post, and more.

There is a limitation that prevents this functionality from being possible: once the image is inserted into the post as an HTML <img> tag, the association of that media back to a media attachment in Wordpress is broken. There is no data linking the img tag back to the ID number of that image in the media library, except for the CSS class which has the ID number, such as: "wp-image-42". The CSS class is unreliable because users can inadvertently delete and change CSS class names through the UI.

I propose a solution: adding a data attribute to the IMG tag, such as data-wp-image="42". Javascript could use this data attribute to determine the ID number of the image, and be able to fetch the attachment's metadata from the database when it pulls up the edit image dialog, giving you access to choose other image sizes for that image, refresh the alt text, un-attach the media from the post, etc.

Those are my thoughts and I welcome other ideas and solutions to this issue.

[markoheijnen]

Yes, you are rights about image editing in the new upload dialog. I'm playing to get something done for that. I'm unsure about the TinyMCE thickbox. I think we should replace that one with our own one that looks like the new upload dialog.

Adding a data attribute is as unreliable as a class name since people can change that too.

[eablokker]

I agree, the TinyMCE thickbox should not be used for this.

Yes the data attribute could be unreliable, but it's slightly less unreliable than the CSS class, since the CSS class is editable from the Advanced tab, whereas the data attribute would not be editable from a UI dialog. Yes it could get mangled by someone editing in code view. However coders should know that CSS classes are there for style purposes, and are welcome to edit them, whereas data attributes are meant for something else.

What are some other solutions?

Insert entire image as a shortcode? Would this pose a problem for content portability? It occurs to me that captions are inserted as shortcodes, so why not images?

Use img src path to determine the media attachment? One potential problem is if the image file name points to a custom size like image640x480.jpg, and the custom image size is changed or deleted or doesn't exist on a different system, then the original image ID would not be found. There could be workarounds for this, but it seems this method, while being immune to user error, would be prone to system error.

Edit: I'm a noob and replied to myself instead of to markoheijnen

[SergeyBiryukov]

There is no data linking the img tag back to the ID number of that image in the media library, except for the CSS class which has the ID number, such as: "wp-image-42".

We have img_html_to_post_id() (introduced in [24240] for #24188). It tries to extract the ID from the CSS class first, and falls back to attachment_url_to_postid().

[eablokker]

Replying to SergeyBiryukov:

We have img_html_to_post_id() (introduced in [24240] for #24188). It tries to extract the ID from the CSS class first, and falls back to attachment_url_to_postid().

Wow, nice work! I can see this working 99.9% of the time except for that one edge case where you've deleted your CSS class, changed your thumbnail sizes, and used a plugin to regenerate your thumbnails. But I suppose future updates to this function could address that, and we can go ahead and use this function for now.

[gcorne]

So after some discussion on IRC, I spent some time putting together an initial patch for editing the various attributes of images embedded in posts via a media modal.

Some notes on the patch:

• To open the modal for now, double-click on an image in a post or select the image and then click again. The plan is to show buttons for edit and delete after selecting the image like there are in 3.8 but with a look-and-feel that better matches the 3.8 admin redesign.
• The “replace image” state is only partly functional, and the “replace image from url” state has not been added. We also should be able to incorporate an “edit image” state when the work needed to incorporate the edit functionality into the modal is completed.
• There probably will need to be some refactoring done to use less inheritance and more composition. All the inheritance can be hard to follow.
• The code for the old modal in the wpeditimage tinymce plugin will need to be removed.
• I didn’t put any energy into the design.

[nacin]

This looks awesome, gcorne!

One thing I noticed during code review is that the Underscore templates weren't reused. Could we use the existing templates? If we need to break them up further to have more granularity, that seems fine. It would be nice to share as much code as possible. (Or maybe I'm not noticing what's different.)

[gcorne]

Could we use the existing templates?

That is the plan. I decided to approach the problem by first getting everything working and then afterwords plan to go back and clean things up.

For anyone that is following along, I am using ​https://github.com/gcorne/develop.wordpress/tree/trac-24409 to keep track of my work and will be posting a patch with the "replace image" functionality implemented later today.

[gcorne]

Added attachment:24409-02.patch​ which includes the following improvements:

• The “Replace Image” is now working.
• Tweaked the design a bit so that it is less awkward.
• Fixed an issue with the linkTo functionality when dealing with external images.
• Fixed an issue where the linkTo url was being focused and selected causing the div to be scrolled down awkwardly when the viewport was not super tall.
• Disabled the built-in tinymce image plugin.
• Changed how the opening of the modal is triggered so that it works in IE

I have the following on my todo list:

• Add spinner and show if it takes longer than 2 seconds to fetch attachment if editing an attachment.
• Explore using a different technique than a reference to the promise when waiting for the attachment to be fetched.
• See how to best reuse templates and see what we can do about refactoring some of the other stuff.
• Consider doing some other refactoring to use composition and dependency injection to avoid the deep inheritance trees a bit.
• Consider disabling or changing the label of the toolbar button when the image being replaced is selected.
• Implement the edit and replace buttons in a similar fashion to 3.8, but with a new design (design is pending).
• Remove old modal code from the wpeditimage plugin.

[gcorne]

Discovered a small css issue with attachment:24409-02.patch​, so attachment:24409-03.patch​ is the actually the good one.

[gcorne]

In addition to the above todos, @helen reminded me that the replace image part should pull the default settings from the image being replaced instead of from the last insertion.

[azaozz]

In [27050]:

Introduce Edit Image (single mode) in the media modal and use it to edit images inserted in the editor. Adds new feature: replace an image in the editor. Props gcorne, see #24409.

[eablokker]

So exciting to see my ticket from last year getting some love! This is awesome.

Patch fails for me. I'm using the beta tester plugin and point release nightlies. Which version of WP am I supposed to apply the patch to?

I have an idea for the image resizing. The resizing handles in TinyMCE 4.0 are interesting. Ideally a just-in-time thumbnail crunch could be created for the exact size that you resize to. That could be tricky and maybe too much for this version. An alternative could be to automatically select the right thumbnail size for the dimensions you have resized to. For example, you insert a medium size image and then drag the handle to make it larger -- it automatically switches to the large thumbnail when the resized dimensions are larger than the "medium" thumbnail dimensions. Conversely, you drag to shrink the image and it automatically switches back to medium when its dimensions become equal to or smaller than the "medium" thumbnail dimensions. That way you are preventing pixelation as well as preventing wasted pixels, and making the UI more intuitive. Thoughts?

[bpetty]

Replying to eablokker:

Patch fails for me. I'm using the beta tester plugin and point release nightlies. Which version of WP am I supposed to apply the patch to?

For the nightlies, you'd need "bleeding edge", and even then, you won't get a version with this patch already applied until tomorrow, but since it has been committed, you would get it right now if you ​checked out WordPress from SVN trunk, which is also the appropriate place to apply the patch against anyway.

[wonderboymusic]

In 27051:

For starters, [27050] is rad. This just cleans up some extra new lines that were littered about, updates *some* of the inline docs (needs more), moves wp.media.controller.ImageDetails closer to its parent class, and de-dupes some code in media-template.php.

See #24409.

[eablokker]

Just got the update in my nightlies. This is awesome! Best thing to happen to the WordPress visual editor in years.

From what I can tell Image Details and Replace Image have identical functionality, or at least appear to, except with replace image you can choose a different image. If I just replace with the currently selected image and change some of the details in the right sidebar, doesn't that accomplish the same thing? If image details had more options, that would make more sense, like dimensions, css class, a dropdown of configurable predefined image classes, margin and border options, etc.

Also the cancel button seems to be in the wrong place. I think it would make more sense as a secondary button down next to the update/replace button.

Very nice work!

[helen]

Replying to eablokker:

From what I can tell Image Details and Replace Image have identical functionality, or at least appear to, except with replace image you can choose a different image. If I just replace with the currently selected image and change some of the details in the right sidebar, doesn't that accomplish the same thing? If image details had more options, that would make more sense, like dimensions, css class, a dropdown of configurable predefined image classes, margin and border options, etc.

So first, yes, I think the single mode will have some more things in it, or at least be extensible to be able to include more. That said, there is a fundamental difference here, and right now I think it's important that the two interfaces reflect that. With insert, you have returned to the Media Library. What you change in the sidebar changes that item's details in the database. In single edit mode, you are now editing the content you have inserted into your post - changes made there don't go back to the database. That is to say, it is not tied to the library in general anymore, you are just editing content.

This is alpha, though, so it's all subject to change. A huge part of the reason we like to commit things early rather than when they're "perfect" is (besides that perfect doesn't always exist) so that we can really use it and get a feel for it.

[gcorne]

In attachment:24409-04.patch​, I made the following adjustments to the wpeditimage tinymce plugin:

• added a toolbar with a dashicon for edit and remove to make the edit functionality a little more discoverable and to make it easier to remove images on a touch-based device. Not sure how I feel about the design.
• remove dead code used in the old 3.8 version of the modal.
• fixed an issue where the selection would be lost when canceling the modal.
• cleaned up the structure of the code a bit.

[azaozz]

In 24409.05.patch:

• Use a data attribute to "mark" the selected image. Keeping a reference to the node can crash in old IE.
• Add/remove the data attribute when adding/removing the image toolbar.
• Don't open the edit modal on second click on the image. Makes the "edit" button somewhat pointless and can sometimes trigger after resizing the image in IE.
• When the image has caption: attempt to prevent IE11 from making the caption element resizable and wrapping it in thick border.
• When the caret is inside a caption next to the image, pressing Enter will create a new <p> tag above the caption.
• Keep the parts that add support for captions to the default image modal as fallback in case the media modal is not used.

Also added a transparent overlay on the selected image, only in Firefox and WebKit (interferes with resizing in old IE).

TODO: UX review of the image toolbar and buttons, UI/styling review both inside the editor and the Edit Image modal.

[azaozz]

In 27143:

Remove a stray </div> from the Edit Image template, was breaking it in IE < 9. Props gcorne, see #24409

[gcorne]

@azaozz, thanks for the updates. I especially enjoyed the subtle design adjustments to the resize handles.

One drawback of using adding a div that sits above the entire selected image is that it inhibits the user from dragging the image to a new location, which is not disabled if the image doesn't have a caption. Something to keep in mind as we figure out the design/ux.

I also noticed that when using the resize handles to reduce the size of the image that often on mouseup the modal opens, which is not expected. I tried to disable opening the modal by taking advantage of the TinyMCE ObjectResizeStart event, but it doesn't seem to be firing when expected.

[gcorne]

I also noticed that when using the resize handles to reduce the size of the image that often on mouseup the modal opens, which is not expected. I tried to disable opening the modal by taking advantage of the TinyMCE ObjectResizeStart event, but it doesn't seem to be firing when expected.

This comment was actually invalid because 24409.05.patch​ actually doesn't actually open the modal when a clicking a second time on a selected image. Chalk it up to getting a little mixed up when merging my changes.

One drawback of using adding a div that sits above the entire selected image is that it inhibits the user from dragging the image to a new location, which is not disabled if the image doesn't have a caption. Something to keep in mind as we figure out the design/ux.

To address this for the time being, I removed the overlay in 24409-06.patch​. I improved the UX a little by removing the toolbar when starting to drag and when cutting or copying a selected image to avoid ending up with an orphaned toolbar when dragging an image to a new location.

[gcorne]

To address this for the time being, I removed the overlay in 24409-06.patch​. I improved the UX a little by removing the toolbar when starting to drag and when cutting or copying a selected image to avoid ending up with an orphaned toolbar when dragging an image to a new location.

On second thought, we only want to remove the toolbar on cut, which is adjusted in 24409-07.patch​.

[azaozz]

In 27159:

Edit image in TinyMCE:

• Add a "toolbar" at the top of the image with two buttons: Edit and Delete.
• Don't open the edit modal on second click on the image. Makes the "edit" button somewhat pointless and can sometimes trigger after resizing the image.
• When the image has caption: attempt to prevent IE11 from making the caption element resizable and wrapping it in thick border.
• When the caret is inside a caption next to the image, pressing Enter will create a new <p> tag above the caption.
• Hide the image toolbar on drag, cut, align.

Props gcorne, see #24409.

[azaozz]

In 27160:

Bump the TinyMCE version, see #24409.

[azaozz]

When clicking on an image in the top/right corner (where the "delete" button will appear), the toolbar is shown on mousedown and the button is activated/the image is deleted on the following mouseup. Patch coming up.

[azaozz]

In 27451:

TinyMCE editimage: show the toolbar on mouseup to avoid accidental clicks on the buttons, fixes #24409

[azaozz]

This is done. If there are bugs please open new tickets.

[azaozz]

In 27660:

TinyMCE:

• Fix PressThis styling when the popup is resized.
• Don't force loading of the media JS.
• Add the 'image' plugin as fallback when no media button(s).
• Add back the link icon tooltip/menu item string for translation.

See #24067, see #24409

[gcorne]

24409-08.patch​ fixes an issue where the modal would load blank if an invalid or no longer valid image size class was set on the image.

[nacin]

In 27687:

Media manager: Avoid a blank modal when an invalid image size class is set on the image.

props gcorne.
fixes #24409.

[nacin]

azaozz called this done a few weeks ago, so I've closed this out. Let's do new tickets for any remaining issues.

[azaozz]

In 27694:

TinyMCE: bring back removal of the size-* class when the user soft-resizes an image, see #24409

[azaozz]

In 27764:

Add the 'attachment_' prefix to caption IDs after editing, see #24409

[joedolson]

For accessibility issues, see #27642