DashboardScene: Support remember scroll position when coming back from view panel, panel edit and settings (#92185)

* DashboardScene: Support remember scroll position when coming back from view panel, panel edit and settings

* remove unused state prop

* Update

* Fixes

* Update e2e

e2e/scenes/dashboards-suite/general-dashboards.spec.ts

@@ -24,9 +24,7 @@ describe('Dashboards', () => {
     e2e.components.Panels.Panel.menuItems('Edit').click();
     e2e.components.NavToolbar.editDashboard.backToDashboardButton().click();
 
-    // And the last panel should still be visible!
-    // TODO: investigate scroll to on navigating back
-    // e2e.components.Panels.Panel.title('Panel #50').should('be.visible');
-    // e2e.components.Panels.Panel.title('Panel #1').should('be.visible');
+    // The last panel should still be visible!
+    e2e.components.Panels.Panel.title('Panel #50').should('be.visible');
   });
 });

public/app/core/components/NativeScrollbar.tsx

@@ -2,28 +2,35 @@ import { css, cx } from '@emotion/css';
 import { useEffect, useRef } from 'react';
 
 import { config } from '@grafana/runtime';
-import { CustomScrollbar, useStyles2 } from '@grafana/ui';
+import { useStyles2 } from '@grafana/ui';
 
-type Props = Parameters<typeof CustomScrollbar>[0];
+export interface Props {
+  children: React.ReactNode;
+  onSetScrollRef?: (ref: ScrollRefElement) => void;
+  divId?: string;
+}
+
+export interface ScrollRefElement {
+  scrollTop: number;
+  scrollTo: (x: number, y: number) => void;
+}
 
 // Shim to provide API-compatibility for Page's scroll-related props
 // when bodyScrolling is enabled, this is a no-op
 // TODO remove this shim completely when bodyScrolling is enabled
