WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#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 2 years ago.
22667-narrowish-working.png (35.9 KB) - added by Viper007Bond 2 years ago.
22667-narrow-broken.png (27.5 KB) - added by Viper007Bond 2 years ago.
22667.png (7.0 KB) - added by SergeyBiryukov 2 years ago.
22667.patch (1.5 KB) - added by SergeyBiryukov 2 years ago.
22667.after.png (7.0 KB) - added by SergeyBiryukov 2 years ago.
22667.diff (2.0 KB) - added by cdog 2 years ago.
22667.jpg (106.4 KB) - added by cdog 2 years ago.
22667-issues.jpg (26.9 KB) - added by cdog 2 years ago.
22667.2.diff (1.4 KB) - added by cdog 2 years ago.
22667.3.diff (1.6 KB) - added by koopersmith 2 years ago.
22667.4.diff (1.5 KB) - added by cdog 2 years ago.
22667.3.jpg (41.8 KB) - added by cdog 2 years ago.
22667.delete-permanently.png (6.6 KB) - added by SergeyBiryukov 2 years ago.
22667.5.diff (379 bytes) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 @Viper007Bond2 years 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 @nacin2 years 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!

@SergeyBiryukov2 years ago

@SergeyBiryukov2 years ago

@SergeyBiryukov2 years ago

comment:3 @SergeyBiryukov2 years 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.

@cdog2 years ago

@cdog2 years ago

comment:4 @cdog2 years ago

  • Cc catalin.dogaru@… added

comment:5 @cdog2 years 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 @koopersmith2 years 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.

@cdog2 years ago

comment:7 follow-up: @cdog2 years 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 2 years ago by cdog (previous) (next) (diff)

@cdog2 years ago

comment:8 in reply to: ↑ 7 @koopersmith2 years 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.

@koopersmith2 years ago

comment:9 follow-up: @koopersmith2 years ago

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

@cdog2 years ago

@cdog2 years ago

comment:10 in reply to: ↑ 9 @cdog2 years 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 @nacin2 years 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 @nacin2 years 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.

@SergeyBiryukov2 years ago

comment:13 @SergeyBiryukov2 years 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 @nacin2 years 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 @nacin2 years ago

  • Keywords commit added; needs-testing removed

Per IRC.

comment:16 @ryan2 years 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.