From 50dd95c09b537400b27f8ba4e2d300ca60ba9ac2 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Tue, 18 Jun 2024 14:33:08 +0100 Subject: [PATCH] Chore: Remove `betterPageScrolling` toggle (#89339) * remove betterPageScrolling toggle * add scrollTo test shim --- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/services/featuremgmt/registry.go | 8 --- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 -- pkg/services/featuremgmt/toggles_gen.json | 3 +- public/app/app.ts | 11 --- ...laggedScroller.tsx => NativeScrollbar.tsx} | 13 +--- public/app/core/components/Page/Page.tsx | 10 +-- public/sass/_grafana.scss | 1 - public/sass/components/_scrollbar.scss | 72 ------------------- public/test/jest-setup.ts | 2 + 12 files changed, 11 insertions(+), 116 deletions(-) rename public/app/core/components/{FlaggedScroller.tsx => NativeScrollbar.tsx} (72%) delete mode 100644 public/sass/components/_scrollbar.scss diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 99c345d9eb0..f58f4a964c5 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -62,7 +62,6 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `logRowsPopoverMenu` | Enable filtering menu displayed when text of a log line is selected | Yes | | `lokiQueryHints` | Enables query hints for Loki | Yes | | `alertingQueryOptimization` | Optimizes eligible queries in order to reduce load on datasources | | -| `betterPageScrolling` | Removes CustomScrollbar from the UI, relying on native browser scrollbars | Yes | | `cloudWatchNewLabelParsing` | Updates CloudWatch label parsing to be more accurate | Yes | | `pluginProxyPreserveTrailingSlash` | Preserve plugin proxy trailing slash. | | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index ca52ce578c1..ecf1ee58d56 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -169,7 +169,6 @@ export interface FeatureToggles { kubernetesAggregator?: boolean; expressionParser?: boolean; groupByVariable?: boolean; - betterPageScrolling?: boolean; authAPIAccessTokenAuth?: boolean; scopeFilters?: boolean; ssoSettingsSAML?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index de3a47a560c..c51cd816c69 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1126,14 +1126,6 @@ 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: "authAPIAccessTokenAuth", Description: "Enables the use of Auth API access tokens for authentication", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index c920feb800e..28ceeea27f6 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -150,7 +150,6 @@ tlsMemcached,experimental,@grafana/grafana-operator-experience-squad,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 authAPIAccessTokenAuth,experimental,@grafana/identity-access-team,false,false,false scopeFilters,experimental,@grafana/dashboards-squad,false,false,false ssoSettingsSAML,preview,@grafana/identity-access-team,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 0cd93f4753f..6ff8c3ec65c 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -611,10 +611,6 @@ const ( // Enable groupBy variable support in scenes dashboards FlagGroupByVariable = "groupByVariable" - // FlagBetterPageScrolling - // Removes CustomScrollbar from the UI, relying on native browser scrollbars - FlagBetterPageScrolling = "betterPageScrolling" - // FlagAuthAPIAccessTokenAuth // Enables the use of Auth API access tokens for authentication FlagAuthAPIAccessTokenAuth = "authAPIAccessTokenAuth" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 731316282d3..4d8099eb846 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -463,7 +463,8 @@ "metadata": { "name": "betterPageScrolling", "resourceVersion": "1717578796182", - "creationTimestamp": "2024-03-06T15:06:47Z" + "creationTimestamp": "2024-03-06T15:06:47Z", + "deletionTimestamp": "2024-06-18T09:25:56Z" }, "spec": { "description": "Removes CustomScrollbar from the UI, relying on native browser scrollbars", diff --git a/public/app/app.ts b/public/app/app.ts index 770653fe974..f48ad85af7a 100644 --- a/public/app/app.ts +++ b/public/app/app.ts @@ -42,7 +42,6 @@ import { import { setPanelDataErrorView } from '@grafana/runtime/src/components/PanelDataErrorView'; import { setPanelRenderer } from '@grafana/runtime/src/components/PanelRenderer'; import { setPluginPage } from '@grafana/runtime/src/components/PluginPage'; -import { getScrollbarWidth } from '@grafana/ui'; import config, { updateConfig } from 'app/core/config'; import { arrayMove } from 'app/core/utils/arrayMove'; import { getStandardTransformers } from 'app/features/transformers/standardTransformers'; @@ -136,10 +135,6 @@ export class GrafanaApp { // This needs to be done after the `initEchoSrv` since it is being used under the hood. startMeasure('frontend_app_init'); - if (!config.featureToggles.betterPageScrolling) { - addClassIfNoOverlayScrollbar(); - } - setLocale(config.bootData.user.locale); setWeekStart(config.bootData.user.weekStart); setPanelRenderer(PanelRenderer); @@ -367,12 +362,6 @@ function initEchoSrv() { } } -function addClassIfNoOverlayScrollbar() { - if (getScrollbarWidth() > 0) { - document.body.classList.add('no-overlay-scrollbar'); - } -} - /** * Report when a metric of a given name was marked during the document lifecycle. Works for markers with no duration, * like PerformanceMark or PerformancePaintTiming (e.g. created with performance.mark, or first-contentful-paint) diff --git a/public/app/core/components/FlaggedScroller.tsx b/public/app/core/components/NativeScrollbar.tsx similarity index 72% rename from public/app/core/components/FlaggedScroller.tsx rename to public/app/core/components/NativeScrollbar.tsx index 9d9c3cc5043..76d447f4311 100644 --- a/public/app/core/components/FlaggedScroller.tsx +++ b/public/app/core/components/NativeScrollbar.tsx @@ -1,21 +1,12 @@ 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[0]; - -export default function FlaggedScrollbar(props: FlaggedScrollerProps) { - if (config.featureToggles.betterPageScrolling) { - return {props.children}; - } - - return ; -} +type Props = Parameters[0]; // Shim to provide API-compatibility for Page's scroll-related props -function NativeScrollbar({ children, scrollRefCallback, scrollTop, divId }: FlaggedScrollerProps) { +export default function NativeScrollbar({ children, scrollRefCallback, scrollTop, divId }: Props) { const styles = useStyles2(getStyles); const ref = useRef(null); diff --git a/public/app/core/components/Page/Page.tsx b/public/app/core/components/Page/Page.tsx index f88d0dd011b..c588d17be1e 100644 --- a/public/app/core/components/Page/Page.tsx +++ b/public/app/core/components/Page/Page.tsx @@ -5,7 +5,7 @@ import { GrafanaTheme2, PageLayoutType } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; import { useGrafana } from 'app/core/context/GrafanaContext'; -import FlaggedScrollbar from '../FlaggedScroller'; +import NativeScrollbar from '../NativeScrollbar'; import { PageContents } from './PageContents'; import { PageHeader } from './PageHeader'; @@ -53,7 +53,7 @@ export const Page: PageType = ({ return (
{layout === PageLayoutType.Standard && ( - }
{children}
- + )} {layout === PageLayoutType.Canvas && ( -
{children}
- + )} {layout === PageLayoutType.Custom && children} diff --git a/public/sass/_grafana.scss b/public/sass/_grafana.scss index f19901dc014..4137058f724 100644 --- a/public/sass/_grafana.scss +++ b/public/sass/_grafana.scss @@ -20,7 +20,6 @@ @import 'utils/widths'; // COMPONENTS -@import 'components/scrollbar'; @import 'components/buttons'; @import 'components/alerts'; @import 'components/tags'; diff --git a/public/sass/components/_scrollbar.scss b/public/sass/components/_scrollbar.scss deleted file mode 100644 index 1d2f90f130f..00000000000 --- a/public/sass/components/_scrollbar.scss +++ /dev/null @@ -1,72 +0,0 @@ -// Scrollbars -// Note, this is not applied by default if the `betterPageScrolling` feature flag is applied -.no-overlay-scrollbar { - ::-webkit-scrollbar { - width: 8px; - height: 8px; - } - - ::-webkit-scrollbar:hover { - height: 8px; - } - - ::-webkit-scrollbar-button:start:decrement, - ::-webkit-scrollbar-button:end:increment { - display: none; - } - ::-webkit-scrollbar-button:horizontal:decrement { - display: none; - } - ::-webkit-scrollbar-button:horizontal:increment { - display: none; - } - ::-webkit-scrollbar-button:vertical:decrement { - display: none; - } - ::-webkit-scrollbar-button:vertical:increment { - display: none; - } - ::-webkit-scrollbar-button:horizontal:decrement:active { - background-image: none; - } - ::-webkit-scrollbar-button:horizontal:increment:active { - background-image: none; - } - ::-webkit-scrollbar-button:vertical:decrement:active { - background-image: none; - } - ::-webkit-scrollbar-button:vertical:increment:active { - background-image: none; - } - ::-webkit-scrollbar-track-piece { - background-color: transparent; - } - - ::-webkit-scrollbar-thumb:vertical { - height: 50px; - background: -webkit-gradient( - linear, - left top, - right top, - color-stop(0%, $scrollbarBackground), - color-stop(100%, $scrollbarBackground2) - ); - border: 1px solid $scrollbarBorder; - border-top: 1px solid $scrollbarBorder; - border-left: 1px solid $scrollbarBorder; - } - - ::-webkit-scrollbar-thumb:horizontal { - width: 50px; - background: -webkit-gradient( - linear, - left top, - left bottom, - color-stop(0%, $scrollbarBackground), - color-stop(100%, $scrollbarBackground2) - ); - border: 1px solid $scrollbarBorder; - border-top: 1px solid $scrollbarBorder; - border-left: 1px solid $scrollbarBorder; - } -} diff --git a/public/test/jest-setup.ts b/public/test/jest-setup.ts index 4d34bb4a7fd..640db119ccd 100644 --- a/public/test/jest-setup.ts +++ b/public/test/jest-setup.ts @@ -67,6 +67,8 @@ global.IntersectionObserver = mockIntersectionObserver; global.TextEncoder = TextEncoder; global.TextDecoder = TextDecoder; +// add scrollTo interface since it's not implemented in jsdom +Element.prototype.scrollTo = () => {}; jest.mock('../app/core/core', () => ({ ...jest.requireActual('../app/core/core'),