-export default function NativeScrollbar({ children, scrollRefCallback, scrollTop, divId }: Props) {
+export default function NativeScrollbar({ children, onSetScrollRef, divId }: Props) {
   const styles = useStyles2(getStyles);
   const ref = useRef<HTMLDivElement>(null);
 
   useEffect(() => {
-    if (!config.featureToggles.bodyScrolling && ref.current && scrollRefCallback) {
-      scrollRefCallback(ref.current);
+    if (config.featureToggles.bodyScrolling && onSetScrollRef) {
+      onSetScrollRef(new WindowScrollElement());
     }
-  }, [ref, scrollRefCallback]);
 
-  useEffect(() => {
-    if (!config.featureToggles.bodyScrolling && ref.current && scrollTop != null) {
-      ref.current?.scrollTo(0, scrollTop);
+    if (!config.featureToggles.bodyScrolling && ref.current && onSetScrollRef) {
+      onSetScrollRef(ref.current);
     }
-  }, [scrollTop]);
+  }, [ref, onSetScrollRef]);
 
   return config.featureToggles.bodyScrolling ? (
     children
@@ -35,6 +42,16 @@ export default function NativeScrollbar({ children, scrollRefCallback, scrollTop
   );
 }
 
+class WindowScrollElement {
+  public get scrollTop() {
+    return window.scrollY;
+  }
+
+  public scrollTo(x: number, y: number) {
+    window.scrollTo(x, y);
+  }
+}
+
 function getStyles() {
   return {
     nativeScrollbars: css({

public/app/core/components/Page/Page.tsx

@@ -27,8 +27,7 @@ export const Page: PageType = ({
   className,
   info,
   layout = PageLayoutType.Standard,
-  scrollTop,
-  scrollRef,
+  onSetScrollRef,
   ...otherProps
 }) => {
   const styles = useStyles2(getStyles);
@@ -57,9 +56,7 @@ export const Page: PageType = ({
         <NativeScrollbar
           // This id is used by the image renderer to scroll through the dashboard
           divId="page-scrollbar"
-          autoHeightMin={'100%'}
-          scrollTop={scrollTop}
-          scrollRefCallback={scrollRef}
+          onSetScrollRef={onSetScrollRef}
         >
           <div className={styles.pageInner}>
             {pageHeaderNav && (
@@ -82,9 +79,7 @@ export const Page: PageType = ({
         <NativeScrollbar
           // This id is used by the image renderer to scroll through the dashboard
           divId="page-scrollbar"
-          autoHeightMin={'100%'}
-          scrollTop={scrollTop}
-          scrollRefCallback={scrollRef}
+          onSetScrollRef={onSetScrollRef}
         >
           <div className={styles.canvasContent}>{children}</div>
         </NativeScrollbar>

public/app/core/components/Page/types.ts

@@ -1,8 +1,10 @@
-import { FC, HTMLAttributes, RefCallback } from 'react';
+import { FC, HTMLAttributes } from 'react';
 import * as React from 'react';
 
 import { NavModel, NavModelItem, PageLayoutType } from '@grafana/data';
 
+import { ScrollRefElement } from '../NativeScrollbar';
+
 import { PageContents } from './PageContents';
 
 export interface PageProps extends HTMLAttributes<HTMLDivElement> {
@@ -22,15 +24,11 @@ export interface PageProps extends HTMLAttributes<HTMLDivElement> {
   /** Control the page layout. */
   layout?: PageLayoutType;
   /**
+   * TODO: Not sure we should deprecated it given the sidecar project?
    * @deprecated this will be removed when bodyScrolling is enabled by default
    * Can be used to get the scroll container element to access scroll position
    * */
-  scrollRef?: RefCallback<HTMLDivElement>;
-  /**
-   * @deprecated this will be removed when bodyScrolling is enabled by default
-   * Can be used to update the current scroll position
-   * */
-  scrollTop?: number;
+  onSetScrollRef?: (ref: ScrollRefElement) => void;
 }
 
 export interface PageInfoItem {

public/app/features/dashboard-scene/scene/DashboardScene.tsx

@@ -27,6 +27,7 @@ import {
 } from '@grafana/scenes';
 import { Dashboard, DashboardLink, LibraryPanel } from '@grafana/schema';
 import appEvents from 'app/core/app_events';
+import { ScrollRefElement } from 'app/core/components/NativeScrollbar';
 import { LS_PANEL_COPY_KEY } from 'app/core/constants';
 import { getNavModel } from 'app/core/selectors/navModel';
 import store from 'app/core/store';
@@ -167,6 +168,11 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
    * A reference to the scopes facade
    */
   private _scopesFacade: ScopesFacade | null;
+  /**
+   * Remember scroll position when going into panel edit
+   */
+  private _scrollRef?: ScrollRefElement;
+  private _prevScrollPos?: number;
 
   public constructor(state: Partial<DashboardSceneState>) {
     super({
@@ -925,6 +931,22 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
       }
     });
   }
+
+  public onSetScrollRef = (scrollElement: ScrollRefElement): void => {
+    this._scrollRef = scrollElement;
+  };
+
+  public rememberScrollPos() {
+    this._prevScrollPos = this._scrollRef?.scrollTop;
+  }
+
+  public restoreScrollPos() {
+    if (this._prevScrollPos !== undefined) {
+      setTimeout(() => {
+        this._scrollRef?.scrollTo(0, this._prevScrollPos!);
+      }, 50);
+    }
+  }
 }
 
 export class DashboardVariableDependency implements SceneVariableDependencyConfigLike {

public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx

@@ -1,4 +1,5 @@
 import { css, cx } from '@emotion/css';
+import { useEffect, useMemo } from 'react';
 import { useLocation } from 'react-router-dom';
 
 import { GrafanaTheme2, PageLayoutType } from '@grafana/data';
@@ -16,7 +17,7 @@ import { DashboardScene } from './DashboardScene';
 import { NavToolbarActions } from './NavToolbarActions';
 
 export function DashboardSceneRenderer({ model }: SceneComponentProps<DashboardScene>) {
-  const { controls, overlay, editview, editPanel, isEmpty, meta } = model.useState();
+  const { controls, overlay, editview, editPanel, isEmpty, meta, viewPanelScene } = model.useState();
   const headerHeight = useChromeHeaderHeight();
   const styles = useStyles2(getStyles, headerHeight);
   const location = useLocation();
@@ -25,6 +26,21 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps<DashboardS
   const bodyToRender = model.getBodyToRender();
   const navModel = getNavModel(navIndex, 'dashboards/browse');
   const hasControls = controls?.hasControls();
+  const isSettingsOpen = editview !== undefined;
+
+  // Remember scroll pos when going into view panel, edit panel or settings
+  useMemo(() => {
+    if (viewPanelScene || isSettingsOpen || editPanel) {
+      model.rememberScrollPos();
+    }
+  }, [isSettingsOpen, editPanel, viewPanelScene, model]);
+
+  // Restore scroll pos when coming back
+  useEffect(() => {
+    if (!viewPanelScene && !isSettingsOpen && !editPanel) {
+      model.restoreScrollPos();
+    }
+  }, [isSettingsOpen, editPanel, viewPanelScene, model]);
 
   if (editview) {
     return (
@@ -54,12 +70,11 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps<DashboardS
   } else if (isEmpty) {
     body = [emptyState, withPanels];
   }
-
   return (
     <Page navModel={navModel} pageNav={pageNav} layout={PageLayoutType.Custom}>
       {editPanel && <editPanel.Component model={editPanel} />}
       {!editPanel && (
-        <NativeScrollbar divId="page-scrollbar" autoHeightMin={'100%'}>
+        <NativeScrollbar divId="page-scrollbar" onSetScrollRef={model.onSetScrollRef}>
           <div className={cx(styles.pageContainer, hasControls && styles.pageContainerWithControls)}>
             <NavToolbarActions dashboard={model} />
             {controls && (

public/app/features/dashboard/containers/DashboardPage.tsx

@@ -7,6 +7,7 @@ import { selectors } from '@grafana/e2e-selectors';
 import { config, locationService } from '@grafana/runtime';
 import { Themeable2, withTheme2 } from '@grafana/ui';
 import { notifyApp } from 'app/core/actions';
+import { ScrollRefElement } from 'app/core/components/NativeScrollbar';
 import { Page } from 'app/core/components/Page/Page';
 import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound';
 import { GrafanaContext, GrafanaContextType } from 'app/core/context/GrafanaContext';
@@ -77,7 +78,7 @@ export interface State {
   showLoadingState: boolean;
   panelNotFound: boolean;
   editPanelAccessDenied: boolean;
-  scrollElement?: HTMLDivElement;
+  scrollElement?: ScrollRefElement;
   pageNav?: NavModelItem;
   sectionNav?: NavModel;
 }
@@ -234,11 +235,9 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
       locationService.partial({ editPanel: null, viewPanel: null });
     }
 
-    if (config.featureToggles.bodyScrolling) {
-      // Update window scroll position
-      if (this.state.updateScrollTop !== undefined && this.state.updateScrollTop !== prevState.updateScrollTop) {
-        window.scrollTo(0, this.state.updateScrollTop);
-      }
+    // Update window scroll position
+    if (this.state.updateScrollTop !== undefined && this.state.updateScrollTop !== prevState.updateScrollTop) {
+      this.state.scrollElement?.scrollTo(0, this.state.updateScrollTop);
     }
   }
 
@@ -263,19 +262,17 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
 
     const updatedState = { ...state };
 
-    if (config.featureToggles.bodyScrolling) {
-      // Entering settings view
-      if (!state.editView && urlEditView) {
-        updatedState.editView = urlEditView;
-        updatedState.rememberScrollTop = window.scrollY;
-        updatedState.updateScrollTop = 0;
-      }
+    // Entering settings view
+    if (!state.editView && urlEditView) {
+      updatedState.editView = urlEditView;
+      updatedState.rememberScrollTop = state.scrollElement?.scrollTop;
+      updatedState.updateScrollTop = 0;
+    }
 
-      // Leaving settings view
-      else if (state.editView && !urlEditView) {
-        updatedState.updateScrollTop = state.rememberScrollTop;
-        updatedState.editView = null;
-      }
+    // Leaving settings view
+    else if (state.editView && !urlEditView) {
+      updatedState.updateScrollTop = state.rememberScrollTop;
+      updatedState.editView = null;
     }
 
     // Entering edit mode
@@ -284,12 +281,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
       if (panel) {
         if (dashboard.canEditPanel(panel)) {
           updatedState.editPanel = panel;
-          updatedState.rememberScrollTop = config.featureToggles.bodyScrolling
-            ? window.scrollY
-            : state.scrollElement?.scrollTop;
-          if (config.featureToggles.bodyScrolling) {
-            updatedState.updateScrollTop = 0;
-          }
+          updatedState.rememberScrollTop = state.scrollElement?.scrollTop;
         } else {
           updatedState.editPanelAccessDenied = true;
         }
@@ -311,9 +303,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
         // Should move this state out of dashboard in the future
         dashboard.initViewPanel(panel);
         updatedState.viewPanel = panel;
-        updatedState.rememberScrollTop = config.featureToggles.bodyScrolling
-          ? window.scrollY
-          : state.scrollElement?.scrollTop;
+        updatedState.rememberScrollTop = state.scrollElement?.scrollTop;
         updatedState.updateScrollTop = 0;
       } else {
         updatedState.panelNotFound = true;
@@ -337,7 +327,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
     return updateStatePageNavFromProps(props, updatedState);
   }
 
-  setScrollRef = (scrollElement: HTMLDivElement): void => {
+  setScrollRef = (scrollElement: ScrollRefElement): void => {
     this.setState({ scrollElement });
   };
 
@@ -366,7 +356,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
 
   render() {
     const { dashboard, initError, queryParams, theme } = this.props;
-    const { editPanel, viewPanel, updateScrollTop, pageNav, sectionNav } = this.state;
+    const { editPanel, viewPanel, pageNav, sectionNav } = this.state;
     const kioskMode = getKioskMode(this.props.queryParams);
     const styles = getStyles(theme);
 
@@ -443,8 +433,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
           pageNav={pageNav}
           layout={PageLayoutType.Canvas}
           className={pageClassName}
-          scrollRef={this.setScrollRef}
-          scrollTop={updateScrollTop}
+          onSetScrollRef={this.setScrollRef}
         >
           {showToolbar && (
             <header data-testid={selectors.pages.Dashboard.DashNav.navV2}>