Make WordPress Core

Opened 2 years ago

Closed 4 months ago

#60196 closed defect (bug) (fixed)

Twenty Twenty-One: List Item blocks have extra margin in editor

Reported by: nidhidhandhukiya's profile nidhidhandhukiya Owned by: joedolson's profile joedolson
Milestone: 6.9 Priority: normal
Severity: minor Version: 6.1
Component: Bundled Theme Keywords: has-test-info has-patch commit
Focuses: css Cc:

Description

Steps to reproduce the issue.

  1. Activate Twenty Twenty one theme.
  2. Choose List block.
  3. Add some data into list.
  4. Apply some background color.

Now check both the side you can able to see there is a difference in editor and front end in spacing.
Editor side there is more spacing compare to front side.

I have attached video for better understanding.
Video URL :- https://share.cleanshot.com/ZmNJlFXhGPj4bpJNNP4C

Attachments (5)

60196.patch (537 bytes) - added by nidhidhandhukiya 2 years ago.
60196.2.patch (58.3 KB) - added by rishabhwp 8 months ago.
Attaching a refreshed patch for #60196. The necessary changes were made to the .scss file, and the corresponding .css file was regenerated using the build process.
before-patch-backend.png (53.9 KB) - added by shailu25 5 months ago.
Before Patch Backend
after-patch-backend.png (116.7 KB) - added by shailu25 5 months ago.
After Patch Backend
after-patch-frontend.png (67.5 KB) - added by shailu25 5 months ago.
After Patch Frontend

Download all attachments as: .zip

Change History (39)

#1 @ugyensupport
2 years ago

Twenty Twenty One :- List block having issue with spacing in editor side.

Description

Editor side there is more spacing compare to front side.

Environment

  • WordPress: 6.4.2
  • PHP: 8.0.0
  • Server: Apache/2.4.10 (Debian)
  • Database: mysqli (Server: 5.5.59-MariaDB-1~wheezy / Client: 5.5.62)
  • Browser: Chrome 120.0.0.0 (macOS)
  • Theme: Twenty Twenty-One 2.0
  • MU-Plugins: None activated
  • Plugins:
    • Gutenberg 17.4.1
    • WordPress Beta Tester 3.5.5

Steps to Reproduce

  1. Activate Twenty Twenty-one theme.
  2. Choose List block.
  3. Add some data to list.
  4. Apply some background color.
  5. 🐞 Now check both side you can see there is a difference in the editor and front end in spacing. Editor side there is more spacing compared to the front side.

Expected Results

  1. ✅ Fix front-end listing

Actual Results

  1. [Before Patch]https://s3.filebin.net/filebin/ceb092d29d93c8c812645eed0d37946495501502b2c2b6f8dd6a82ade9d4bd9a/5e7f814cc890bcd7fae39a928c4c7fbf2b8b5424e0ec0991773d71b9e5e14964?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=7pMj6hGeoKewqmMQILjm%2F20240105%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240105T104214Z&X-Amz-Expires=30&X-Amz-SignedHeaders=host&response-cache-control=max-age%3D30&response-content-disposition=filename%3D%22Before_Front-end.png%22&response-content-type=image%2Fpng&X-Amz-Signature=59beedbb7afda2fd6ea5bd09602b89b2bdd6eeb5779fa9f871b9fda2ab85b99b

2[Patch]https://s3.filebin.net/filebin/ceb092d29d93c8c812645eed0d37946495501502b2c2b6f8dd6a82ade9d4bd9a/d91b381be63f6b6adf895010383090b724f05ce1eef03b333c2ebee27aa6c62c?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=7pMj6hGeoKewqmMQILjm%2F20240105%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240105T104450Z&X-Amz-Expires=30&X-Amz-SignedHeaders=host&response-cache-control=max-age%3D30&response-content-disposition=filename%3D%22Patch.png%22&response-content-type=image%2Fpng&X-Amz-Signature=5138b6bc7340417eca5ccae2d36643b1158d9137db6951ec3d14596c07bba14a

  1. [ After Patch]https://s3.filebin.net/filebin/ceb092d29d93c8c812645eed0d37946495501502b2c2b6f8dd6a82ade9d4bd9a/0d05dcacafb1e3f412ae18324211467ab769d4ebaaecaac4350de347bba4d29f?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=7pMj6hGeoKewqmMQILjm%2F20240105%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240105T104338Z&X-Amz-Expires=30&X-Amz-SignedHeaders=host&response-cache-control=max-age%3D30&response-content-disposition=filename%3D%22After.png%22&response-content-type=image%2Fpng&X-Amz-Signature=81e2fbeeea08b2f29b08b6230eac9686e213dd9d6f7bfde76cdb26779530fba0

