WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#29993 closed defect (bug) (fixed)

Media action links are cramped on small screens

Reported by: pento Owned by: bradt
Milestone: 4.3 Priority: high
Severity: minor Version:
Component: Media Keywords: make-flow has-patch good-first-bug
Focuses: ui, administration Cc:

Description

In the Media Library list view, the action links are quite cramped on small screens.

Attachments (10)

IMG_2906.png (99.7 KB) - added by pento 3 years ago.
29993.diff (2.6 KB) - added by bradt 3 years ago.
Screen Shot 2014-11-14 at 9.18.14 PM.png (153.4 KB) - added by bradt 3 years ago.
29993.1.diff (2.5 KB) - added by bradt 3 years ago.
Update patch, merged in trunk only, tested, all good.
IMG_0635.PNG (310.2 KB) - added by ryan 3 years ago.
29993.1.diff on an iPhone 6+
29993.2.diff (2.8 KB) - added by bradt 3 years ago.
Icon column removed, icon moved to title column.
Screen Shot 2015-03-20 at 3.50.17 PM.png (684.6 KB) - added by bradt 3 years ago.
Screen Shot 2015-03-20 at 3.50.52 PM.png (219.9 KB) - added by bradt 3 years ago.
29993.3.diff (2.7 KB) - added by bradt 2 years ago.
Refreshed patch.
29993.4.diff (2.6 KB) - added by helen 2 years ago.

Download all attachments as: .zip

Change History (38)

@pento
3 years ago

#1 @soficgr
3 years ago

  • Severity changed from normal to minor

#2 @bradt
3 years ago

Working on a patch to hide the comments and thumb columns and show the thumb in the file column.

Last edited 3 years ago by bradt (previous) (diff)

@bradt
3 years ago

#3 @bradt
3 years ago

  • Keywords has-patch needs-testing added

When the screen size is < 450px:

  1. Hide the thumb and comment columns.
  2. Show the hidden thumb in the File column.

#4 @DrewAPicture
3 years ago

  • Keywords good-first-bug added
  • Milestone changed from Awaiting Review to 4.2
  • Owner set to bradt
  • Priority changed from normal to high
  • Status changed from new to assigned

#5 @DrewAPicture
3 years ago

  • Priority changed from high to normal

@ryan: What's your take on 29993.diff? What's left here?

#6 @ryan
3 years ago

The patch no longer applies cleanly, but the screenshot looks good.

#7 @helen
3 years ago

Throwing this out there knowing that there could be side effects - what if the thumbnail were just in the title column all the time, narrow screen or not? We could probably catch if the column is removed and hide the thumbnail (it's not in screen options), not sure what other things people are doing with the media list table that would break/stop applying.

#8 @bradt
3 years ago

I'm certainly not an authority on the media list table, but I'd be surprised if removing the icon column didn't break some plugins/themes that assume it will be there.

Our company is doing a WP Contrib Day this Friday, so I can certainly update the patch then.

#9 @helen
3 years ago

I'm not too concerned if something is non-destructively broken (e.g. images start showing when somebody had previously removed the column), but I don't like breaking things without knowing what they are first :)

@bradt
3 years ago

Update patch, merged in trunk only, tested, all good.

@ryan
3 years ago

29993.1.diff on an iPhone 6+

#10 @ryan
3 years ago

Seeing double images on iPhone 6+ and 5 with 29993.1.diff. Looks fine on desktop with Macnchrome.

#11 @bradt
3 years ago

Just tested again on Safari/iPhone 5 and Chrome/Android and don't see double images. Cached stylesheet on iOS maybe?

#12 follow-up: @helen
3 years ago

I'd still like to know what exactly would or could break by removing that column. I can't see it being anything destructive. It's not sortable, it can't be toggled in screen options - I'd really rather just bring the thumbnail into the main title/actions column if we're going this route.

#13 in reply to: ↑ 12 @ryan
3 years ago

Replying to helen:

I'd still like to know what exactly would or could break by removing that column. I can't see it being anything destructive.

I can't see it being destructive either. Go for it and find out during beta?

This ticket was mentioned in Slack in #core-flow by boren. View the logs.


3 years ago

#15 @bradt
3 years ago

Planning to work on this next Friday if no one else gets to it by then.

@bradt
3 years ago

Icon column removed, icon moved to title column.

#16 @bradt
3 years ago

Tested in latest Chrome in OS X and Chrome on Android.

#17 @DrewAPicture
2 years ago

  • Priority changed from normal to high

#18 @DrewAPicture
2 years ago

  • Milestone changed from 4.2 to Future Release

At this point in the cycle, we're two days shy of beta 4 and the changes being discussed here are under-tested and lacking consensus. "Go for it and find out during beta" is a perfectly valid approach, however I think it's too late to do that in 4.2. Punting.

@bradt
2 years ago

Refreshed patch.

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


2 years ago

#20 @afercia
2 years ago

Related: #32254

This ticket was mentioned in Slack in #core-flow by helen. View the logs.


2 years ago

@helen
2 years ago

#22 @helen
2 years ago

  • Keywords needs-testing removed
  • Milestone changed from Future Release to 4.3

Lots of changes in list tables happening, so I refreshed/riffed on @bradt's patch to get us moving again. I didn't add a breakpoint in 29993.4.diff, which we can do. Before we go there though, here's where the patch puts us at 320px wide in English. Note that there are more changes needed for accessibility (see related tickets) and more will come for better responsive list tables, which conveniently this better enables.

http://s.hyhs.me/bXUz/image.png

#23 follow-up: @afercia
2 years ago

Tested a bit and noticed cropped images (created with the Image editor or setting them as Theme headers) can be displayed with a different thumbnail size. Looks like cropped images don't get the right image size and they are big, scaled images? For example, the background1.jpg in the screenshot below is 1024x643 and then scaled in-browser with CSS.

Anyway, if I'm not wrong, the max width set with CSS should be 80px + 2px border, see in common.css the rules with td.media-icon ... as selector that should be updated. Can provide a refreshed patch if needed.

https://cldup.com/7_h11MDIUH.png

#24 @helen
2 years ago

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

In 32687:

Media: move the thumbnail into the title column in list view.

This gives us more space for the title and row actions, and will make a responsive list table enhancement significantly easier.

props bradt for the initial patch.
fixes #29993 and #32255. see #32509 and #32395.

#25 in reply to: ↑ 23 @helen
2 years ago

Replying to afercia:

Ah, that's my fault, all of mine are cropped square so I didn't notice. I'm going to restrict it down to 60px wide instead of the old 80px (which, yes, will make some images smaller when not square thumbnails) and additionally apply some CSS to make sure nothing goes wrong with images of other sizes.

#26 @helen
2 years ago

In 32688:

Media list view: ensure thumbnails are appropriately sized and constrained.

see #29993.

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


2 years ago

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


2 years ago

Note: See TracTickets for help on using tickets.