Fix Paragraph appender layout shift (building on 66061) #66779
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
added
the
[Type] Bug
andrewserong
requested review from
ramonjd,
tellthemachines and
aaronrobertshaw
andrewserong
added
the
Backport to WP 6.7 Beta/RC
|
I've just added this to the 6.7 board in case there's still time to get it in. I'm not sure if it's possible at this stage, but @jasmussen flagged it as a good one for inclusion if it's possible, so thought I'd flag it there. |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -50 B (0%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
ramonjd
approved these changes
Nov 6, 2024
There was a problem hiding this comment.
I can't reproduce the original issue on any browser for any post type.
But since it reduces the CSS and I can't find any regressions, here's a ✅
Trunk
:where(.wp-site-blocks) .block-editor-default-block-appender>:first-child, :where(body .is-layout-constrained) .block-editor-default-block-appender>:first-child {
margin-block-end: 0;
margin-block-start: 0;
}This PR:
.block-editor-default-block-appender .block-editor-default-block-appender__content {
margin-block-end: 0;
margin-block-start: 0;
opacity: .62;
}
|
Thanks for the review @ramonjd! 🙇
Ah, I assume to reproduce we might need a template that sets the post content block to use default/flow layout instead of constrained 🤔 I'll update the test instructions. |
That was it! Thank you! Before2024-11-06.12.21.04.mp4After2024-11-06.12.21.32.mp4LGTM |
aaronrobertshaw
approved these changes
Nov 6, 2024
There was a problem hiding this comment.
Thanks for spinning up this PR @andrewserong and for the original work @Vrishabhsk 👍
✅ Could replicate original issue
✅ Code changes look good
✅ Logic to reset for all appenders makes sense
✅ Fixes the issue
✅ Couldn't spot any regressions
| Before | After |
|---|---|
Screen.Recording.2024-11-06.at.11.29.07.am.mp4 |
Screen.Recording.2024-11-06.at.11.31.10.am.mp4 |
|
Thanks for this one 🙏 |
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Nov 6, 2024
* Style to prevent Layout shift when placeholder is clicked * Refactor style placement to target block-appender__content specifically * Remove no longer needed rules --------- Co-authored-by: Vrishabhsk <vrishabhsk@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: d9b166a |
|
Noting that this missed the final RC3 so it is not certain to be included in 6.7. I'm seeing that the WP 6.7 Design Lead @jasmussen sees this as an important PR. It will be under consideration for inclusion if possible. |
|
I consider this a pretty critical bug, so would definitely like to see this included. |
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
) * Style to prevent Layout shift when placeholder is clicked * Refactor style placement to target block-appender__content specifically * Remove no longer needed rules --------- Co-authored-by: Vrishabhsk <vrishabhsk@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes: #65969
This PR is forked from #66061 (kudos @Vrishabhsk for the work here!)
Set the default block appender to have top and bottom margins of
0.Why?
As discussed in #66061 and #65969, the layout rules used in the post content block means that the first and last child of the post content area will have a
0margin at the top and bottom. The (fake) default paragraph appender therefore needs to have similar rules applied to it in order to override user agent styles that cause a layout shift when a user interacts with the default appender and creates the first block in a post or page.How?
Always apply
0top and bottom margins on the default appender, rather than only for particular layouts.A question for reviewers: does this rule seem safe enough to always be applied? So far in testing, it seems to feel safer to me to apply this reset-like style than not.
Testing Instructions
Note: in order to test the original issue in #65969 you might need to update the single post template's post content block to use the flow/default layout by switching off "Inner blocks use content width" on that block:
Screenshots or screencast
2024-11-06.11.09.13.mp4