Working smoothly and fixed successfully ✅

Last edited 2 years ago by ugyensupport (previous) (diff)

#2 @sabernhardt
2 years ago

  • Focuses css added
  • Keywords has-patch changes-requested added
  • Summary changed from Twenty Twenty One :- List block having issue with spacing in editor side. to Twenty Twenty-One: List Item blocks have extra margin in editor
  • Version changed from 6.4.2 to 6.1

The inner blocks (List Item) added in 6.1 get the default top and bottom margin.

Twenty Twenty-One uses SASS, so the edits belong in assets/sass/05-blocks/list/_editor.scss, which then would be compiled to the CSS files.

The Navigation styles use revert and Social Icons styles set only the top and bottom margins to zero in the editor.

#3 @karmatosed
19 months ago

@nidhidhandhukiya this could do with a refresh are you able to review that? @sabernhardt has left some awesome feedback it would be great to get into the patch.

#4 @sandeepdahiya
9 months ago

#63526 was marked as a duplicate.

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


9 months ago
#5

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

This PR addresses a visual inconsistency in the Twenty Twenty-One theme where list items in the block editor (List block) display additional vertical spacing not present on the frontend.

## Before

https://github.com/user-attachments/assets/a9f2e447-9e32-478d-9b2c-cc4941f33058

## After

https://github.com/user-attachments/assets/a5c17aa0-801e-414d-b379-5f881b1ce0c9

#6 follow-up: @sabernhardt
9 months ago

  • Milestone changed from Awaiting Review to 6.9

@rishabhwp The patch should include changes to the .scss files and then compile those to the CSS files.

If you use the revert method, as [55101] did for the Navigation block, the List block ruleset would belong in assets\sass\05-blocks\list\_editor.scss.

Another option is to edit the selector in assets\sass\05-blocks\utilities\_editor.scss from [data-block] to [data-block]:where(:not(.wp-block-list-item)). Then it would honor a custom style like li {margin-bottom: 0.25em;} in Additional CSS, and the specificity level would remain the same.

#7 in reply to: ↑ 6 @rishabhwp
9 months ago

Replying to sabernhardt:

@rishabhwp The patch should include changes to the .scss files and then compile those to the CSS files.

If you use the revert method, as [55101] did for the Navigation block, the List block ruleset would belong in assets\sass\05-blocks\list\_editor.scss.

Another option is to edit the selector in assets\sass\05-blocks\utilities\_editor.scss from [data-block] to [data-block]:where(:not(.wp-block-list-item)). Then it would honor a custom style like li {margin-bottom: 0.25em;} in Additional CSS, and the specificity level would remain the same.

Thanks for the guidance! I've updated my PR accordingly and committed the changes. Please let me know if anything else needs adjustment.

#8 follow-up: @mleraygp
9 months ago

From testing on Playground, it seems that the margins are still different. Now there is a bit more spacing in the frontend than in the editor.

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


9 months ago

#10 in reply to: ↑ 8 @sabernhardt
9 months ago

  • Keywords changes-requested removed

From testing on Playground, it seems that the margins are still different. Now there is a bit more spacing in the frontend than in the editor.

The spacing is still different because the editor styles sets the line-height on html and the reset styles assign a line-height of normal on the iframe <body>—or a container <div> in the widget editor—using html :where(.editor-styles-wrapper).

I think the theme could set the line-height on both html and body in its editor stylesheets, but that discussion probably belongs on a separate ticket.

@rishabhwp commented on PR #8895:


9 months ago
#11

Hey @sabernhardt,
I'm new to core contributions. I have implemented the changes you suggested on Trac. Could you please review the PR or tag someone who can take a look? Thanks a lot!

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


8 months ago
#12

Trac ticket:

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.

#13 @karmatosed
8 months ago

  • Keywords commit added
  • Milestone changed from 6.9 to 6.8.2
  • Owner set to karmatosed
  • Status changed from new to assigned

Looks like this now has approved changes so I am going to go ahead and see about progressing this to a commit and see what happens from there in testing. Thank you everyone.

#14 @karmatosed
8 months ago

  • Keywords needs-testing added; commit removed

