WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 7 months ago

#37806 closed defect (bug) (fixed)

Attachment Details - Edit: uneven height between input field and "Scale" button

Reported by: eliorivero Owned by: joemcgill
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Media Keywords: has-patch commit
Focuses: ui Cc:

Description

Steps to reproduce:

  1. go to Media
  2. click an image to open its Attachment Details view
  3. click Edit Image button below the image

In the SCALE IMAGE section, the height of the dimension fields and the Scale button don't match.

https://cldup.com/2xTiWqA4aT.png

After applying the attached patch, they look even
https://cldup.com/kmGkRkYwQe.png

This change doesn't affect the height in small screen size:
https://cldup.com/7zMhgqx0yo.png

Attachments (3)

37806.patch (416 bytes) - added by eliorivero 8 months ago.
Removes uneven height applied to dimension fields in Scale Image area.
37806.1.patch (377 bytes) - added by eliorivero 8 months ago.
Even height in Edit Media screen and Edit Image view of Media dialog
37806.diff (403 bytes) - added by joemcgill 8 months ago.

Download all attachments as: .zip

Change History (13)

@eliorivero
8 months ago

Removes uneven height applied to dimension fields in Scale Image area.

#1 @joemcgill
8 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.7
  • Owner set to joemcgill
  • Status changed from new to accepted

Hi @eliorivero,

Good catch, and thanks for contributing. Your patch seems to work well for the image edit view inside the media modal, but misses the same issue on the /wp-admin/post.php?post={{post_id}}&action=edit screen. The easiest way to get to that screen is from the list view in the media library.

Looks like by removing the styles in your patch, the padding and line-height for these inputs fall back to the defaults set for any input inside a .edit-attachment-frame (see relevant lines of code), which explains why it's only being applied in the media modal.

Perhaps we should apply the same default padding and line-height being applied to .edit-attachment-frame input and .edit-attachment-frame textarea to .imgedit-settings input and .imgedit-settings textarea. What do you think?

#2 @jorbin
8 months ago

  • Keywords reporter-feedback added

@eliorivero
8 months ago

Even height in Edit Media screen and Edit Image view of Media dialog

#3 @eliorivero
8 months ago

Hi @joemcgill,

thanks for the review. There's a new patch attached that covers both screens. I didn't use the line-height because it made the field 1 px taller and needed to tweak the vertical padding to make it look right

https://cldup.com/GJVRzfKKQM-1200x1200.png

so I just added a tweaked padding plus restored the font size again because otherwise the font was smaller.

https://cldup.com/1ddKCEGQ4h-3000x3000.png

Cheers!

Last edited 8 months ago by eliorivero (previous) (diff)

@joemcgill
8 months ago

#4 @joemcgill
8 months ago

  • Keywords ui-feedback added; reporter-feedback removed

Thanks @eliorivero. 37806.1.patch looks better to me. Just for kicks, 37806.diff removes the padding and font-size rules and allows these to take on the default sizes that apply to all other text inputs in the media modal. However, if we're intentionally bumping up the font size on those particular inputs then your last patch is the best option.

Would be good to get some feedback from some our our resident designers here, so I'll reach out for a second opinion.

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


8 months ago

#6 @karmatosed
8 months ago

My concern here is there is a historical reason for the font-size rules. @hugobaeta do you know of any reason - adding you here as you may. If there is no reason and device testing is solid, I'm totally keen we get rid of extra things and go with the cleaner patch.

Visually I am also very keen this gets in, it's awesome tidying up - thanks @eliorivero and @joemcgill for this.

Last edited 8 months ago by karmatosed (previous) (diff)

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


8 months ago

#8 @FolioVision
8 months ago

This is a splendid improvement. Much tidier, Elio and Joe.

Nice work.

#9 @joemcgill
7 months ago

  • Keywords commit added; ui-feedback removed

Let's go ahead and go with 37806.1.patch and leave the font size in these inputs alone for now.

#10 @joemcgill
7 months ago

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

In 38626:

Media: Align input and button heights in attachment details.

This tweaks the padding of the text inputs in image settings boxes on
image edit screens to match the height of the scale button.

Props eliorivero.
Fixes #37806.

Note: See TracTickets for help on using tickets.