Page: Use browser native scrollbars for the main page content (#82919)

* remove custom scroll bars from Page component

* make flagged scroller the actual scrolling element,

* enable feature flag by default

* re-enable the scroll props in Page

* rename feature toggle

* fix css

* only update when deleted

* set .scrollbar-view on our scrolling wrapper

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
This commit is contained in:
Josh Hunt 2024-03-06 15:06:47 +00:00 committed by GitHub
parent 0929bf7c00
commit 6a4e0c692a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 133 additions and 40 deletions

View File

@ -5834,12 +5834,6 @@ exports[`better eslint`] = {
[0, 0, 0, "Styles should be written using objects.", "3"],
[0, 0, 0, "Styles should be written using objects.", "4"]
],
"public/app/plugins/panel/gettingstarted/components/Step.tsx:5381": [
[0, 0, 0, "Styles should be written using objects.", "0"],
[0, 0, 0, "Styles should be written using objects.", "1"],
[0, 0, 0, "Styles should be written using objects.", "2"],
[0, 0, 0, "Styles should be written using objects.", "3"]
],
"public/app/plugins/panel/gettingstarted/components/TutorialCard.tsx:5381": [
[0, 0, 0, "Styles should be written using objects.", "0"],
[0, 0, 0, "Styles should be written using objects.", "1"],

View File

@ -56,6 +56,7 @@ Some features are enabled by default. You can disable these feature by setting t
| `lokiQueryHints` | Enables query hints for Loki | Yes |
| `alertingPreviewUpgrade` | Show Unified Alerting preview and upgrade page in legacy alerting | Yes |
| `alertingQueryOptimization` | Optimizes eligible queries in order to reduce load on datasources | |
| `betterPageScrolling` | Removes CustomScrollbar from the UI, relying on native browser scrollbars | Yes |
| `alertingUpgradeDryrunOnStart` | When activated in legacy alerting mode, this initiates a dry-run of the Unified Alerting upgrade during each startup. It logs any issues detected without implementing any actual changes. | Yes |
## Preview feature toggles

View File

@ -178,6 +178,7 @@ export interface FeatureToggles {
kubernetesAggregator?: boolean;
expressionParser?: boolean;
groupByVariable?: boolean;
betterPageScrolling?: boolean;
alertingUpgradeDryrunOnStart?: boolean;
scopeFilters?: boolean;
}

View File

@ -1188,6 +1188,14 @@ var (
HideFromDocs: true,
HideFromAdminPage: true,
},
{
Name: "betterPageScrolling",
Description: "Removes CustomScrollbar from the UI, relying on native browser scrollbars",
Stage: FeatureStageGeneralAvailability,
FrontendOnly: true,
Owner: grafanaFrontendPlatformSquad,
Expression: "true", // enabled by default
},
{
Name: "alertingUpgradeDryrunOnStart",
Description: "When activated in legacy alerting mode, this initiates a dry-run of the Unified Alerting upgrade during each startup. It logs any issues detected without implementing any actual changes.",

View File

@ -159,5 +159,6 @@ newPDFRendering,experimental,@grafana/sharing-squad,false,false,false
kubernetesAggregator,experimental,@grafana/grafana-app-platform-squad,false,true,false
expressionParser,experimental,@grafana/grafana-app-platform-squad,false,true,false
groupByVariable,experimental,@grafana/dashboards-squad,false,false,false
betterPageScrolling,GA,@grafana/grafana-frontend-platform,false,false,true
alertingUpgradeDryrunOnStart,GA,@grafana/alerting-squad,false,true,false
scopeFilters,experimental,@grafana/dashboards-squad,false,false,false

1 Name Stage Owner requiresDevMode RequiresRestart FrontendOnly
159 kubernetesAggregator experimental @grafana/grafana-app-platform-squad false true false
160 expressionParser experimental @grafana/grafana-app-platform-squad false true false
161 groupByVariable experimental @grafana/dashboards-squad false false false
162 betterPageScrolling GA @grafana/grafana-frontend-platform false false true
163 alertingUpgradeDryrunOnStart GA @grafana/alerting-squad false true false
164 scopeFilters experimental @grafana/dashboards-squad false false false

View File

@ -647,6 +647,10 @@ const (
// Enable groupBy variable support in scenes dashboards
FlagGroupByVariable = "groupByVariable"
// FlagBetterPageScrolling
// Removes CustomScrollbar from the UI, relying on native browser scrollbars
FlagBetterPageScrolling = "betterPageScrolling"
// FlagAlertingUpgradeDryrunOnStart
// When activated in legacy alerting mode, this initiates a dry-run of the Unified Alerting upgrade during each startup. It logs any issues detected without implementing any actual changes.
FlagAlertingUpgradeDryrunOnStart = "alertingUpgradeDryrunOnStart"

View File

@ -2087,6 +2087,19 @@
"stage": "experimental",
"codeowner": "@grafana/grafana-app-platform-squad"
}
},
{
"metadata": {
"name": "betterPageScrolling",
"resourceVersion": "1709583501630",
"creationTimestamp": "2024-03-04T20:18:21Z"
},
"spec": {
"description": "Removes CustomScrollbar from the UI, relying on native browser scrollbars",
"stage": "GA",
"codeowner": "@grafana/grafana-frontend-platform",
"frontend": true
}
}
]
}

