WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 35 hours ago

Last modified 34 hours ago

#53636 closed defect (bug) (fixed)

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

Reported by: siliconforks Owned by: 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 5 months 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 5 months ago.
change top to 82px
Before patch.png (48.0 KB) - added by mukesh27 5 months ago.
After patch.png (48.0 KB) - added by mukesh27 5 months ago.
53636.1.diff (570 bytes) - added by sabernhardt 4 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 4 months ago.
link text field with 53636.1.diff
insert-url-image-widget-before-preview.png (19.2 KB) - added by sabernhardt 3 weeks ago.
image widget without a preview (with 53636.1.diff)
insert-url-image-widget-with-preview.png (21.5 KB) - added by sabernhardt 3 weeks ago.
image widget showing the preview (with 53636.1.diff)

Download all attachments as: .zip

Change History (24)

@siliconforks
5 months ago

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

@sabernhardt
5 months ago

change top to 82px

#1 @sabernhardt
5 months 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
5 months ago

#2 @mukesh27
5 months ago

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

@sabernhardt
4 months ago

removing top margin from Link text field's container

@sabernhardt
4 months ago

link text field with 53636.1.diff

#3 @sabernhardt
4 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 3 weeks ago by sabernhardt (previous) (diff)

@sabernhardt
3 weeks ago

image widget without a preview (with 53636.1.diff)

@sabernhardt
3 weeks ago

image widget showing the preview (with 53636.1.diff)

#4 @sabernhardt
3 weeks 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
2 weeks 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.


12 days ago

#7 @joedolson
12 days ago

  • Milestone changed from 6.0 to 5.9

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


11 days ago

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


11 days ago

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


8 days ago

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


7 days ago

#12 @Ankit K Gupta
6 days 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 6 days ago by Ankit K Gupta (previous) (diff)

#13 @Ankit K Gupta
6 days ago

  • Keywords has-screenshots added; needs-testing removed

#14 @audrasjb
35 hours 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
35 hours 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
34 hours 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.