Interface: prevent sidebar layout shift with explicit scroll area #31858
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
ellatrix
added
[Package] Interface
|
Size Change: +40 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
|
I'm pretty sure this used to not be an issue but haven't tracked down what brought on the change. There's another issue #31165 (Firefox and Safari) that the change in this PR could be said to resolve but not in the ideal way. That is, it resolves the horizontal overflow but it also constantly renders the horizontal scroll track. Here's a demo in Firefox: sidebar-overflow.mp4 |
|
Hm, right, we should set only |
ellatrix
force-pushed
the
try/sidebar-scroll-layout-shift
branch
from
c103461 to
c0649d8
Compare
@stokesman Fixed the issue now. |
|
👍 that does it. I tested in Firefox and Safari. I think it'd be fine to link #31165 as this does incidentally resolve it. |
|
Before:
After:
I'm conflicted. On the one hand I love that it removes the shift. On the other hand, that empty scrollbar is a bit annoying, especially for block inspectors which we can hopefully compress and compact through #27331 so that you won't need to scroll most of the time. @stokesman you say you remember this not being an issue — now you mention it I feel the same. Can you recall how it works? Was there reserved space for the scrollbar to take up? I think I remember the sidebar shifting inwards, which if that was the case then I think I like this one better as it doesn't cause a layout shift. It'd be nice to look up how it used to work. But otherwise, I lean towards us landing this, if nothing else then as an interim fix on the way to better things. |
The silver lining is that even layout shift within the sidebar is eliminated. Also the gutter is not always present as touch-enabled devices (including touchy mice like apple makes) employ an overlaid scrollbar. If we really wanted there's also
Last October when I worked on #26255, I made this screen cap: |
|
Great point. This seems worth it. I think we should try it. |
|
I think we should do it. Personally I find it pretty annoying :) |
|
Hi @jasmussen, instead of that fixed space for the scrollbar that is possible with
Like Slack: |
|
Hello Amir, thank you for the suggestion. I think you may actually be on to something. If we can get a CSS/custom scrollbar to work consistently across the browsers we support, it might actually be the best solution. We'll want to move a little carefully because it technically re-skins a very common operating system control. However the pros might outweight he cons, as it allows us to ensure good contrast with the control, it allows us to force the scrollbar visibility for people who prefer reduced motion, and it solves the layout shift problem. It might be worth just prototyping in a PR to see how it feels — we might find it appropriate some contexts, like the inspector sidebar, the List View, for popover menus, for when the block toolbar needs to scroll horizontally, or for the site editor navigation sidebar. |
|
Seems no one cares here, so let's close. |
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.
Description
This has been annoying me for a while.
How has this been tested?
Open the inspector and switch between selecting the heading and paragraph for example.
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).