diff --git a/e2e/scenes/dashboards-suite/general-dashboards.spec.ts b/e2e/scenes/dashboards-suite/general-dashboards.spec.ts index 1edd34482b6..6291c787320 100644 --- a/e2e/scenes/dashboards-suite/general-dashboards.spec.ts +++ b/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'); }); }); diff --git a/public/app/core/components/NativeScrollbar.tsx b/public/app/core/components/NativeScrollbar.tsx index d0eb36adab7..9116f47b055 100644 --- a/public/app/core/components/NativeScrollbar.tsx +++ b/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[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(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({ diff --git a/public/app/core/components/Page/Page.tsx b/public/app/core/components/Page/Page.tsx index 9056a7dfca5..0f82fd4ba8c 100644 --- a/public/app/core/components/Page/Page.tsx +++ b/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 = ({
{pageHeaderNav && ( @@ -82,9 +79,7 @@ export const Page: PageType = ({
{children}
diff --git a/public/app/core/components/Page/types.ts b/public/app/core/components/Page/types.ts index f1df40ab280..cc83c18aade 100644 --- a/public/app/core/components/Page/types.ts +++ b/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 { @@ -22,15 +24,11 @@ export interface PageProps extends HTMLAttributes { /** 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; - /** - * @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 { diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index 8a63083655a..5274e75c459 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/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 { * 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) { super({ @@ -925,6 +931,22 @@ export class DashboardScene extends SceneObjectBase { } }); } + + 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 { diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx index 215d83f4dcc..39e7b1f47dc 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx +++ b/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) { - 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 { + 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 {editPanel && } {!editPanel && ( - +
{controls && ( diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index e4f56d47b29..ecc58a535a6 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/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 { 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 { 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 { 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 { // 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 { return updateStatePageNavFromProps(props, updatedState); } - setScrollRef = (scrollElement: HTMLDivElement): void => { + setScrollRef = (scrollElement: ScrollRefElement): void => { this.setState({ scrollElement }); }; @@ -366,7 +356,7 @@ export class UnthemedDashboardPage extends PureComponent { 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 { pageNav={pageNav} layout={PageLayoutType.Canvas} className={pageClassName} - scrollRef={this.setScrollRef} - scrollTop={updateScrollTop} + onSetScrollRef={this.setScrollRef} > {showToolbar && (