I am still seeing line-height issues and I feel like we should get that fixed in this issue but happy to be wrong. I also am not seeing such a difference currently in testing without the PR. I would like further testing to confirm states outside of just me.

#15 @sabernhardt
8 months ago

I suggested addressing the line-height on a separate ticket, and #63549 has PR 8940 for that (it's also linked to this ticket).

The line-height issue technically is bigger because it affects more blocks, though I would not mind combining both pull requests into one testing process and/or one commit.

#16 @SirLouen
8 months ago

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

Test Report

Description

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

Patch tested: https://core.trac.wordpress.org/attachment/ticket/60196/60196.patch

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. Following exact instructions provided by OP
  2. 🐞 The line spacing is massive in the editor. But the fun thing is that it not line spacing between list elements but margins between data-blocks.

Expected Results

  • No margins between data-blocks

Actual Results

  1. ✅ Issue resolved with patch.

Additional Comments

  • I think there has been a little confusion here and I also had a hard time identifying the differences, but as @sabernhardt this is affecting many other blocks and you might be doing so screenshots and comparing side by side because differences are extremely subtle. Although they have decided to move into #63549 because technically this is another. I will be posting the corresponding test report there.
  • But focusing on this specific patch I'm testing here, judging by the screenshots I'm providing, I think the problem with lists is pretty clear, data-block top and bottom margins for each element of the list is very disastrous in the editor (while not affecting the front end)

Supplemental Artifacts

Lists without patch
https://i.imgur.com/VpTSSl3.png

Lists with patch
https://i.imgur.com/bJ7UvXq.png

#17 @karmatosed
8 months ago

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

In 60366:

Twenty Twenty-One: Fixes List Item Block Margin.

The margin was different depending on the front or back of the editor. This patch is a companion to the next commit incoming from issue 63549. That issue resolves the line-height inconsistency, and this one does not.

Props nidhidhandhukiya, ugyensupport, sandeepdahiya, rishabhwp, mleraygp, SirLouen, sabernhardt.
Fixes #60196.

#18 follow-up: @karmatosed
8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I had some issues with tests and this patch. The more I consider this the more I air on the fact I would prefer to have both issues merged together for at least ease. It's very complicated to commit as it closes out the other one mentioning which isn't optimal. If we can get that done I will happily merge the new commit.

Version 0, edited 8 months ago by karmatosed (next)

#19 in reply to: ↑ 18 @SirLouen
8 months ago

Replying to karmatosed:

Ultimately the tests were breaking for other reasons - maybe because it is a Monday. However, the complexity did not help in working things out. For that reason alone we should simplify.

The problem is that this CSS has been added to the end of the file. And not added a new line after that, hence its failing the test.

#20 @skithund
8 months ago

@karmatosed the change should go into SASS/.scss files too.

Tests are failing because CSS built from SASS differs from committed CSS change.

#21 @karmatosed
8 months ago

For now, I stand by the revert but thank you for feedback. I was being light about the reason as needed to also diagnose myself and just get a revert in meantime. It's going to be better to merge into one for ease as we are going across two tickets and it most likely will break as a result in doing that also. Let's get this merged then we can happily commit a version that fixes throughout.

#22 @karmatosed
8 months ago

  • Milestone changed from 6.8.2 to 6.9

#23 @karmatosed
8 months ago

  • Keywords needs-refresh added; has-patch removed
  • Owner karmatosed deleted
  • Status changed from reopened to assigned

@rishabhwp
8 months ago

Attaching a refreshed patch for #60196. The necessary changes were made to the .scss file, and the corresponding .css file was regenerated using the build process.

#24 @rishabhwp
8 months ago

  • Keywords has-patch added; needs-refresh removed

#25 @sabernhardt
8 months ago

@rishabhwp Could you combine PRs 8895 and 8940 (not the .patch files) into one pull request so it runs GitHub tests on both the margin and line-height changes together?

Then we can be more confident whether all six files are ready to commit:

  • assets/sass/05-blocks/utilities/_editor.scss
  • assets/sass/06-components/editor.scss
  • assets/css/style-editor.css
  • assets/css/ie-editor.css
  • assets/css/style-editor.css.map
  • assets/css/ie-editor.css.map

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


8 months ago
#26

#27 follow-up: @rishabhwp
8 months ago

@sabernhardt I’ve created a new combined PR with the changes from both PRs 8895 and 8940. The updates were made to the SCSS files, and then the build command was run to generate the respective CSS and map files. All six files listed above have been created as expected.

#28 in reply to: ↑ 27 @sabernhardt
8 months ago

  • Keywords changes-requested added

