Fix : Paragraph appender layout shift

[Vrishabhsk]

What?

• Adds a style that targets block-editor-default-block-appender__content placeholder
• The style sets margin-block-start & margin-block-end to 0

Why?

• Paragraph appender: layout shift when Content block fills the container #65969
• The style margin-block-start & margin-block-end are set as 1em w.r.t styles associated to the p tag
• When Type / to choose a block is clicked, the following function is executed, which causes the layout shift due to replacement of several elements and their style.

const onAppend = () => {
	insertDefaultBlock( undefined, rootClientId );
	startTyping();
};

How?

• The style added here ensure the layout of the content in the editor does not shift when onClick is triggered

[github-actions]

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
• Labels found: .

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Vrishabhsk <vrishabhsk@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[jasmussen]

Thanks for the PR. As an override, I imagine this can work, but I'd like to see if we can get to the root of why the rule to zero out margins exists for constrained layouts in the first place, and not outside of it. To that end, we may need to get closer to the layout engine. I pinged Aaron in case he knows, but it's possible he hasn't been involved with layout, in which case hopefully he can pass on the ping to someone else.

[aaronrobertshaw]

Thanks for the ping @jasmussen and spinning up this PR @Vrishabhsk 👍

I'm no layout expert so take the following with a grain of salt. It might also pay to get @andrewserong and @tellthemachines (once they are back on deck) to weigh in.

With that said, here's what I found digging into this a little:

• The paragraph element within the block list appender gets extraneous spacing due to user agent margins set by the browser.
• As noted on the original issue and this PR, those margins are reset on a paragraph block leading to the layout shift issue
• My suspicion is that these margins should be reset across the board so that whatever is being set by layouts take affect on the appender's wrapper.
• The BlockListAppender isn't exported from the block editor package and only used within the BlockList component which should make the blanket margin reset safer.

but I'd like to see if we can get to the root of why the rule to zero out margins exists for constrained layouts in the first place, and not outside of it.

• These margins were reset for the constrained layout arbitrarily in Editor initrial appender: Zero out margins in constrained layouts. #52026
• The reset styles above can be seen here

---

In terms of this PR, the selector for the proposed styles has a very high specificity (0-4-1) and could be simplified a bit.

There are a couple of options:

1. Simply reset the paragraph margins within the block list appender.

diff --git a/packages/block-editor/src/components/default-block-appender/content.scss b/packages/block-editor/src/components/default-block-appender/content.scss
index 51e0b4381a..c82a04e8c7 100644
--- a/packages/block-editor/src/components/default-block-appender/content.scss
+++ b/packages/block-editor/src/components/default-block-appender/content.scss
@@ -24,20 +24,11 @@
 	.block-editor-default-block-appender__content {
 		// Set the opacity of the initial block appender to the same as placeholder text in an empty Paragraph block.
 		opacity: 0.62;
-	}
-
-	// In "constrained" layout containers, the first and last paragraphs have their margins zeroed out.
-	// In the case of this appender, it needs to apply those same rules to avoid layout shifts.
-	// Such shifts happen when the bottom margin of the Title block has been set to less than the default 1em margin of paragraphs.
-	:where(body .is-layout-constrained) &,
-	:where(.wp-site-blocks) & {
-		> :first-child {
-			margin-block-start: 0;
-			margin-block-end: 0;
-		}
 
-		// Since this appender will only ever appear on an entirely empty document, we don't account for last-child.
-		// This is also because it will never be the last child, the block inserter that sits in this appender is the last child.
+		// The following prevent user agent styles from applying margins to the appender's inner paragraph.
+		// This in turn prevents layout shift do to layout styles removing margins from first and last children.
+		margin-block-start: 0;
+		margin-block-end: 0;
 	}
 
 	// Dropzone.

2. Reset the paragraph margins when the appender is within any block with a class matching `is-layout-*`

diff --git a/packages/block-editor/src/components/default-block-appender/content.scss b/packages/block-editor/src/components/default-block-appender/content.scss
index 51e0b4381a..384c0986b3 100644
--- a/packages/block-editor/src/components/default-block-appender/content.scss
+++ b/packages/block-editor/src/components/default-block-appender/content.scss
@@ -29,7 +29,7 @@
 	// In "constrained" layout containers, the first and last paragraphs have their margins zeroed out.
 	// In the case of this appender, it needs to apply those same rules to avoid layout shifts.
 	// Such shifts happen when the bottom margin of the Title block has been set to less than the default 1em margin of paragraphs.
-	:where(body .is-layout-constrained) &,
+	:where(body [class*="is-layout-"]) &,
 	:where(.wp-site-blocks) & {
 		> :first-child {
 			margin-block-start: 0;

Both of the options above applied to trunk resolve the layout shift for me. I didn't spot any regressions from those changes but have only done some basic smoke testing.

I hope that helps!

[Vrishabhsk]

Thanks @aaronrobertshaw

• After running a search, I found that block-editor-default-block-appender__content class was set only in the default-block-appender
• I have implemented the first suggestion

Simply reset the paragraph margins within the block list appender.

[andrewserong]

Thanks for the ping!

My suspicion is that these margins should be reset across the board so that whatever is being set by layouts take affect on the appender's wrapper.

That's my suspicion, too. The way the default appender appears to work is that it's a paragraph element wrapped in a div, and that paragraph is rendered to look like a paragraph block. This means that the styling needs to match what a paragraph block would look like within a container, and the way each of the layouts work is that the first block within a container has its top margin set to 0, even if that block has its own margin styles in global styles. So overall the idea of applying this in all cases sounds good to me 👍

I suppose a potential edge case could be a classic theme that doesn't expect the styles to work in this way. But even then, for Gutenberg to provide a default for this component that it sets margins to 0 sounds reasonable to me.

This is working well in testing. I notice that the rule directly after it applies to the first child (the rules that currently target is-layout-constrained). As block-editor-default-block-appender__content is the first child, do we still need that set of rules if we're going with always applying the 0 margin?

[aaronrobertshaw]

That's my suspicion, too.

Thanks for the confirmation @andrewserong 👍

do we still need that set of rules if we're going with always applying the 0 margin?

I don't think we do. That's why I removed them in the snippets I floated previously. Looks like it was just missed that's all.

I suppose a potential edge case could be a classic theme that doesn't expect the styles to work in this way.

Yeah, I wasn't too sure how far to follow that thread. After tweaking styles locally, I only had a quick smoke test of the usual classic theme suspects like TT1 etc. They seemed ok to me.

Click clip of TT1 appender

Screen.Recording.2024-10-15.at.5.34.19.pm.mp4

But even then, for Gutenberg to provide a default for this component that it sets margins to 0 sounds reasonable to me.

Agreed.

[andrewserong]

I was in the process of giving this a rebase but noticed that in trunk there's a regression for the position of the appender. I've left a comment on the PR where it appears to have been introduced #66630 (review) — a may not get a chance to work on a fix before the end of the week, though. IMO that regression will need to be resolved before we attempt to land this one.

[andrewserong]

Now that the fix in #66711 has been merged, I attempted to give this PR a rebase and update, but for some reason I'm unable to push to this branch.

I hope you don't mind @Vrishabhsk, but I've copied your work here over into a fresh PR in #66779, incorporating some of the feedback from @aaronrobertshaw in #66061 (review).

Thanks again for all your work here!

[getdave]

Now #66779 has merged can this be closed out?

[andrewserong]

Yes, I'll close this out now. Thanks again for the fix @Vrishabhsk!