From 8d05df83edc4ed2570ce36585c9554ec9ac08eea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= <pm.jamroz@gmail.com>
Date: Mon, 24 May 2021 10:34:37 +0200
Subject: [PATCH] CustomScrollbar: Invoke setScrollTop callback only after
 scrolling finishes (#34263)

* Invoke setScrollTop callback only after scrolling finishes

When the state is updated while scroll events are being dispatched (like in QueryGroup) it may cause resetting the scroll position to the first emitted event because setting the scroll happens only after render (useEffect).

* Memoize onScrollStop callback
---
 .../CustomScrollbar/CustomScrollbar.tsx          | 12 +++++++++---
 packages/grafana-ui/src/components/index.ts      |  2 +-
 .../dashboard/containers/DashboardPage.tsx       |  9 ++++-----
 .../app/features/query/components/QueryGroup.tsx | 16 ++++++++++++----
 4 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/packages/grafana-ui/src/components/CustomScrollbar/CustomScrollbar.tsx b/packages/grafana-ui/src/components/CustomScrollbar/CustomScrollbar.tsx
index 15f23febba1..412ad8d9ff0 100644
--- a/packages/grafana-ui/src/components/CustomScrollbar/CustomScrollbar.tsx
+++ b/packages/grafana-ui/src/components/CustomScrollbar/CustomScrollbar.tsx
@@ -2,10 +2,12 @@ import React, { FC, useCallback, useEffect, useRef } from 'react';
 import { isNil } from 'lodash';
 import classNames from 'classnames';
 import { css } from '@emotion/css';
-import Scrollbars from 'react-custom-scrollbars';
+import Scrollbars, { positionValues } from 'react-custom-scrollbars';
 import { useStyles2 } from '../../themes';
 import { GrafanaTheme2 } from '@grafana/data';
 
+export type ScrollbarPosition = positionValues;
+
 interface Props {
   className?: string;
   autoHide?: boolean;
@@ -15,7 +17,7 @@ interface Props {
   hideHorizontalTrack?: boolean;
   hideVerticalTrack?: boolean;
   scrollTop?: number;
-  setScrollTop?: (event: any) => void;
+  setScrollTop?: (position: ScrollbarPosition) => void;
   autoHeightMin?: number | string;
   updateAfterMountMs?: number;
 }
@@ -101,11 +103,15 @@ export const CustomScrollbar: FC<Props> = ({
     return <div {...passedProps} className="scrollbar-view" />;
   }, []);
 
+  const onScrollStop = useCallback(() => {
+    ref.current && setScrollTop && setScrollTop(ref.current.getValues());
+  }, [setScrollTop]);
+
   return (
     <Scrollbars
       ref={ref}
       className={classNames(styles.customScrollbar, className)}
-      onScroll={setScrollTop}
+      onScrollStop={onScrollStop}
       autoHeight={true}
       autoHide={autoHide}
       autoHideTimeout={autoHideTimeout}
diff --git a/packages/grafana-ui/src/components/index.ts b/packages/grafana-ui/src/components/index.ts
index 6ac0c481cbc..21214a8eb10 100644
--- a/packages/grafana-ui/src/components/index.ts
+++ b/packages/grafana-ui/src/components/index.ts
@@ -6,7 +6,7 @@ export { Tooltip, PopoverContent } from './Tooltip/Tooltip';
 export { PopoverController } from './Tooltip/PopoverController';
 export { Popover } from './Tooltip/Popover';
 export { Portal } from './Portal/Portal';
-export { CustomScrollbar } from './CustomScrollbar/CustomScrollbar';
+export { CustomScrollbar, ScrollbarPosition } from './CustomScrollbar/CustomScrollbar';
 export { TabbedContainer, TabConfig } from './TabbedContainer/TabbedContainer';
 
 export { ClipboardButton } from './ClipboardButton/ClipboardButton';
diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx
index cf71804f686..81afc13554b 100644
--- a/public/app/features/dashboard/containers/DashboardPage.tsx
+++ b/public/app/features/dashboard/containers/DashboardPage.tsx
@@ -1,11 +1,11 @@
 import $ from 'jquery';
-import React, { MouseEvent, PureComponent } from 'react';
+import React, { PureComponent } from 'react';
 import { css } from 'emotion';
 import { hot } from 'react-hot-loader';
 import { connect } from 'react-redux';
 import { locationService } from '@grafana/runtime';
 import { selectors } from '@grafana/e2e-selectors';
-import { CustomScrollbar, stylesFactory, Themeable2, withTheme2 } from '@grafana/ui';
+import { CustomScrollbar, ScrollbarPosition, stylesFactory, Themeable2, withTheme2 } from '@grafana/ui';
 
 import { createErrorNotification } from 'app/core/copy/appNotification';
 import { Branding } from 'app/core/components/Branding/Branding';
@@ -241,9 +241,8 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
     $('body').toggleClass('panel-in-fullscreen', isFullscreen);
   }
 
-  setScrollTop = (e: MouseEvent<HTMLElement>): void => {
-    const target = e.target as HTMLElement;
-    this.setState({ scrollTop: target.scrollTop, updateScrollTop: undefined });
+  setScrollTop = ({ scrollTop }: ScrollbarPosition): void => {
+    this.setState({ scrollTop, updateScrollTop: undefined });
   };
 
   onAddPanel = () => {
diff --git a/public/app/features/query/components/QueryGroup.tsx b/public/app/features/query/components/QueryGroup.tsx
index 8d3b7fb80c7..c919376e229 100644
--- a/public/app/features/query/components/QueryGroup.tsx
+++ b/public/app/features/query/components/QueryGroup.tsx
@@ -1,7 +1,16 @@
 // Libraries
 import React, { PureComponent } from 'react';
 // Components
-import { Button, CustomScrollbar, HorizontalGroup, Icon, Modal, stylesFactory, Tooltip } from '@grafana/ui';
+import {
+  Button,
+  CustomScrollbar,
+  HorizontalGroup,
+  Icon,
+  Modal,
+  ScrollbarPosition,
+  stylesFactory,
+  Tooltip,
+} from '@grafana/ui';
 import { getDataSourceSrv, DataSourcePicker } from '@grafana/runtime';
 import { QueryEditorRows } from './QueryEditorRows';
 // Services
@@ -275,9 +284,8 @@ export class QueryGroup extends PureComponent<Props, State> {
     this.onScrollBottom();
   };
 
-  setScrollTop = (event: React.MouseEvent<HTMLElement>) => {
-    const target = event.target as HTMLElement;
-    this.setState({ scrollTop: target.scrollTop });
+  setScrollTop = ({ scrollTop }: ScrollbarPosition) => {
+    this.setState({ scrollTop: scrollTop });
   };
 
   onQueriesChange = (queries: DataQuery[]) => {