Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#37806 closed defect (bug) (fixed)

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

Reported by: eliorivero's profile eliorivero Owned by: joemcgill's profile 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 years ago.
Removes uneven height applied to dimension fields in Scale Image area.
37806.1.patch (377 bytes) - added by eliorivero 8 years ago.
Even height in Edit Media screen and Edit Image view of Media dialog
37806.diff (403 bytes) - added by joemcgill 8 years ago.

Download all attachments as: .zip

Change History (13)

@eliorivero
8 years ago

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

#1 @joemcgill
8 years 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 years ago

  • Keywords reporter-feedback added

@eliorivero
8 years ago

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

#3 @eliorivero
8 years 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 years ago by eliorivero (previous) (diff)

@joemcgill
8 years ago

#4 @joemcgill
8 years 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 years ago

#6 @karmatosed
8 years 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 years ago by karmatosed (previous) (diff)

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


8 years ago

#8 @FolioVision
8 years ago

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

Nice work.

#9 @joemcgill
8 years 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
8 years 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.