View File

@ -126,7 +126,7 @@ func TestFeatureToggleFiles(t *testing.T) {
item.Annotations[utils.AnnoKeyUpdatedTimestamp] = created.String()
item.Spec = v // the current value
}
} else {
} else if item.DeletionTimestamp == nil {
item.DeletionTimestamp = &created
fmt.Printf("mark feature as deleted")
}

View File

@ -132,7 +132,11 @@ export class GrafanaApp {
initIconCache();
// This needs to be done after the `initEchoSrv` since it is being used under the hood.
startMeasure('frontend_app_init');
addClassIfNoOverlayScrollbar();
if (!config.featureToggles.betterPageScrolling) {
addClassIfNoOverlayScrollbar();
}
setLocale(config.bootData.user.locale);
setWeekStart(config.bootData.user.weekStart);
setPanelRenderer(PanelRenderer);

View File

@ -0,0 +1,54 @@
import { css, cx } from '@emotion/css';
import React, { useEffect, useRef } from 'react';
import { config } from '@grafana/runtime';
import { CustomScrollbar, useStyles2 } from '@grafana/ui';
type FlaggedScrollerProps = Parameters<typeof CustomScrollbar>[0];
export default function FlaggedScrollbar(props: FlaggedScrollerProps) {
if (config.featureToggles.betterPageScrolling) {
return <NativeScrollbar {...props}>{props.children}</NativeScrollbar>;
}
return <CustomScrollbar {...props} />;
}
// Shim to provide API-compatibility for Page's scroll-related props
function NativeScrollbar({ children, scrollRefCallback, scrollTop }: FlaggedScrollerProps) {
const styles = useStyles2(getStyles);
const ref = useRef<HTMLDivElement>(null);
useEffect(() => {
if (ref.current && scrollRefCallback) {
scrollRefCallback(ref.current);
}
}, [ref, scrollRefCallback]);
useEffect(() => {
if (ref.current && scrollTop != null) {
ref.current?.scrollTo(0, scrollTop);
}
}, [scrollTop]);
return (
// Set the .scrollbar-view class to help e2e tests find this, like in CustomScrollbar
<div ref={ref} className={cx(styles.nativeScrollbars, 'scrollbar-view')}>
{children}
</div>
);
}
function getStyles() {
return {
nativeScrollbars: css({
label: 'native-scroll-container',
minHeight: `calc(100% + 0px)`, // I don't know, just copied from custom scrollbars
maxHeight: `calc(100% + 0px)`, // I don't know, just copied from custom scrollbars
display: 'flex',
flexDirection: 'column',
flexGrow: 1,
overflow: 'auto',
}),
};
}

View File

@ -1,11 +1,12 @@
// Libraries
import { css, cx } from '@emotion/css';
import React, { useLayoutEffect } from 'react';
import { GrafanaTheme2, PageLayoutType } from '@grafana/data';
import { CustomScrollbar, useStyles2 } from '@grafana/ui';
import { useStyles2 } from '@grafana/ui';
import { useGrafana } from 'app/core/context/GrafanaContext';
import FlaggedScrollbar from '../FlaggedScroller';
import { PageContents } from './PageContents';
import { PageHeader } from './PageHeader';
import { PageTabs } from './PageTabs';
@ -52,7 +53,7 @@ export const Page: PageType = ({
return (
<div className={cx(styles.wrapper, className)} {...otherProps}>
{layout === PageLayoutType.Standard && (
<CustomScrollbar autoHeightMin={'100%'} scrollTop={scrollTop} scrollRefCallback={scrollRef}>
<FlaggedScrollbar autoHeightMin={'100%'} scrollTop={scrollTop} scrollRefCallback={scrollRef}>
<div className={styles.pageInner}>
{pageHeaderNav && (
<PageHeader
@ -67,13 +68,15 @@ export const Page: PageType = ({
{pageNav && pageNav.children && <PageTabs navItem={pageNav} />}
<div className={styles.pageContent}>{children}</div>
</div>
</CustomScrollbar>
</FlaggedScrollbar>
)}
{layout === PageLayoutType.Canvas && (
<CustomScrollbar autoHeightMin={'100%'} scrollTop={scrollTop} scrollRefCallback={scrollRef}>
<FlaggedScrollbar autoHeightMin={'100%'} scrollTop={scrollTop} scrollRefCallback={scrollRef}>
<div className={styles.canvasContent}>{children}</div>
</CustomScrollbar>
</FlaggedScrollbar>
)}
{layout === PageLayoutType.Custom && children}
</div>
);
@ -95,6 +98,9 @@ const getStyles = (theme: GrafanaTheme2) => {
label: 'page-content',
flexGrow: 1,
}),
primaryBg: css({
background: theme.colors.background.primary,
}),
pageInner: css({
label: 'page-inner',
padding: theme.spacing(2),

View File

@ -20,9 +20,15 @@ export interface PageProps extends HTMLAttributes<HTMLDivElement> {
subTitle?: React.ReactNode;
/** Control the page layout. */
layout?: PageLayoutType;
/** Can be used to get the scroll container element to access scroll position */
/**
* Can be used to get the scroll container element to access scroll position
* */
// Probably will deprecate this in the future in favor of just scrolling document.body directly
scrollRef?: RefCallback<HTMLDivElement>;
/** Can be used to update the current scroll position */
/**
* Can be used to update the current scroll position
* */
// Probably will deprecate this in the future in favor of just scrolling document.body directly
scrollTop?: number;
}

View File

@ -37,30 +37,30 @@ export const Step = ({ step }: Props) => {
const getStyles = (theme: GrafanaTheme2) => {
return {
setup: css`
display: flex;
width: 95%;
`,
info: css`
width: 172px;
margin-right: 5%;
setup: css({
display: 'flex',
width: '95%',
}),
info: css({
width: '172px',
marginRight: '5%',
${theme.breakpoints.down('xxl')} {
margin-right: ${theme.spacing(4)};
}
${theme.breakpoints.down('sm')} {
display: none;
}
`,
title: css`
color: ${theme.v1.palette.blue95};
`,
cards: css`
overflow-x: scroll;
overflow-y: hidden;
width: 100%;
display: flex;
justify-content: flex-start;
`,
[theme.breakpoints.down('xxl')]: {
marginRight: theme.spacing(4),
},
[theme.breakpoints.down('sm')]: {
display: 'none',
},
}),
title: css({
color: theme.v1.palette.blue95,
}),
cards: css({
overflowX: 'auto',
overflowY: 'hidden',
width: '100%',
display: 'flex',
justifyContent: 'flex-start',
}),
};
};

View File

@ -115,6 +115,7 @@
}
// Scrollbars
// Note, this is not applied by default if the `betterPageScrolling` feature flag is applied
.no-overlay-scrollbar {
::-webkit-scrollbar {
width: 8px;