Edit Post: Hide overflowing content in visual editor wrapper to prevent block popovers from creating scrollbars #49978
Conversation
This file contains hidden or 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
…nt block popovers from creating scrollbars
andrewserong
added
[Type] Bug
andrewserong
requested review from
aaronrobertshaw,
apeatling,
ciampo,
ellatrix,
jasmussen,
ntsekouras,
ramonjd,
talldan,
tellthemachines and
youknowriad
Contributor
Author
|
Apologies for the large number of pings for reviews — while this is only a single line change, I thought I'd cast the net wide, just in case anyone's aware of problems this rule might cause! Feel free to ignore if this isn't an area any of you are working in 🙂 |
6 tasks
|
Size Change: +6 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Contributor
From memory, I think the template editing mode and the Mobile / Tablet views are worth testing. I couldn't spot any regressions. |
Member
|
I could reproduce problem and |
Contributor
Author
Thanks for testing! The site editor already has the |
Contributor
|
I could replicate the issue and confirm this PR fixes it for me. FWIW the scrollbar setting was different for me using MacOS Ventura. It was under Appearance rather than General but the double scrollbars showed up without turning on "always" show scrollbars.
|
Member
That would do it 🤕 |
Contributor
|
Thank you for attention to detail here, this one is important. I recall similar issues in the past, in case #30055 can provide some context on what used to be the issue and how that was fixed. |
Contributor
Author
|
Thanks for linking through that earlier discussion @jasmussen! It looks like the fix for the site editor was in #43332 from @ciampo and reviewed/merged by @talldan, which added the I think this one should be good to go now, but I'll just add a slightly bigger ping to @WordPress/gutenberg-core just in case. |
ramonjd
approved these changes
Apr 23, 2023
Member
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Based on fix for site editor, this LGTM
Contributor
Author
|
Thanks for the approving review @ramonjd! Out of an abundance of caution, since this updates a styling rule of a prominent part of the editor, I'll leave this PR open until I'm back on Wednesday, and will merge in then if there aren't any objections 🙂 (If anyone else would like to, feel free to merge in before then) |
aristath
approved these changes
Apr 24, 2023
apeatling
approved these changes
Apr 24, 2023
Contributor
apeatling
left a comment
apeatling
left a comment
There was a problem hiding this comment.
✅ Confirmed works as expected LGTM
Contributor
Author
|
Thanks for the confidence check / reviews everyone! 🙇 |
andrewserong
deleted the
update/hide-scrollbars-in-edit-post-visual-editor
branch
This was referenced Aug 10, 2023
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?
Add
overflow: hiddento the.edit-post-visual-editorclass, so that block popovers don't cause additional scrollbars in the editor canvas.Why?
Block popovers are rendered into a popover slot that has the
edit-post-visual-editorwrapper as its parent, withposition: relativeset on it. The current behaviour withBlockPopoverand floating UI is that popovers are rendered using absolute positioning, so when a block popover goes past the bottom of the screen, it results in theedit-post-visual-editorwrapper overflowing. While we could also look into updating the popover behaviour here, I thought a good step would be to prevent these overflowing popovers from causing a scrollbar to be displayed. (The real post content is scrolled via the.interface-interface-skeleton__contentwrapper, so I think this should hopefully be a safe change).This issue currently only appears in the post editor, as the site editor already has a corresponding
overflow: hiddenrule on this line, added back in #44770. To test and reproduce the issue, if you're on a Mac, you'll likely want to set scrollbars in the OS to display Always for the issue to be visible.The kinds of popovers that result in the additional scrollbar include the block toolbar once it's scrolled past the bottom of the screen, but also Padding and Margin visualisers in the Group block, for example. I noticed this issue while working on a visualiser for position support over in #49321.
How?
overflow: hiddento theedit-post-visual-editorclass.Testing Instructions
Before testing, make sure your OS is set to always display scrollbars. On a Mac, go to System Preferences > General and set it to Always. Here's a screenshot:
trunkwith a long post in the post editor, select a block somewhere around the middle of the page and scroll up and down the editor canvas. Notice that a second scrollbar appears once the block toolbar goes past the bottom of the screen.Testing Instructions for Keyboard
Screenshots or screencast
Before
2023-04-21.12.35.59.mp4
After
2023-04-21.12.37.11.mp4