Make WordPress Core

Opened 10 months ago

Closed 7 months ago

Last modified 7 months ago

#58756 closed defect (bug) (fixed)

Media Library Imrpovements: UI, Non-Closing Options, and Button Select State Issues in Image Editing

Reported by: nithi22's profile nithi22 Owned by: joedolson's profile joedolson
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.3
Component: Media Keywords: has-patch has-testing-info commit
Focuses: ui, accessibility, administration Cc:

Description

When editing an image in the media library, multiple issues occur simultaneously. Firstly, the UI for the image rotation option is poorly designed, making it difficult to use. Secondly, the options do not close when clicking outside the designated area, leading to a cluttered interface. Lastly, the button select state does not work correctly when clicking outside,

Environment

  • WordPress: 6.3-beta3-56155
  • PHP: 8.0.8
  • Server: Apache/2.4.46 (Unix) OpenSSL/1.0.2u PHP/8.0.8 mod_wsgi/3.5 Python/2.7.18 mod_fastcgi/mod_fastcgi-SNAP-0910052141 mod_perl/2.0.11 Perl/v5.30.1
  • Database: mysqli (Server: 5.7.34 / Client: mysqlnd 8.0.8)
  • Browser: Firefox 114.0 (macOS)
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins:

Steps to Reproduce:

  1. Access the media library.
  2. Select an image for editing.
  3. Click on the rotation option.
  4. Click on the button to activate its selected state.

Expected Behavior:

  1. The rotation option should be displayed in a user-friendly and intuitive UI.
  2. The rotation options should be presented clearly and in an organized manner.
  3. The button should enter a selected state when clicked.
  4. The button should maintain its selected state even when clicking outside.

Actual Behavior:

  1. The UI for the rotation option is poorly designed.
  2. When clicking outside the options, they do not close, leading to a cluttered and confusing interface.
  3. Clicking outside the designated area unexpectedly affects the selected state of the button, causing it to change or revert to its default state.

Attachments (6)

Screenshot 2023-07-07 at 8.41.44 PM.png (1.3 MB) - added by nithi22 10 months ago.
reference.png (515.0 KB) - added by deepakvijayan 9 months ago.
Screenshot 2023-07-13 111342.jpg (32.0 KB) - added by joedolson 9 months ago.
Design update
58756-2.diff (1.4 KB) - added by antpb 9 months ago.
refreshed patch
58756.diff (1.0 KB) - added by joedolson 9 months ago.
Corrected file
58756.3.diff (5.2 KB) - added by joedolson 7 months ago.
Follow up to close menu when focus elsewhere

Download all attachments as: .zip

Change History (36)

#1 follow-up: @joedolson
9 months ago

  • Keywords needs-patch reporter-feedback added
  • Milestone changed from Awaiting Review to 6.3
  • Owner set to joedolson
  • Status changed from new to accepted
  • Type changed from enhancement to defect (bug)
  • Version set to trunk

Hi, @nithi22 - thanks for your commments!

If you can provide any suggested changes for the design, that would be appreciated. The description "is poorly designed" isn't really actionable. Providing some concrete ideas about what is making this a problem for you would be helpful.

I agree that the rotation menu items should close when clicking outside the menu; that seems very reasonable.

#2 in reply to: ↑ 1 @azaozz
9 months ago

Replying to joedolson:

The description "is poorly designed" isn't really actionable. Providing some concrete ideas about what is making this a problem for you would be helpful.

Yep, "poorly designed" may mean many different things to different people :)

Also thinking it may be a bit too late for any substantial changes to be made for 6.3. The RC is in few days. Perhaps this can be improved for 6.4?

#3 @deepakvijayan
9 months ago

@joedolson @azaozz

I understand what he is getting at.

For starters, say when using the 'Image Rotation' feature. It should enable an 'active' state for the 'Image Rotation' button as visual feedback for the user. Similar practices should be followed for other controls as well.

Also, the submenu/options for said 'Image Rotation' buttons could be styled better. Right now it feels something was broken somewhere.

I've attached an image to reference how it could be improved.

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


9 months ago

#5 @joedolson
9 months ago

  • Keywords has-patch needs-testing added; needs-patch reporter-feedback removed

This patch updates the design of the image rotation, but does not implement the close when clicking outside suggestion. I suggest committing this change and reopening to try and get that in later; the design changes are helpful, particularly the addition of affordance to improve the image rotation menu.

#6 @joedolson
9 months ago

  • Keywords needs-testing removed

@joedolson
9 months ago

