WordPress.org

Make WordPress Core

#22667 closed defect (bug) (fixed)

3.5 Media: Compat Meta can float wrap around to the right

Reported by: Viper007Bond Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description (last modified by Viper007Bond)

The compat-meta div which adds extra stuff to the meta data in the new media UI is floated to the left. However it also needs a clear, otherwise it can end up looking weird depending on the image size (a narrow image seems to be required to set it off).

Screenshots attached of me adding an "Advanced Edit" link using the following:

add_filter( 'media_meta', function( $media_meta, $attachment ) {

	// Abort when actually editing this media item
	// We only want to modify this outside of the media edit screen
	if ( ! empty( $_GET['post'] ) && $attachment->ID == $_GET['post'] )
		return $media_meta;

	$media_meta .= '<a href="' . esc_url( get_edit_post_link( $attachment->ID ) ) . '" target="_blank">Advanced Edit</a>';

	return $media_meta;

}, 10, 2 );

Attachments (15)

22667-wide-working.png (39.8 KB) - added by Viper007Bond 17 months ago.
22667-narrowish-working.png (35.9 KB) - added by Viper007Bond 17 months ago.
22667-narrow-broken.png (27.5 KB) - added by Viper007Bond 17 months ago.
22667.png (7.0 KB) - added by SergeyBiryukov 17 months ago.
22667.patch (1.5 KB) - added by SergeyBiryukov 17 months ago.
22667.after.png (7.0 KB) - added by SergeyBiryukov 17 months ago.
22667.diff (2.0 KB) - added by cdog 17 months ago.
22667.jpg (106.4 KB) - added by cdog 17 months ago.
22667-issues.jpg (26.9 KB) - added by cdog 17 months ago.
22667.2.diff (1.4 KB) - added by cdog 17 months ago.
22667.3.diff (1.6 KB) - added by koopersmith 17 months ago.
22667.4.diff (1.5 KB) - added by cdog 17 months ago.
22667.3.jpg (41.8 KB) - added by cdog 17 months ago.
22667.delete-permanently.png (6.6 KB) - added by SergeyBiryukov 17 months ago.
22667.5.diff (379 bytes) - added by SergeyBiryukov 17 months ago.

Download all attachments as: .zip

Change History (31)

comment:1 Viper007Bond17 months ago

  • Description modified (diff)
  • Keywords needs-patch added; has-patch removed
  • Summary changed from 3.5 Meida: Compat Meta needs to clear: left; to 3.5 Meida: Compat Meta can float wrap around to the right

A clear: left; just always puts it below the image. Needs a better fix than that.

comment:2 nacin17 months ago

  • Keywords needs-body added
  • Milestone changed from Awaiting Review to 3.5
  • Summary changed from 3.5 Meida: Compat Meta can float wrap around to the right to 3.5 Media: Compat Meta can float wrap around to the right

Needs a patch!

SergeyBiryukov17 months ago

SergeyBiryukov17 months ago

SergeyBiryukov17 months ago

comment:3 SergeyBiryukov17 months ago

More obvious example: 22667.png.

Could we perhaps move .compat-meta inside .details? 22667.patch does that. Also adds nowrap for "Delete Permanently" link.

Screenshot: 22667.after.png.

cdog17 months ago

cdog17 months ago

comment:4 cdog17 months ago

  • Cc catalin.dogaru@… added

comment:5 cdog17 months ago

attachment:22667.diff​ fixes the bugs illustrated in: attachment:22667.jpg (the left column is before and the right column is after applying the patch).

I've added 10 more px to the media-sidebar to make space for longer strings (e.g. September 28, 2012). Lowering the image size should also save some space but it would affect the page consistency.

comment:6 koopersmith17 months ago

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

Let's go with SergeyBiryukov's attachment:22667.patch. Grouping the two works well. +1.


cdog, with the combination of translated strings and browser/OS settings, tuning widths is particularly difficult. For example, 'September 28, 2012' easily fits on the line (with a max-width image), with about 20-30px to spare. Also, we shouldn't fix the width on the details and compat containers, as their contents can easily be wider than the column itself. The UI should handle these situations gracefully.

cdog17 months ago

comment:7 follow-up: cdog17 months ago

I agree with you, grouping the two helps.

I've also made some tests with attachment:22667.patch and noticed some issues. A longer date or filename is pulling the details below the thumbnail as shown in attachment:22667-issues.jpg.

A possible solution is to set a fixed width for the details element (attachment-info.width - thumbnail.max-width should be fine). This should make the details to remain aligned with the thumbnail while the longer strings will be wrapped or clipped.

Edit: It's just a suggestion. If the details should always stay aligned with the thumbnail setting nowrap to delete-attachment is not sufficient. Else, SergeyBiryukov's patch does the trick.

Version 2, edited 17 months ago by cdog (previous) (next) (diff)

cdog17 months ago

comment:8 in reply to: ↑ 7 koopersmith17 months ago

Replying to cdog:

I've also made some tests with attachment:22667.patch and noticed some issues. A longer date or filename is pulling the details below the thumbnail as shown in attachment:22667-issues.jpg.

This is intentional — the intended behavior is that if the details do not fit within the space to the right of the thumbnail, they all are moved below it. Save the space if we can, but display things nicely if we can't.

koopersmith17 months ago

comment:9 follow-up: koopersmith17 months ago

attachment:22667.3.diff builds off of 22667.patch, allowing extra long filenames to break properly.

cdog17 months ago

cdog17 months ago

comment:10 in reply to: ↑ 9 cdog17 months ago

Replying to koopersmith:

attachment:22667.3.diff builds off of 22667.patch, allowing extra long filenames to break properly.

Just tested attachment:22667.3.diff. Good idea on using word-wrap but for narrow images and long strings I've got a lot of unused space and a long scrollbar as shown in attachment:22667.3.jpg (left).

Take a look at attachment:22667.4.diff. All the available space is filled and the scrollbar is shorter as shown in attachment:22667.3.jpg (right). And the code is smaller too.

Again, this is just a suggestion. Feel free to use the most suitable solution.

comment:11 nacin17 months ago

One issue would be that word-wrap could break something like "Delete Permanently" in an odd way. (And when strings are translated, they can additionally grow in length up to 2x.) It is better to let details decide to wherever it can fit, without shoving it into a too-narrow area.

comment:12 nacin17 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 22983:

Media: Prevent the compat view from wrapping around the attachment details. Allow the filename to break-word. props koopersmith, SergeyBiryukov. fixes #22667.

SergeyBiryukov17 months ago

comment:13 SergeyBiryukov17 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Was the omission of nowrap for "Delete Permanently" link in [22983] intentional?

It still wraps to a second line, which doesn't look nice on hover: 22667.delete-permanently.png.

comment:14 nacin17 months ago

I was worried about nowrap pushing a long translation off the screen. Cleared it with koop, but did not see the results in your screenshot. Happy to reconsider.

comment:15 nacin17 months ago

  • Keywords commit added; needs-testing removed

Per IRC.

comment:16 ryan17 months ago

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

In 22998:

No wrap for the attachment Delete Permanently link in the media modal.

Props SergeyBiryukov
fixes #22667

Note: See TracTickets for help on using tickets.