WordPress.org
Get WordPress

Make WordPress Core

Opened 6 weeks ago

Last modified 9 days ago

#63549 assigned defect (bug)

Twenty Twenty-One: Line-height inconsistency in the editor with Lists and other blocks

Reported by: rishabhwp's profile rishabhwp Owned by:
Milestone: 6.9 Priority: normal
Severity: normal Version: 6.4
Component: Bundled Theme Keywords: has-test-info has-screenshots has-patch
Focuses: css Cc:

Description (last modified by sabernhardt)

This ticket is a follow-up to #60196, which fixed margin differences between the editor and frontend. However, there's still a noticeable spacing issue due to a mismatch in line-height.

As mentioned in comment:10:ticket:60196, the editor sets line-height on the <html> tag, while the iframe used in the editor applies line-height: normal on the <body>. This causes the text spacing in the editor to look slightly different from the frontend.

Backend Editor

https://i.postimg.cc/k5j8rX9J/backend.png

Frontend

https://i.postimg.cc/50TC076D/frontend.png

Attachments (2)

63549.patch​ (278 bytes) - added by dilip2615 4 weeks ago.
This is solution for this spacing issue.
63549.2.patch​ (58.6 KB) - added by rishabhwp 9 days ago.

Download all attachments as: .zip

Change History (21)

This ticket was mentioned in ​PR #8940 on ​WordPress/wordpress-develop by ​@rishabhwp.
6 weeks ago #1

  • Keywords has-patch added

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

This PR addresses a line-height inconsistency between the editor and the frontend. It follows up on [Trac ticket #60196](https://core.trac.wordpress.org/ticket/60196), which resolved margin issues but left a spacing difference due to how line-height is applied in the editor iframe.

#2 @sabernhardt
6 weeks ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 6.9
  • Summary changed from Twenty Twenty-One: Line-height inconsistency in List Item blocks between editor and frontend due to reset styles – follow-up to #60196 to Twenty Twenty-One: Line-height inconsistency in List Item blocks between editor and frontend
  • Version set to 6.4

Thanks for creating the ticket and a patch!

Version: 6.4 stopped converting html and body selectors to .editor-styles-wrapper in the printed style tag.

#3 @dilip2615
5 weeks ago

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

  • WordPress: 6.8.1
  • PHP: 8.2.27
  • Server: nginx/1.26.1
  • Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.2.27)
  • Browser: Edge 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-One 2.5
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Error condition occurs (reproduced).

Additional Notes

Reproduction Steps:
Installed Twenty Twenty-One
Add unorder list and check the editor screen
When I add the list according to screenshot I face the issue related to space.
Please see the screen shot.

Supplemental Artifacts

Screenshot: ​https://prnt.sc/m4OmNwFhO0FL

@dilip2615
4 weeks ago

This is solution for this spacing issue.

#4 @dilip2615
4 weeks ago

Attached a patch: ​https://prnt.sc/CDTiJUI2Vo8L

Last edited 4 weeks ago by dilip2615 (previous) (diff)

#5 @dilip2615
4 weeks ago

  • Keywords needs-testing added

Attached a patch: ​https://prnt.sc/CDTiJUI2Vo8L
Need to test.

#6 @SirLouen
4 weeks ago

  • Keywords changes-requested has-test-info added; needs-testing removed

Test Report

Description

❌ This report can't validate that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8940.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-One 2.5
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Test Instructions

Actual Results

  1. ❌ The patch is not solving the problem

Additional Notes

  • On the other hand, the patch by dilip does solve the problem, but is not adequately made (and obviously not applying). It should be targeting one of the SCSS files and built them afterwards, just like the PR 8940 is doing. @dilip2615 can you adapt it to this format?

#7 @rishabhwp
4 weeks ago

I believe there’s a bit of confusion. My PR on this issue is a follow-up to a previous one. The original issue was reported in ticket #60196 , for which I initially submitted a PR here: ​https://github.com/WordPress/wordpress-develop/pull/8895.

