Browse Mode: Remove horizontal scrollbar during screen transitions #51189
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
[Type] Bug
andrewserong
requested review from
oandregal,
noisysocks,
jameskoster,
SaxonF,
jasmussen,
ramonjd and
tellthemachines
|
Size Change: -12 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
tellthemachines
approved these changes
Jun 2, 2023
There was a problem hiding this comment.
Thanks for fixing this! The flashing scrollbar is quite annoying 😄
The style seems to have been added in this commit to make the sidebar heading sticky. Testing these changes, the heading is still behaving stickily so we should be fine! (I'd expect overflow-y to have more of an influence in the sticky behaviour than overflow-x, anyway.)
|
Judging from the before/after, this looks good, thank you for providing those visuals for a quick sanity check. |
|
I've tested this on a different OS (linux) using chrome and firefox: I can repro the horizontal scrollbar is present in As for what that rule was for: I cannot provide much insight. It was a very collaborative PR and I see it was introduced at d61d0c2 and it seems it has to do with sticking the header #45100 (comment) As far as I can see, it still works as expected. cc @youknowriad in case he remembers any other thing that could break with this. |
|
I don't remember to be honest. I tested quickly and I don't see any obvious regression here. |
|
Thanks for testing and confirming, folks! I'll merge this in, but it's an easy revert if we run into any issues 🙂 |
andrewserong
deleted the
fix/scrollbar-when-transitioning-screens-in-browse-mode
branch
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 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?
Remove the
overflow-x: unset;rule for the browse mode sidebar content, as it resulted in horizontal scrollbars appearing during screen transitions in browsers that are set to always display scrollbars.Why?
To improve how it feels to navigate between screens on browsers / OSes that are set to always display scrollbars
How?
overflow-x: unset;rule for the sidebar contentTesting Instructions
Set your OS's accessibility settings to always show scrollbars (if on a Mac). Here's how it looks in Monterey:
A question for the reviewer: was this rule needed? Will this removal break anything? I see the rule was added in #45100 by @oandregal, but removing it doesn't immediately appear to break anything, but I might have missed something!
Screenshots or screencast
Look for the flash of horizontal scrollbar during screen transitions below: