Make WordPress Core

Opened 3 years ago

Last modified 41 hours ago

#50523 accepted enhancement

Redesign the admin Image Editor

Reported by: afercia's profile afercia Owned by: joedolson's profile joedolson
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Media Keywords: early needs-testing has-testing-info needs-design-feedback has-patch
Focuses: ui, accessibility Cc:

Description

Splitting this out from #47136.

In #47136 the visual order and DOM order of the elements in the admin Image Editor was fixed. The discussion on the ticket highlighted the need for a major redesign of the Image Editor, which is pretty old and has room for improvements.

Quoting from ticket:47136#comment:16

this indicates the need to consider a redesign from ground up of the media interface. I know many already feel this. If we can from the start consider a11y and the best possible interactions, then we aren't forcing something or hitting the limits of an existing design

Quoting from ticket:47136#comment:35

Some initial redesign ideas were explored by @nrqsnchz see starting from https://core.trac.wordpress.org/ticket/47116#comment:11

As a first step, all the explorations made on #47116 should be moved to this ticket, including screenshots :) New design from the Design team is very welcome.

Attachments (14)

moving_buttons_to_sidebar_3.diff (6.1 KB) - added by joedolson 20 months ago.
Design patch from @nrqsnchz migrated from #47116
50523.diff (4.9 KB) - added by joedolson 20 months ago.
Refresh patch from @nrqsnchz
50523.2.diff (36.0 KB) - added by joedolson 12 months ago.
Work in progress patch for discussion
50523.3.diff (39.4 KB) - added by joedolson 12 months ago.
Updated work in progress
50523.4.diff (39.9 KB) - added by joedolson 12 months ago.
Current state for discussion
50523.5.diff (45.6 KB) - added by joedolson 11 months ago.
Updated patch; continuing dev
50523.6.diff (44.4 KB) - added by joedolson 2 weeks ago.
Refreshed patch
50523.7.diff (89.2 KB) - added by joedolson 2 weeks ago.
Updates to patch - group controls in menu, layout changes
50523.8.diff (45.6 KB) - added by joedolson 2 weeks ago.
Moves notice position, changes numeric inputs to type=number, improves scale warning
50523-modal-before.jpg (85.9 KB) - added by joedolson 9 days ago.
Modal view in 6.2
50523-modal-after.jpg (107.1 KB) - added by joedolson 9 days ago.
Modal view after patch
50523-attachment-before.jpg (77.4 KB) - added by joedolson 9 days ago.
Attachment view in 6.2
50523-attachment-after.jpg (92.9 KB) - added by joedolson 9 days ago.
Attachment view after patch
50523.9.diff (47.9 KB) - added by joedolson 8 days ago.
Refresh patch following r55859

Download all attachments as: .zip

Change History (76)

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


22 months ago

#2 @joedolson
22 months ago

  • Milestone changed from Awaiting Review to 5.9
  • Owner set to joedolson
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


20 months ago

#4 @joedolson
20 months ago

Migrating https://core.trac.wordpress.org/ticket/47116#comment:11 by @nrqsnchz


The more I think about it, the more I don't think that having those options in the place where they currently are is ideal (if we want to also have labels).

It takes way too much space on small viewports and when the user zooms their browser. That last morkcup you shared @sabernhardt is a good example of what I mean.

I've been thinking about the possibility of having these buttons on the sidebar. We seem to have some related items in there already (Scale Image and Image Crop). So what if we grouped all of these together? (we can keep Undo and Redo in the same place)

https://cldup.com/EQOj47HGCC.png

The stuff that is currently in Scale Image and Image Crop can be revealed contextually. So, for example, if I click on Crop, the aspect ratio and selection settings can appear below it:

https://cldup.com/bSnb1H8L0D.png

#5 @joedolson
20 months ago

Migrating https://core.trac.wordpress.org/ticket/47116#comment:25 by @karmatosed

---

Wow, this ticket moved on a bit, thanks for all the work everyone. Whilst I am not going to counter the work done here, I would like to offer some design feedback. I want to do this to not block but have us consider in beta a few points.

First, the position in the sidebar I feel is problematic from a cognitive and action point of view. It's removing a direct interaction from where it happens. I understand why now because the buttons are just too vast, however just putting them in the sidebar I don't feel solves this as removes from where they made sense to be. The placing in the sidebar is what for me takes this from bug to enhancement, so I agree with @mikeschroder on that. I understand labels put it in a bug, but this ticket does both.

I have concerns over moving and what plugins hook into this. For example, if a plugin adds icons/buttons here do we end up having lots in the sidebar? That could be seriously problematic on smaller screen heights and in general cause a lot of problems.

For me, the moving to the sidebar is done without exploring where the best possible location should be. I would love to see exploration as to that. I wonder if they are even buttons anymore like this? Why can't we have buttons that have tooltips/explanations for example? What about exploring hiding interactions in a toolbar of sorts even a drop-down? If text is desirable, I am sure another option can be found.

My advice at this point would be to not move to the sidebar in this as that takes it to an enhancement. If a better option is found for placing during beta 1, we could evaluate then, but for now my feeling is the sidebar doesn't do that. I am not commenting on text being by icon, but placement.

I will note, we are all pushing against the limits of the current interface so I recognise wins are hard in this confined space. That said, I think we shouldn't move buttons and actions without considering their impact in such a fixed design. What for example in a small sidebar happens to that text in translations, with plugins hooking in? It's for these and interaction reasons I am recommending we don't move them this release.

@joedolson
20 months ago

Design patch from @nrqsnchz migrated from #47116

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


20 months ago

@joedolson
20 months ago

Refresh patch from @nrqsnchz

#7 @joedolson
20 months ago

On reviewing this proposal, I agree with @karmatosed that moving the controls to the sidebar creates problems. Having the buttons to control your editing actions at such a distance from the editing target is a problem. The cancel/save buttons would also need to move, for control proximity.

I do very much like the proposal that the scale & crop controls open their respective panels, however.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


20 months ago

#9 @sabernhardt
19 months ago

  • Milestone changed from 5.9 to Future Release
  • Type changed from defect (bug) to enhancement

This needs more than a week, and it's more of an enhancement anyway. Moving the ticket to future release.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


18 months ago

#11 @joedolson
17 months ago

  • Milestone changed from Future Release to 6.0

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


16 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


15 months ago

This ticket was mentioned in Slack in #design by chaion07. View the logs.


14 months ago

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


14 months ago

#16 @antpb
14 months ago

Just noting that this will likely not make it in 6.0 due to the amount of decisions still to be made in this effort. We're leaving in 6.0 until there is a 6.1 milestone to continue discussions on it through this release cycle.

This ticket was mentioned in Slack in #design by chaion07. View the logs.


14 months ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


14 months ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


14 months ago

#20 @chaion07
14 months ago

  • Milestone changed from 6.0 to Future Release

Thanks @afercia for reporting this. We reviewed this ticket during a recent core bug-scrub session. Considering Beta 01 coming to effect by the end of the day and the feedback received from the team we are updating the milestone to 6.1 to stay aligned with Anthony's most recent comment. It also helps everyone and the reports stay organized in such way.

Props to @costdev & @peterwilsoncc

Cheers!

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


12 months ago

@joedolson
12 months ago

Work in progress patch for discussion

@joedolson
12 months ago

Updated work in progress

#22 @joedolson
12 months ago

50523.3.diff continues the work in 50523.2. Additional changes include:

  • Larger crop corners
  • Slight additional increase in image preview size (640px)
  • Add 'Apply Crop' button with crop settings
  • Add keyboard shortcuts for undo/redo (Ctrl+Z, Ctrl+Y)
  • Add listeners to track crop coordinates to inputs
  • Show crop settings either when 'Crop' button clicked or if a crop is selected.
  • Limit width of imgedit settings panel to prevent reflow when help info is expanded.

Changes already in 50523.2 include adding settings for starting coordinates, moving undo/redo to be collected with cancel/save, moving Save button after the 'Thumbnail Settings' data, increasing the preview image size, and using flex to place controls next to the image preview instead of all the way to the side.

#23 @joedolson
12 months ago

  • Keywords early added
  • Milestone changed from Future Release to 6.1

@joedolson
12 months ago

Current state for discussion

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


12 months ago

@joedolson
11 months ago

Updated patch; continuing dev

#25 @joedolson
11 months ago

50523.5.diff adds a background grid for image reference, a 'clear crop' button, and continuing implementation improvements.

#26 @joedolson
11 months ago

Noting #44867 as a set of image editor changes that should be integrated as an enhancement after this is finished.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


11 months ago

This ticket was mentioned in Slack in #core-media by azaozz. View the logs.


11 months ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


11 months ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


11 months ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


11 months ago

This ticket was mentioned in Slack in #core by martin.krcho. View the logs.


11 months ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


10 months ago

#35 @chaion07
10 months ago

  • Keywords needs-testing added

Thanks @afercia for reporting this. We reviewed this during a recent bug-scrub session. Here's the summary of the feedback:

  1. Adding keyword 'needs-testing'
  2. The latest patch still applies cleanly against trunk
  3. We plan to raise this ticket in the design channel to get more attention from Design Contributors.

Thanks!

Props to @costdev

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


10 months ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


10 months ago

This ticket was mentioned in Slack in #design by chaion07. View the logs.


10 months ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


10 months ago

#40 @joedolson
10 months ago

  • Milestone changed from 6.1 to Future Release

Punting to future release; will try to get in early for 6.2.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


9 months ago

#43 @joedolson
7 months ago

  • Milestone changed from Future Release to 6.2

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


7 months ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


6 months ago

#46 @bgoewert
5 months ago

  • Keywords has-testing-info added

Test Report

Patch tested: 50523.5.diff