Following a comment on that ticket (https://core.trac.wordpress.org/ticket/60196#comment:10), I was advised to open a separate ticket to specifically address the line-height for both html and body elements.

This ticket and my PR targets that specific issue. For more context, please refer to the issue description.

#8 follow-up: @sabernhardt
4 weeks ago

  • Keywords changes-requested removed

If a ruleset like the margin revert in 63549.patch is appropriate for all themes, but at zero specificity, that would belong in the Gutenberg styles (not the TinyMCE stylesheet). See ​GB42526.

For this ticket, 8940.diff correctly applies the theme's line-height to List and Navigation blocks in the editor. If you want to see the List blocks match the front-end spacing within the editor, apply both 8940.diff and 8895.diff (or wait until one of them is committed).

If I had made a patch for this ticket, I would have grouped both the font-family and line-height in one ruleset for html and body, but I am not entirely convinced my way is better than PR 8940.

html,
body {
	font-family: var(--global--font-secondary);
	line-height: var(--global--line-height-body);
}

body {
	--wp--typography--line-height: var(--global--line-height-body);
	color: var(--global--color-primary);
	background-color: var(--global--color-background);
	font-size: var(--global--font-size-base);
	font-weight: normal;
	-moz-osx-font-smoothing: grayscale;
	-webkit-font-smoothing: antialiased;
}

#9 in reply to: ↑ 8 @SirLouen
4 weeks ago

Replying to sabernhardt:

If a ruleset like the margin revert in 63549.patch is appropriate for all themes, but at zero specificity, that would belong in the Gutenberg styles (not the TinyMCE stylesheet). See ​GB42526.

Hard to test this one isolated. Why not integrating this directly #60196

I'm seeing reports covering half revamp of WP with 7 features at the same time, and others with just 3 characters struggling because "they cover too much".

Not so bald nor with three toupees…

#10 @sabernhardt
3 weeks ago

  • Summary changed from Twenty Twenty-One: Line-height inconsistency in List Item blocks between editor and frontend to Twenty Twenty-One: Line-height inconsistency in the editor with Lists and other blocks

The margin issue (#60196) should only affect the List Item blocks.

The line-height issue in Twenty Twenty-One affects more blocks. The theme's editor styles give a line-height for paragraphs, headings, table cells and figcaption elements. However, reset.scss defines the line-height as normal for:

  • List blocks
  • Preformatted, Code and Verse blocks
  • Navigation blocks
  • the month (table caption) in Calendar blocks

As I said on #60196, I would not mind combining both pull requests into one testing process and/or one commit, but that would involve testing more blocks.

Last edited 3 weeks ago by sabernhardt (previous) (diff)

#11 @SirLouen
3 weeks ago

  • Keywords has-screenshots added

Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8940

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.29.0
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-One 2.5
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Testing Instructions

  1. Add some blocks: Navigation with some elements and Preformatted blocks
  2. Add some background to the blocks to clearly see the limits of the block
  3. 🐞 Check the difference between the block editor and the front end. Is little but there is some difference

Expected Results

  • Line Height should not be different between front and editor.

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • Now with additional info provided here, I can clearly see the problem that has been causing confusion in #60196 (because this also happens to affect Lists, but since Lists have an even bigger problem with data-blocks margins that is like an elephant in the room, this little visual glitch is difficult to distinguish in that block.

Supplemental Artifacts

Preformatted/Navigation pre patch (PR8940)

https://i.imgur.com/fybeLwf.png

Preformatted/Navigation with patch (PR8940)

https://i.imgur.com/GldUOpb.png

#12 @karmatosed
3 weeks ago

  • Keywords commit added

Assigning to myself so I can tackle both of these together.

#13 @karmatosed
3 weeks ago

  • Owner set to karmatosed
  • Status changed from new to assigned

#14 @karmatosed
3 weeks ago

  • Keywords commit removed

Putting hold on this as the other ticket has a fail right now in tests so need to see why that is happening and look at fixing before moving this to commit.

#15 follow-up: @karmatosed
3 weeks ago

I recommend merging this with the other issue so we can have one PR and then get that commited combined with #60196. This is an easier, calmer approach.

#16 in reply to: ↑ 15 @SirLouen
3 weeks ago

Replying to karmatosed:

I recommend merging this with the other issue so we can have one PR and then get that commited combined with #60196. This is an easier, calmer approach.

Technically, you should not encounter any troubles between them both and as @sabernhardt explained, it seems that these two problems are unrelated. I have commented in #60196 why that one, was not passing.

Although if you like, you can pick both patches, merge them both at the same time (fixing the EOF in #60196), close this as duplicate to the other and send the revision in #60196 with "Fixes #60196 #63549"

PS: A nice trick that @joemcgill told me in Slack some months ago: Create a PR with the patches you are going to upload to GH before adding them into your local repo for SVN co, this way you will get a nice run on GHA and see what's going on before committing. ​Here more info.

Last edited 3 weeks ago by SirLouen (previous) (diff)

#17 @karmatosed
3 weeks ago

  • Keywords needs-refresh added; has-patch removed

#18 @karmatosed
3 weeks ago

  • Owner karmatosed deleted

@rishabhwp
9 days ago

#19 @rishabhwp
9 days ago

  • Keywords has-patch added; needs-refresh removed
Note: See TracTickets for help on using tickets.