Make WordPress Core

Opened 12 months ago

Last modified 3 weeks ago

#59960 reopened defect (bug)

Replace em-based `line-height` with unitless values

Reported by: sabernhardt's profile sabernhardt Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Administration Keywords: good-first-bug has-patch needs-testing
Focuses: css, administration, coding-standards Cc:

Description (last modified by sabernhardt)

Follow-up to #44643

The default line-height of 1.4em can cause issues, at least when elements have a larger font size than their container. Other instances probably could be unitless too (while keeping their numeric value).

body {
	line-height: 1.4;
}

Change History (21)

#1 @sabernhardt
12 months ago

I found ten em-based line-height values in the CSS files within the wp-admin and wp-includes directories.

4 rulesets in wp-admin\css\common.css

/* Line 228 */
body {
	line-height: 1.4em;
}

/* Line 513 */
.widefat td,
.widefat td p,
.widefat td ol,
.widefat td ul {
	line-height: 1.5em;
}

/* Line 520 */
.widefat th,
.widefat thead td,
.widefat tfoot td {
	line-height: 1.3em;
}

/* Line 782 */
.widget .widget-top,
.postbox .hndle,
.stuffbox .hndle,
.control-section .accordion-section-title,
.sidebar-name,
#nav-menu-header,
#nav-menu-footer,
.menu-item-handle,
.checkbox,
.side-info,
#your-profile #rich_editing,
.widefat thead th,
.widefat thead td,
.widefat tfoot th,
.widefat tfoot td {
	line-height: 1.4em;
}

wp-admin\css\edit.css (line 1418)

p.popular-tags {
	line-height: 2em;
}

wp-admin\css\install.css (line 93)

#logo {
	line-height: 1.3em;
}

wp-admin\css\list-tables.css (line 2252)

@media screen and (max-width: 782px) {
	table.plugin-install td.column-name strong {
		line-height: 1.6em;
	}
}

wp-admin\css\media.css (line 203)

.media-item .progress {
	line-height: 2em;
}

wp-admin\css\nav-menus.css (line 181)

.bulk-actions input.menu-items-delete {
	line-height: 2.1em;
}

Thickbox styles (compiled from thickbox.css, line 83)

#TB_ajaxContent {
	line-height: 1.4em;
}

#2 @faisalmehmood786
12 months ago

Hi,
How can I reproduce this issue?

This ticket was mentioned in PR #5719 on WordPress/wordpress-develop by @andbalashov.


11 months ago
#3

  • Keywords has-patch added; needs-patch removed

This commit addresses ticket #59960, which identified issues with em-based line-height values in various CSS files within the WordPress Administration interface. Following the suggestion in the ticket, the em-based line-heights are replaced with unitless values while keeping their numeric value the same.

This change primarily affects the 'common.css' file but also includes updates in 'edit.css', 'install.css', 'list-tables.css', 'media.css', 'nav-menus.css', and the Thickbox styles. The update aims to enhance the consistency and scalability of the CSS and resolve issues particularly noted in iframe-based editors.

Referencing the follow-up to ticket #44643, this update aligns with the focus on coding standards and CSS improvements within the WordPress administration environment. By making line-heights unitless, we ensure better adaptability across different font sizes and display environments.

Files updated:

  • wp-admin/css/common.css
  • wp-admin/css/edit.css
  • wp-admin/css/install.css
  • wp-admin/css/list-tables.css
  • wp-admin/css/media.css
  • wp-admin/css/nav-menus.css
  • Compiled Thickbox styles (thickbox.css)

Trac ticket: https://core.trac.wordpress.org/ticket/59960

@mukesh27 commented on PR #5719:


11 months ago
#4

Thanks @andbalashov for the PR. The changes look good to me.

@sabernhardt could you please review it so we mark this ready for commit

#5 @SergeyBiryukov
7 months ago

  • Milestone changed from Awaiting Review to 6.6

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


6 months ago

This ticket was mentioned in PR #6537 on WordPress/wordpress-develop by @hmrisad.


6 months ago
#7

I found some em-based line-height values in the CSS files within the wp-admin directories.

Files updated:


common.css
install.css
edit.css
nav-menus.css
list-tables.css

@hmrisad commented on PR #6537:


6 months ago
#8

no I was replace line height value from em to unitless okay I will do it
again and will create pull request with before after changed

On Fri, May 17, 2024 at 10:55 AM Mukesh Panchal *@*.*>
wrote:

*@*. commented on this pull request.


In src/wp-admin/css/common.css
<https://github.com/WordPress/wordpress-develop/pull/6537#discussion_r1604349132>
:

@@ -508,14 +508,14 @@ code {

.widefat td ol,
.widefat td ul {

font-size: 13px;

  • line-height: 1.5em;

+ line-height: 1.5;

Does the line-height value will same? Could you share the screenshot
before after chances?


Reply to this email directly, view it on GitHub
<https://github.com/WordPress/wordpress-develop/pull/6537#pullrequestreview-2062337718>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AT5AZRFKWEL27RUYOG3WJMTZCWET7AVCNFSM6AAAAABHP7HO6OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANRSGMZTONZRHA>
.
You are receiving this because you modified the open/close state.Message
ID: *@*.*>

This ticket was mentioned in PR #6642 on WordPress/wordpress-develop by @hmrisad.


6 months ago
#9

no I was found some line height em values in
replace line height value from em to unitless

Files updated:

  • edit.css
  • nav-menus.css
  • list-tables.css

@hmrisad commented on PR #6642:


6 months ago
#10

Here are 3 screen short where I get this and now change it

https://github.com/WordPress/wordpress-develop/assets/83496132/705f9910-bd7a-4ad3-b044-27373bbdde17
https://github.com/WordPress/wordpress-develop/assets/83496132/b5cd79ba-9880-4c20-aa49-dbb8e235d827
https://github.com/WordPress/wordpress-develop/assets/83496132/86a477d3-aad2-469b-af07-2a25735f80a1

@mukesh27 commented on PR #6642:


6 months ago
#11

Closed in favour of #5719

#12 @oglekler
5 months ago

  • Keywords needs-testing added

PR 5719 needs to be tested.

#13 @sabernhardt
5 months ago

  • Description modified (diff)
  • Milestone changed from 6.6 to 6.7

I'm not comfortable with changing all of these during beta. I do not even know where to find some of these elements.

To test, compare each admin screen before and after applying the patch. First, go to your browser settings and set a minimum font size of 16 to 20 to discover how the page changes without the unit. Then test again without the minimum font size to make sure the difference with the patch is not too much (as it seemed for the popular-tags section).

I had mentioned the iframe-based editor earlier because that is how I first noticed the small line-height, but I do not recommend testing PR 5719 in it. The main issue there is that admin body styles should not apply at all in the iframe (GB57176).

#14 @sabernhardt
5 months ago

  • Description modified (diff)

#15 @mosne
5 months ago

I agree that popular-tags (In plugin install page) is the only case where the em is need. For the other is a quite safe change.

p.popular-tags {
	line-height: 2em;
}

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


5 months ago

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


3 months ago

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


2 months ago

#19 @navinrangar
2 months ago

Last edited 2 months ago by navinrangar (previous) (diff)

#20 @pkbhatt
2 months ago

  • Resolution set to worksforme
  • Status changed from new to closed

I checked this issue with my latest pull and it works for me.

#21 @sabernhardt
5 weeks ago

  • Milestone changed from 6.7 to Future Release
  • Resolution worksforme deleted
  • Status changed from closed to reopened
Note: See TracTickets for help on using tickets.