Editor: fix scrollbar doubling #62632
Closed
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
|
Size Change: +15 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
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. |
|
There's a lot of changes going on here which makes me nervous to include it in 6.6. What is the minimal fix without improving anything else? |
stokesman
force-pushed
the
fix/editor-scrollbar-doubling
branch
from
988996c
to
314035a
Compare
|
I dropped the changes that improved anything else. There are alternatives approaches that could be more minimal. Here’s one that I didn’t prefer because there’s more coupling (some of it cross-package) when ideally none of these components have to concern themselves with each other. Also, it uses diff --git a/packages/edit-post/src/components/layout/style.scss b/packages/edit-post/src/components/layout/style.scss
index 2c5a035cad..5fdaceaa00 100644
--- a/packages/edit-post/src/components/layout/style.scss
+++ b/packages/edit-post/src/components/layout/style.scss
@@ -1,8 +1,3 @@
-.editor-visual-editor {
- flex: 1 0 auto;
- height: auto;
-}
-
.edit-post-layout__metaboxes {
flex-shrink: 0;
clear: both;
diff --git a/packages/editor/src/components/editor-interface/style.scss b/packages/editor/src/components/editor-interface/style.scss
index e7e0a93af2..c2d6777d21 100644
--- a/packages/editor/src/components/editor-interface/style.scss
+++ b/packages/editor/src/components/editor-interface/style.scss
@@ -1,3 +1,7 @@
.editor-editor-interface .entities-saved-states__panel-header {
height: $header-height + $border-width;
}
+
+.editor-editor-interface:has(.editor-visual-editor.is-iframed) .interface-interface-skeleton__content {
+ overflow: hidden;
+}
Here’s one I like better. It’s encapsulated and two birds with one stone because it also fixes #62639 but would probably create the same caution for 6.6 that some folks had about #62681. diff --git a/packages/editor/src/components/visual-editor/style.scss b/packages/editor/src/components/visual-editor/style.scss
index 597769ff7f..42f4fe2d7d 100644
--- a/packages/editor/src/components/visual-editor/style.scss
+++ b/packages/editor/src/components/visual-editor/style.scss
@@ -3,6 +3,8 @@
height: 100%;
display: block;
background-color: $gray-300;
+ // Make this element the context for positioning and stacking (z-index) of children.
+ contain: layout;
// Centralize the editor horizontally (flex-direction is column).
align-items: center;
|
ellatrix
added
[Feature] Zoom Out
Backport to WP 6.7 Beta/RC
|
Would love some opinion from CSS experts (maybe @jasmussen or if you know someone else 😃) |
|
Okay, I realized there was one more thing this was still doing besides fixing the double scrollbars and pushed a commit to revert that. Now it’s about as minimal as any of the alternative solutions I mentioned. What this was doing besides was allowing horizontal overflow when not zoomed out. That was so that if a device preview overflows horizontally it’s not clipped and can be scrolled. It seems like an oversight when zoom out was added but it’s probably a minor concern. I removed the zoom out label since now the changes don’t touch anything related. |
|
The one thing I originally included in this PR that I think might qualify to include is fixing the triple scrollbars that appear in the post editor in device previews. I mentioned it in the issue #62319 (comment). It is not present 6.5 so also a regression we’d ideally fix for 6.6. It can be done simply: diff --git a/packages/edit-post/src/components/layout/style.scss b/packages/edit-post/src/components/layout/style.scss
index 2c5a035cad..5fdaceaa00 100644
--- a/packages/edit-post/src/components/layout/style.scss
+++ b/packages/edit-post/src/components/layout/style.scss
@@ -1,8 +1,3 @@
-.editor-visual-editor {
- flex: 1 0 auto;
- height: auto;
-}
-
.edit-post-layout__metaboxes {
flex-shrink: 0;
clear: both;
I don’t really see it as a separate issue but I'll put it in separate PR if that’s what others think is best. |
|
Thanks for the ping. Based on the most recent changes there's not a lot of CSS, so it might be safer now. In a quick surface test this seems to work as intended, though I had a hard time replicating the double scrollbar in trunk myself. It's a hard one to test deeply, while these are indeed sensitive rules, it's also hard to foresee every side effect. It seems fine given its current small footprint. |
|
I also can't reproduce the original issue. 🤔 |
|
Which browser are you using? |
Maybe it depends on having OS preferences so that scrollbars always show. I would think it doesn’t but maybe it’s hard to tell that there are nested areas of overflow without. I'm using Chrome/macOS. |
|
In my environment (Windows OS, Chrome), I can consistently reproduce this issue by following these steps. If you're on a Mac, you may need to change your OS settings to always show the scroll bars.
I have confirmed that this issue is resolved in this PR. 6174b6412f8ad477bcdc95134692ec83.mp4 |
|
#62894 resolved this nicely so I'll close. |
stokesman
removed
the
Backport to WP 6.7 Beta/RC
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?
An improvement of overflow handling in the block canvas.
Why?
Fixes #62319 (double scrollbars when selected block is out of view).
How?
Makes the block canvas contained by the height of its parent so the parent won’t overflow.
Testing Instructions
Fixed scrollbar doubling
Testing Instructions for Keyboard
The general instructions should be applicable.
Screenshots or screencast
62632.mp4