Page: Use browser native scrollbars for the main page content #82919
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
joshhunt
changed the title
joshhunt
force-pushed
the
joshhunt/no-custom-page-scrollbars
branch
from
c767bb7
to
ae9a5ad
Compare
joshhunt
force-pushed
the
joshhunt/no-custom-page-scrollbars
branch
4 times, most recently
from
90ca002
to
fc5627b
Compare
joshhunt
changed the title
joshhunt
requested review from
ashharrison90 and
eledobleefe
and removed request for
a team
joshhunt
commented
Mar 4, 2024
| color: theme.v1.palette.blue95, | ||
| }), | ||
| cards: css({ | ||
| overflowX: 'auto', |
There was a problem hiding this comment.
The actual change here is from overflow-x: scroll to overflow-x: auto to prevent the scroll bars from showing when there's no overflow.
ryantxu
reviewed
Mar 4, 2024
joshhunt
force-pushed
the
joshhunt/no-custom-page-scrollbars
branch
from
6066da0
to
f92815d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
|
@joshhunt are there doc changes needed for this or should we remove the docs label? |
|
@derek-cadzow No. I believe it got auto-tagged with it due to the addition of the feature toggle. |
kaydelaney
approved these changes
Mar 6, 2024
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.
This PR removes the
<CustomScrollbar />component from the main<Page />, to prefer using platform default scrollbars. This behaviour is controlled via thebetterPageScrollingfeature toggle, which is enabled by default.As raised in #80429, there's now cross-browser support for customising the native browser scrollbar in a limited and declarative manner. However, I believe there is little need to customise the scroll bar at at least the page level - we should defer to doing less and preferring platform defaults.
This PR adds a new 'FlaggedScrollbars' interim component that wraps grafana-ui's CustomScrollbars, or uses a
NativeScrollbarscomponent when the toggle is enabled. The goal is to later removeFlaggedScrollbarsandNativeScrollbars, and just scroll the document body element.Part of #80429
Screenshots
On Windows, Chrome does not use 'overlay' scroll bars, so they used to get CustomScrollbars. Firefox does use overlay scroll bars, is it never used CustomScrollbars, and should see no change through this
Regular tall page
Non-scrolling page
Small dashboard without scroll bars
Large dashboard with scroll bars