Steps to Test

  1. Upload image (random image)
  2. Select image
  3. Edit more details
  4. Edit image
  5. Crop
  6. Undo
  7. Scale
  8. Rotate
  9. Flip
  10. Save
  11. Restore

Expected Results

All image actions work

  • Crop
  • Undo
  • Scale
  • Rotate
  • Flip
  • Save
  • Restore

Environment

  • OS: Pop!_OS 22.04
  • Web Server: Docker Desktop
  • PHP: 7.4.33
  • WordPress: 6.2-alpha-54642-src
  • Browser: Firefox 108, Chrome 108
  • Theme: Twenty Twenty-Three
  • Active Plugins: None

Actual Results

  • ✅ Crop
  • ✅ Undo
  • ✅ Scale
  • ✅ Rotate
  • ✅ Flip
  • ✅ Save
  • ✅ Restore

Additional Notes

  • Are the crop/scale action settings supposed to be in a sidebar? Because they were not.
  • Noticed that scale saves automatically and there is no way to undo a scale unless you restore the original image.
  • Also noticed the scale ignores any additional unsaved changes and applies without those changes.
  • After applying a crop, the "Crop Image" settings do not get removed unless you click crop again.

Supplemental Artifacts

Didn't embed images so as to not create a long comment.

Before applying patch
https://i.imgur.com/7TR9LNS.png

After applying patch
https://i.imgur.com/pUZCf9H.jpg

Cropping
https://i.imgur.com/Wi7b7A5.png

After applying crop
https://i.imgur.com/1NEa0Jy.png

Scaling
https://i.imgur.com/6OoBdny.png

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


5 months ago

#48 @audrasjb
5 months ago

  • Milestone changed from 6.2 to Future Release

As per today's bug scrub, moving this ticket to Future Release to give it more time for testing and design feedback.

#49 @joedolson
4 months ago

  • Milestone changed from Future Release to 6.3

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 months ago

#51 @joedolson
4 weeks ago

One of the biggest recurring issues with the admin image editor is the clarity of the crop feature. See:

#52221, #50395, #48754, #17247

If the only thing this ticket accomplished was to clarify the use of the crop tool, I think that would be a valuable change. Let's try and finish this off for 6.3.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


3 weeks ago

@joedolson
2 weeks ago

Refreshed patch

@joedolson
2 weeks ago

Updates to patch - group controls in menu, layout changes

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 weeks ago

@joedolson
2 weeks ago

Moves notice position, changes numeric inputs to type=number, improves scale warning

This ticket was mentioned in Slack in #accessibility by joesimpsonjr. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


9 days ago

This ticket was mentioned in Slack in #design by joedolson. View the logs.


9 days ago

@joedolson
9 days ago

Modal view in 6.2

@joedolson
9 days ago

Modal view after patch

@joedolson
9 days ago

Attachment view in 6.2

@joedolson
9 days ago

Attachment view after patch

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 days ago

#58 @audrasjb
8 days ago

  • Keywords needs-design removed

As per today's bug scrub, let's remove the needs-design keyword since we already have a design for this change. Let's test the proposed patch to make sure it's ready to ship with other tickets marked early.

#59 @audrasjb
8 days ago

Note: let's wait for #26381 to be committed first.

#60 @audrasjb
8 days ago

  • Keywords needs-design-feedback added

@joedolson
8 days ago

Refresh patch following r55859

#61 @joedolson
8 days ago

List of changes:

1) Organization: editing buttons are grouped together in a menu above the editing canvas and settings. This helps improve issues with proximity of controls and findability, and keeps all controls in a consistent location across views.

2) Crop button is no longer multifunction. It only activates or deactivates the crop tools. This makes the use of the tool more predictable. On activation, the crop tool panel is made visible & an initial crop selection is made. If the selection is changed, that selection is retained if the user re-opens the panel. There is now an explicit set of Apply/Cancel buttons for cropping.

3) Scale button added to menu. Scale button toggles the scale tool settings.

4) Sidebar tools have been removed from the sidebar design and placed directly next/after to the editing canvas. This makes sidebar changes & warnings more obvious when visual focus is on the editing canvas, and makes the visual relationship better.

5) Crop settings now include X and Y values so that a crop placement can be fully configured without use of a mouse.

6) Editing canvas has been enlarged and a grid background added for a visual editing reference.

7) Crop handles have been enlarged for easier mouse selection. (From 5px to 10px)

8) Cancel button has improved text specifying that you are cancelling the entire editing session, rather than cancelling a specific action taken (e.g., cancelling crop selection.)

9) Crop & scale inputs changed to number inputs.

Patch is large, but most of that is because the order of controls has changed.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


42 hours ago

This ticket was mentioned in PR #4527 on WordPress/wordpress-develop by @joedolson.


41 hours ago
#63

  • Keywords has-patch added

Update image editor design in core.

Trac ticket: https://core.trac.wordpress.org/ticket/50523

Note: See TracTickets for help on using tickets.