Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#46529 closed defect (bug) (fixed)

CSS line-height values should be unitless - media-views.css

Reported by: ianbelanger's profile ianbelanger Owned by: ianbelanger's profile ianbelanger
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch good-first-bug commit
Focuses: ui, administration, coding-standards Cc:

Description (last modified by ianbelanger)

As outlined in #44643, CSS line-height values should be unitless unless necessary to be defined as a specific pixel value. It was suggested that we break up 44643 by stylesheet in order to better track them.

This ticket covers much of wp-includes/css/media-views.css, see patch notes below

Note: The patch was tested only on a Windows machine in Chrome, Firefox, IE11 and Edge. As per https://core.trac.wordpress.org/ticket/44643#comment:23 it should probably be tested on other OS's and supported browsers.

Patch Notes:

Unable to find selectors in the DOM. These may no longer exist in Core, but I am not 100% sure about that.

line 328 - .media-sidebar .sidebar-title
line 1139 - .attachments-browser .instructions

Attachments (4)

46529.diff (2.3 KB) - added by ianbelanger 6 years ago.
46529.1.diff (2.4 KB) - added by ianbelanger 6 years ago.
Updates patch with more precise line-height calculations
instructions.png (70.0 KB) - added by afercia 6 years ago.
46529.2.diff (433 bytes) - added by ianbelanger 6 years ago.
Update for last line-height change

Download all attachments as: .zip

Change History (16)

@ianbelanger
6 years ago

#1 @ianbelanger
6 years ago

  • Description modified (diff)

#2 @ianbelanger
6 years ago

  • Component changed from General to Media

#3 @joemcgill
6 years ago

  • Milestone changed from Awaiting Review to 5.3

Hi @ianbelanger. I'd be happy to see this change land. Thanks for the patch!

#4 @mukesh27
6 years ago

I tested attached patch successfully without any error in Ubuntu 18.04.2 LTS with Google Chrome Version 73.0.3683.75 and Firefox Version 65.0.1

@ianbelanger
6 years ago

Updates patch with more precise line-height calculations

#5 @SergeyBiryukov
6 years ago

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

In 45478:

CSS Coding Standards: Use unitless values for line-height in wp-includes/css/media-views.css.

Props ianbelanger, pbiron, afercia.
Fixes #46529. See #44643.

#6 @afercia
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

line 328 - .media-sidebar .sidebar-title
line 1139 - .attachments-browser .instructions

.attachments-browser .instructions is still used for the instructions that appear in the edit gallery view, edit playlist view, and for example in the Customizer when setting a header image. See attached screenshot.

Re: .media-sidebar .sidebar-title after some software archeology, I think it's a leftover, as well as sidebar-content and .media-sidebar .search and these rules can be safely removed.

They were introduced in [22321] for the first implementation of the media sidebar. Then removed right after in [22362]. Also, as far as I can tell there's no search in the sidebar.

@afercia
6 years ago

This ticket was mentioned in Slack in #core by antpb. View the logs.


6 years ago

@ianbelanger
6 years ago

Update for last line-height change

#8 follow-up: @ianbelanger
6 years ago

  • Keywords commit added

46529.2.diff addresses .attachments-browser .instructions, but I could not locate .media-sidebar .sidebar-title, sidebar-content or .media-sidebar .search in media-views.css

Thanks for pointing that out @afercia

#9 @ianbelanger
6 years ago

  • Keywords needs-testing removed

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


6 years ago

#11 in reply to: ↑ 8 @SergeyBiryukov
6 years ago

Replying to ianbelanger:

46529.2.diff addresses .attachments-browser .instructions, but I could not locate .media-sidebar .sidebar-title, sidebar-content or .media-sidebar .search in media-views.css

Right, they were removed in [45499].

#12 @SergeyBiryukov
6 years ago

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

In 46431:

CSS Coding Standards: Use unitless values for line-height in wp-includes/css/media-views.css.

Follow-up to [45478].

Props ianbelanger, afercia.
Fixes #46529. See #44643.

Note: See TracTickets for help on using tickets.