Your PR 8895 replaced the existing [data-block] with [data-block]:where(:not(.wp-block-list-item)), which was better than setting all margins to zero.

For my sites, I tend to add bottom padding to li elements in the content area, but it would be good to honor similar custom styles for anyone who uses margin instead.

#29 @rishabhwp
8 months ago

  • Keywords changes-requested removed

Thanks for the feedback, @sabernhardt!

I've made the requested changes. I removed the rule that set margins to zero for list items and updated the selector to exclude .wp-block-list-item from receiving vertical margins. This should better respect any custom styles applied to list items.

Let me know if anything else needs adjustment.

@sabernhardt commented on PR #9164:


7 months ago
#30

Test Report

Description

PR 9164 corrects both the margins for List Item blocks and the line-height for multiple blocks within the editor.

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

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.4.0
  • Server: local WAMP, Apache/2.4.62 (Win64) PHP/8.4.0 mod_fcgid/2.3.10-dev
  • Database: mysqli (Server: 9.1.0 / Client: mysqlnd 8.4.0)
  • Browser: Firefox 141.0
  • OS: Windows 11
  • Theme: Twenty Twenty-One 2.6
  • MU Plugins: custom plugin to add bottom margin to list items in specific List blocks
  • Plugins: only Test Reports 1.2.0

Actual Results

Editor margins and line-height values match the front end.

  • ✅ List Item blocks now have zero margin on top and bottom (by default).
  • ✅ List items honor custom margins applied to the li element.
  • ✅ List, Navigation, Preformatted, Code, and Verse blocks now have a line-height of 1.7.
  • ✅ List, Navigation, Preformatted, Code, and Verse blocks still respect the Line Height block setting under Typography.
  • ✅ The month caption and navigation links in Calendar blocks now have a line-height of 1.7, and they respect the Line Height block setting.
  • ✅ Paragraph blocks still have a line-height of 1.7.
  • ✅ Heading blocks (H2) still have a line-height of 1.3.

Additional Notes

  • I used this set of blocks. If you paste these, replace the ref number in the Navigation blocks with a menu on your site.
  • The Preformatted block font size is a separate issue on Trac 60993.
  • Firefox/Windows adds an extra pixel of height to each list item in an unordered list on the front end. Chrome matches the height.
  • The custom li style did not apply when added to the Customizer's Additional CSS. The following code could work (well enough for testing) in a child theme, but I added it in a special plugin instead.
    function add_custom_margin_to_list_items() {
    	wp_register_style( 't21-extra', false );
    	wp_add_inline_style( 't21-extra', '.li-spacing li { margin-bottom: 0.5em; }' );
    	wp_enqueue_style( 't21-extra' );
    }
    add_action( 'wp_enqueue_scripts',   'add_custom_margin_to_list_items' );
    add_action( 'enqueue_block_assets', 'add_custom_margin_to_list_items' );
    

Supplemental Artifacts

Chrome screenshots, comparing front end and post editor with the patch:

margin and line height match

Firefox screenshots:

#31 @shailu25
5 months ago

Test Report

This Report Validates that the indicated patch address the issue.✅

Patch Tested: https://github.com/WordPress/wordpress-develop/pull/9164

Environment:

WordPress - 6.9-alpha-20250702.070537
OS - Windows
Browser - Chrome
Theme: Twenty Twenty One
PHP - 7.4.31
Plugin - None

Actual Results:

  • Issue Resolved with Patch ✅ (Editor line-height values match the frontend side.)

Supplemental Artifacts

  • Attached Screenshots

@shailu25
5 months ago

Before Patch Backend

@shailu25
5 months ago

After Patch Backend

@shailu25
5 months ago

After Patch Frontend

#32 @sabernhardt
5 months ago

  • Keywords commit added

PR 9164 could close #60196 and #63549.

#33 @joedolson
4 months ago

  • Owner set to joedolson
  • Status changed from assigned to accepted

#34 @joedolson
4 months ago

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

In 61106:

Twenty Twenty One: Fix editor spacing with inner blocks.

Fix margin on inner blocks (e.g. list item blocks) and body line-height in the editor styles for Twenty Twenty One.

Props nidhidhandhukiya, rishabhwp, shailu25, ugyensupport, sabernhardt, karmatosed, sandeepdahiya, mleraygp, sirlouen, skithund, dilip2615.
Fixes #60196, #63549.

Note: See TracTickets for help on using tickets.