WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 7 months ago

Last modified 7 months ago

#40578 closed defect (bug) (fixed)

Media Library: Clicking the edges of the media does not select, it only focuses the li.

Reported by: circlecube Owned by: adamsilverstein
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.8
Component: Media Keywords: good-first-bug has-patch needs-testing
Focuses: ui Cc:

Description

A user attempting to select the media element from the library can often not click the center of the thumbnail. There is an 8px margin on the containing LI element and if the thumbnail is clicked within that 8pm margin, the LI will receive focus (which is nice), but the thumbnail is not selected. This is also possible to do by clicking the white space between the thumbnails in the grid. The closer thumbnail will receive focus, but not be selected.

For a user, It's a confusing moment, because the user has clicked the element and see the thumbnail highlight since if receives focus (which I assume if for keyboard controls and accessibility), but the image is not selected so the button in the media library modal (usually says "insert into page") is still inactive. A user would either need to click the image again (and be sure to click closer to the center of the thumbnail) or press the spacebar to activate the checkbox within focus.

I suggest somehow removing the 8px margin so that any click on the element will also select the media. It will also still need to be remain as accessible as it currently is and allow keyboard controls to work.

This occurs in every current browser. Unless there is a reason for this behavior, I'd say it is a UX bug.

Attachments (6)

20170426-media-library-focus-select.gif (1.8 MB) - added by circlecube 8 months ago.
media library focus select issue
40578.diff (923 bytes) - added by psiico 8 months ago.
patch
40578-2.diff (134.7 KB) - added by circlecube 8 months ago.
May-01-2017 10-59-20-patch40578-2.gif (2.0 MB) - added by circlecube 8 months ago.
here's a screengif of the patch with the fix.
40578-3.diff (519 bytes) - added by circlecube 7 months ago.
cleaner patch with just the single line change
40578.2.diff (1.1 KB) - added by adamsilverstein 7 months ago.

Change History (20)

@circlecube
8 months ago

media library focus select issue

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


8 months ago

#2 follow-up: @adamsilverstein
8 months ago

  • Keywords needs-patch good-first-bug added

@circlecube Thanks for the bug report. This does seem like an issue and I agree it is confusing to have the images show highlighting when they aren't really selected. I checked and this issue exists back to 4.0 when the media grid was introduced.

Ideally, clicks in between the images wouldn't highlight anything.

Experimenting in my local environment, the issue seems to be the image padding. switching that to a margin fixed the issue. A CSS solution should be possible here. Marking this as a good first bug.

#3 in reply to: ↑ 2 @circlecube
8 months ago

Replying to adamsilverstein:

Experimenting in my local environment, the issue seems to be the image padding. switching that to a margin fixed the issue. A CSS solution should be possible here. Marking this as a good first bug.

Yes, it does seem contained to just a CSS fix. When I tried this fix, by simply switching the padding to margin, the focus "glow" on the media item is lost. I haven't had time to look deeper, but I assume there's a simple enough fix to get it all to work properly. I'll work on it in the next few days and try to get a patch together.

@psiico
8 months ago

patch

#4 @psiico
8 months ago

  • Keywords has-patch added; needs-patch removed

This patch fixes the same visual bug on the media library grid view. Left the shadow on focus property while in select mode since it may be useful for navigating and selecting several items (arrows & space) at once which is the case of this bug report (insert media option).

Last edited 8 months ago by psiico (previous) (diff)

#5 @adamsilverstein
8 months ago

  • Keywords needs-testing needs-screenshots added

@psiico thanks for the patch, I will give this a test.

#6 @adamsilverstein
8 months ago

@psiico I tried applying your patch and when I test I still see the clicking in the margin issue. Did you leave off possibly part of the fix in the patch?

I am testing in mac chrome dev and this is what I see with the patch applied.... clicks in the margin area still highlight the images without actually selecting them (no checkbox):

https://cl.ly/3C0a2i3J002J/Screen%20Recording%202017-04-28%20at%2009.13%20AM.gif

#7 @psiico
8 months ago

@adamsilverstein
My patch removes the shadows from Media -> Library as long as it isn't in bulk select mode.

I kept the focus shadow while on bulk select mode (bulk select mode = multiple file selection as seen in your GIF) in order to be able to select multiple files with the arrows and space key (in my opinion very useful when you have a large number of files). without this focus/visual guidance it's (almost?) impossible to select multiple files using the keyboard.

I've noticed just now that the same visual bug applies while choosing the featured image which only allows selecting one image, therefore, the focus shadow here should be removed too.

Or maybe I'm taking the wrong approach here. If so, I'm sorry.

PS: I'm also using mac chrome dev build.

Last edited 8 months ago by psiico (previous) (diff)

@circlecube
8 months ago

#8 @circlecube
8 months ago

After some digging I think the best way to resolve this is to update the js to match the css. As it's set now, clicking the li.attachment will give it focus, but only by clicking the div.attachment-preview (which is contained within the li) will select the media. So here's a patch that updates the javascript event attached to the media attachments, so clicking the li.attachment will select it.

From my testing the keyboard controls all still work, and not I'm not able to focus an element without also selecting it. Styling is all the same. See Attachment 40578-2.diff (it includes the js file as well as the min.js file)

@circlecube
8 months ago

here's a screengif of the patch with the fix.

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


8 months ago

#10 @afercia
8 months ago

  • Focuses accessibility removed

Seeing it's already in good hands, I'd remove the accessibility focus to clean-up our Trac report :)

@circlecube
7 months ago

cleaner patch with just the single line change

#11 @circlecube
7 months ago

  • Keywords needs-screenshots removed

@adamsilverstein, Here's (40578-3.diff) a cleaned up patch removing the stray console log and minified files.

#12 @adamsilverstein
7 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to adamsilverstein
  • Status changed from new to assigned

#13 @adamsilverstein
7 months ago

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

In 40874:

Media: Fix an issue selecting media when clicking item edges.

Adjust targeting of the click handler for media item selection so clicking edges of media items properly selects them.

Props circlecube, psiico.
Fixes #40578.

#14 @adamsilverstein
7 months ago

  • Milestone changed from Future Release to 4.9
Note: See TracTickets for help on using tickets.