Make WordPress Core

Opened 2 years ago

Closed 19 months ago

Last modified 19 months ago

#53636 closed defect (bug) (fixed)

"Insert from URL": top of preview is cut off

Reported by: siliconforks's profile siliconforks Owned by: audrasjb's profile audrasjb
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-screenshots commit
Focuses: ui, css, administration Cc:

Description

In the "Add Media" dialog box, when using the "Insert from URL" function, the top of the preview is cut off.

Steps to reproduce:

  1. Install the "Classic Editor" plugin.
  2. Go to "Pages", then "Add New".
  3. Click the "Add Media" button.
  4. Click "Insert from URL".
  5. Enter the URL of an image. A preview of the image will be displayed below the URL field. Note that the top of the image is cut off.

This is especially noticeable for small images. For example, consider the following image:

https://s.w.org/style/trac/common/feed.png

Almost the entire image will be cut off.

Attachments (8)

insert-from-url.png (29.3 KB) - added by siliconforks 2 years ago.
This screenshot was taken with WordPress 5.8-RC2 (but the issue seems to be present in 5.7 too).
53636.diff (382 bytes) - added by sabernhardt 2 years ago.
change top to 82px
Before patch.png (48.0 KB) - added by mukesh27 2 years ago.
After patch.png (48.0 KB) - added by mukesh27 2 years ago.
53636.1.diff (570 bytes) - added by sabernhardt 23 months ago.
removing top margin from Link text field's container
insert-from-url-link-text-margin.2.png (26.6 KB) - added by sabernhardt 23 months ago.
link text field with 53636.1.diff
insert-url-image-widget-before-preview.png (19.2 KB) - added by sabernhardt 19 months ago.
image widget without a preview (with 53636.1.diff)
insert-url-image-widget-with-preview.png (21.5 KB) - added by sabernhardt 19 months ago.
image widget showing the preview (with 53636.1.diff)

Download all attachments as: .zip

Change History (24)

@siliconforks
2 years ago

This screenshot was taken with WordPress 5.8-RC2 (but the issue seems to be present in 5.7 too).

@sabernhardt
2 years ago

change top to 82px

#1 @sabernhardt
2 years ago

  • Component changed from General to Media
  • Focuses ui css administration added
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.9
  • Owner set to sabernhardt
  • Status changed from new to accepted

I have seen this before (ticket:48378#comment:6), but it was not fixed on that ticket.

It also happens when embedding videos with Insert from URL.

The patch increases the top spacing to 82 pixels to match the specified height of the input plus padding on the span.

To accommodate the possibility of users adjusting text size slightly larger than 18px, we could also reduce the bottom padding on the span (as low as 2px to leave room for the focus outline). The 82px top spacing would still give 16 pixels between the input border and the media details container at the expected font size.

@mukesh27
2 years ago

@mukesh27
2 years ago

#2 @mukesh27
2 years ago

The patch 53636.diff working fine once the image loaded from URL but it adds more space. Please check above attached screenshots.

@sabernhardt
23 months ago

removing top margin from Link text field's container

@sabernhardt
23 months ago

link text field with 53636.1.diff

#3 @sabernhardt
23 months ago

  • Keywords needs-testing added

53636.1.diff: With a top margin of zero on the Link text setting's container, its label and field are 2 pixels lower than they have been. However, that is much closer than 12 (in the previous patch).

Last edited 19 months ago by sabernhardt (previous) (diff)

@sabernhardt
19 months ago

image widget without a preview (with 53636.1.diff)

@sabernhardt
19 months ago

image widget showing the preview (with 53636.1.diff)

#4 @sabernhardt
19 months ago

The preview was also partially hidden when using the image widget, and the patch's top value adjustment pushes the preview container down (as when inserting into the classic post editor).

In the widget dialog, I think the 10-pixel margin above the Alternative Text label and input should stay. It's necessary when the preview shows.

#5 @sabernhardt
19 months ago

  • Milestone changed from 5.9 to 6.0

With only a few hours before Beta, let's revisit this later, for next release.

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


19 months ago

#7 @joedolson
19 months ago

  • Milestone changed from 6.0 to 5.9

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


19 months ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


19 months ago

This ticket was mentioned in Slack in #core-test by monikarao. View the logs.


19 months ago

This ticket was mentioned in Slack in #core-test by ankit-k-gupta. View the logs.


19 months ago

#12 @Ankit K Gupta
19 months ago

Test Report

Env

  • WordPress 5.9-alpha-51272-src
  • Chrome Version - 96.0.4664.55 (Official Build) (x86_64)
  • OS - macOS Monterey
  • Theme: Twenty Twenty One
  • Editor - Classic Editor
  • PHP - 8.0.0
  • Web Server - nginx
  • Database - MySQL 8.0.16
  • Used patch file - 53636.1.diff

Steps to test

  1. Install the "Classic Editor" plugin.
  2. Go to "Pages", then "Add New".
  3. Click the "Add Media" button.
  4. Click "Insert from URL".
  5. Enter the URL of an image. A preview of the image will be displayed below the URL field.

Test result

Working expected now. Image preview has proper margin from top and bottom. Tested with small as well as large size images.

Screencast example:

Before Fix (Large Image) -

https://i.imgur.com/lnD2zhK.jpg

After Fix -

https://i.imgur.com/2wiE3db.jpg

Before Fix (Small Image) -

https://i.imgur.com/2dhIqP5.jpg

After Fix -

https://i.imgur.com/zVoZFEY.jpg

Last edited 19 months ago by Ankit K Gupta (previous) (diff)

#13 @Ankit K Gupta
19 months ago

  • Keywords has-screenshots added; needs-testing removed

#14 @audrasjb
19 months ago

  • Keywords commit added
  • Owner changed from sabernhardt to audrasjb
  • Status changed from accepted to assigned

I tested the patch and it fixes the issue.
Self-assigning for commit.

#15 @audrasjb
19 months ago

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

In 52263:

Media: Ensure media preview is fully viewable in the "Add Media" modal.

This change ensures the media preview is not cut off in the "Add Media" modal box, when using the "Insert from URL" feature.

Props siliconforks, sabernhardt, mukesh27, ankit-k-gupta.
Fixes #53636.

#16 @joyously
19 months ago

This seems like a fragile fix. Why is that div styled as position:absolute anyway? Or why does that label have a white background (which is what covers the preview)?

Note: See TracTickets for help on using tickets.