Design update

#7 @joedolson
9 months ago

I chose not to use the primary button design for an active state, since it already has a strong alternate meaning in the WordPress ecosystem.

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


9 months ago

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


9 months ago

#10 @sumitsingh
9 months ago

  • Keywords needs-design-feedback added

@antpb
9 months ago

refreshed patch

#11 @antpb
9 months ago

@joedolson I'm not sure why the patch doesn't apply clean but I uploaded a new refreshed one that I've confirmed to apply. The patch looks and works good in my review. 👍

just make sure my new patch doesn't have anything I missed :)

#12 @joedolson
9 months ago

  • Keywords commit added; needs-design-feedback removed

I'm not sure why the patch didn't apply cleanly, but it includes some CSS that belongs to #58692. As a result, your review of this patch applies to both those issues, I guess. That's my mistake; the patch I uploaded doesn't match the patch I have locally, so I assume I revised the patch but uploaded an earlier version.

Uploading the correct version of the patch, but I don't think it makes any concrete difference to your review.

@joedolson
9 months ago

Corrected file

#13 @joedolson
9 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56239:

Media: Improve layout of image rotation options panel.

Add visual affordance to expanded/collapsed states and move panel under the control rather than placing it next to the toggle, to prevent overflow with the save menu.

Props nithi22, deepakvijayan, antpb, joedolson.
Fixes #58756.

#14 @joedolson
9 months ago

In 56240:

Revert package-lock.json changes committed by mistake in [56239]

Props joedolson.
See #58756.

#15 @joedolson
9 months ago

  • Milestone changed from 6.3 to 6.4
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening and moving to 6.4 to address closing menu when focusing elsewhere.

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


9 months ago

#17 @joedolson
9 months ago

  • Keywords needs-patch added; has-patch commit removed

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


7 months ago

#19 @marybaum
7 months ago

@joedolson I think this is in your wheelhouse, so we won't interfere from a random scrub. Look forward to seeing this land!

@joedolson
7 months ago

Follow up to close menu when focus elsewhere

#20 @joedolson
7 months ago

  • Keywords has-patch needs-testing has-testing-info added; needs-patch removed

#21 @joedolson
7 months ago

To test:

1) Go to edit an image.
2) Open the image rotation menu.
3) Either tab outside of the menu or click elsewhere on the page. Observe that the menu closes when focus leaves the menu.

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


7 months ago

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


7 months ago

#24 @Dharm1025
7 months ago

Thanks for the testing instructions @joedolson

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58756/58756.3.diff
(Note: I am getting errors in applying the patch, but since there aren't many changes, I applied it manually and tested it.)

Environment

  • OS: macOS Ventura 13.4.1 (c)
  • Web Server: nginx/1.25.0
  • PHP: 7.4.33
  • WordPress: 6.4-alpha-56267-src
  • Browser: Chrome Version 116.0.5845.187 (Official Build) (arm64)
  • Theme: Twenty Twenty-Three
  • Active Plugins: -

Test Results

✅ Works as expected with a patch.

I have tested the patch as per testing instructions and it works as expected.

https://i.imgur.com/HTyXdy8.gif

#25 @joedolson
7 months ago

  • Keywords commit added; needs-testing removed

#26 @joedolson
7 months ago

Thanks for testing, @Dharm1025!

#27 @faisal03
7 months ago

Test Report

Tested this in SAFARI and found the patch https://core.trac.wordpress.org/attachment/ticket/58756/58756.3.diff is working as expected.

Environment

  • OS: macOS 14.0 (23A339)
  • Web Server: nginx/1.25.0
  • PHP: 7.4
  • WordPress: 6.4-alpha-56267-src
  • Browser: Safari Version 17.0 (19616.1.27.111.16)
  • Theme: Twenty Twenty-Three
  • Active Plugins: -

Image

https://i.imgur.com/sHGoP3O.gif

#28 @joedolson
7 months ago

Thank you, @faisal03!

#29 @joedolson
7 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56652:

Media: Close image rotation menu when not focused.

Implement a focus monitor so that if user focus moves away from the image rotation menu, it closes and doesn't block the image editing canvas. Follow up to [56239], [55919].

Props nithi22, deepakvijayan, dharm1025, faisal03.
Fixes #58756.

#30 @joedolson
7 months ago

In 56653:

Coding Standards: Remove unused variable el in monitorPopup.

Remove an unused variable to resolves coding standards failure. Follow up to [56651].

Props joedolson.
See #58756.

Note: See TracTickets for help on using tickets.