From 43dba8c3f9f590e2f968dd28587972f014a7d92d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 19 Aug 2024 15:42:45 +0200 Subject: [PATCH] DashboardScene: Support bodyScrolling (#91888) * Progress * Fix dashboards pane * almost working * add hook to get scopesDashboardsScene state * check whether it's enabled when considering opened state * add height to container * Update * revert change * Make it work when bodyScrolling is disabled * Last tweaks * Update scenes * Updating * Fix * fix tests * fix lint issues * fix lint issues --------- Co-authored-by: Ashley Harrison --- .../general-dashboards.spec.ts | 9 +- .../general-dashboards.spec.ts | 11 +- package.json | 2 +- .../pages/DashboardScenePage.test.tsx | 1 + .../scene/DashboardControls.tsx | 15 +-- .../scene/DashboardSceneRenderer.test.tsx | 27 ++--- .../scene/DashboardSceneRenderer.tsx | 106 ++++++------------ public/app/features/scopes/scopes.test.tsx | 18 ++- yarn.lock | 10 +- 9 files changed, 77 insertions(+), 122 deletions(-) diff --git a/e2e/dashboards-suite/general-dashboards.spec.ts b/e2e/dashboards-suite/general-dashboards.spec.ts index 00584030c23..f450f698e98 100644 --- a/e2e/dashboards-suite/general-dashboards.spec.ts +++ b/e2e/dashboards-suite/general-dashboards.spec.ts @@ -12,12 +12,9 @@ describe('Dashboards', () => { e2e.components.Panels.Panel.title('Panel #1').should('be.visible'); // scroll to the bottom - e2e.pages.Dashboard.DashNav.navV2() - .parent() - .parent() // Note, this will probably fail when we change the custom scrollbars - .scrollTo('bottom', { - timeout: 5 * 1000, - }); + cy.get('#page-scrollbar').scrollTo('bottom', { + timeout: 5 * 1000, + }); // The last panel should be visible... e2e.components.Panels.Panel.title('Panel #50').should('be.visible'); diff --git a/e2e/scenes/dashboards-suite/general-dashboards.spec.ts b/e2e/scenes/dashboards-suite/general-dashboards.spec.ts index 8270a7b1e5e..1edd34482b6 100644 --- a/e2e/scenes/dashboards-suite/general-dashboards.spec.ts +++ b/e2e/scenes/dashboards-suite/general-dashboards.spec.ts @@ -12,12 +12,9 @@ describe('Dashboards', () => { e2e.components.Panels.Panel.title('Panel #1').should('be.visible'); // scroll to the bottom - e2e.pages.Dashboard.DashNav.scrollContainer() - .children() - .first() - .scrollTo('bottom', { - timeout: 5 * 1000, - }); + cy.get('#page-scrollbar').scrollTo('bottom', { + timeout: 5 * 1000, + }); // The last panel should be visible... e2e.components.Panels.Panel.title('Panel #50').should('be.visible'); @@ -30,6 +27,6 @@ describe('Dashboards', () => { // 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'); + // e2e.components.Panels.Panel.title('Panel #1').should('be.visible'); }); }); diff --git a/package.json b/package.json index d5c506ec6b0..457f1b37e4c 100644 --- a/package.json +++ b/package.json @@ -266,7 +266,7 @@ "@grafana/prometheus": "workspace:*", "@grafana/runtime": "workspace:*", "@grafana/saga-icons": "workspace:*", - "@grafana/scenes": "5.7.4", + "@grafana/scenes": "^5.8.0", "@grafana/schema": "workspace:*", "@grafana/sql": "workspace:*", "@grafana/ui": "workspace:*", diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx b/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx index 0aebf3fe3d2..e7524d1ceeb 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx +++ b/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx @@ -25,6 +25,7 @@ jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), setPluginExtensionGetter: jest.fn(), getPluginLinkExtensions: jest.fn(), + useChromeHeaderHeight: jest.fn().mockReturnValue(80), getBackendSrv: () => { return { get: jest.fn().mockResolvedValue({ dashboard: simpleDashboard, meta: { url: '' } }), diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.tsx index 3e992340079..5d83e8a244e 100644 --- a/public/app/features/dashboard-scene/scene/DashboardControls.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardControls.tsx @@ -1,4 +1,4 @@ -import { css, cx } from '@emotion/css'; +import { css } from '@emotion/css'; import { GrafanaTheme2, VariableHide } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; @@ -119,7 +119,7 @@ function DashboardControlsRenderer({ model }: SceneComponentProps +
{!hideVariableControls && variableControls.map((c) => )} @@ -159,18 +156,12 @@ function getStyles(theme: GrafanaTheme2) { flexDirection: 'row', flexWrap: 'nowrap', position: 'relative', - background: theme.colors.background.canvas, - zIndex: theme.zIndex.activePanel, width: '100%', marginLeft: 'auto', [theme.breakpoints.down('sm')]: { flexDirection: 'column-reverse', alignItems: 'stretch', }, - [theme.breakpoints.up('sm')]: { - position: 'sticky', - top: 0, - }, }), embedded: css({ background: 'unset', diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx index c8ee835f62b..881883dd441 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx @@ -1,15 +1,15 @@ -import { render, screen } from '@testing-library/react'; -import { Provider } from 'react-redux'; -import { Router } from 'react-router'; -import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; +import { screen } from '@testing-library/react'; +import { render } from 'test/test-utils'; import { selectors } from '@grafana/e2e-selectors'; -import { locationService } from '@grafana/runtime'; -import { GrafanaContext } from 'app/core/context/GrafanaContext'; -import { configureStore } from 'app/store/configureStore'; import { transformSaveModelToScene } from '../serialization/transformSaveModelToScene'; +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + useChromeHeaderHeight: jest.fn(), +})); + describe('DashboardSceneRenderer', () => { it('should render Not Found notice when dashboard is not found', async () => { const scene = transformSaveModelToScene({ @@ -46,18 +46,7 @@ describe('DashboardSceneRenderer', () => { }, }); - const store = configureStore({}); - const context = getGrafanaContextMock(); - - render( - - - - - - - - ); + render(); expect(await screen.findByTestId(selectors.components.EntityNotFound.container)).toBeInTheDocument(); }); diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx index 149c884462f..215d83f4dcc 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx @@ -1,12 +1,11 @@ import { css, cx } from '@emotion/css'; import { useLocation } from 'react-router-dom'; -import { useMedia } from 'react-use'; import { GrafanaTheme2, PageLayoutType } from '@grafana/data'; -import { selectors } from '@grafana/e2e-selectors'; -import { config } from '@grafana/runtime'; +import { useChromeHeaderHeight } from '@grafana/runtime'; import { SceneComponentProps } from '@grafana/scenes'; -import { CustomScrollbar, useStyles2, useTheme2 } from '@grafana/ui'; +import { useStyles2 } from '@grafana/ui'; +import NativeScrollbar from 'app/core/components/NativeScrollbar'; import { Page } from 'app/core/components/Page/Page'; import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; import { getNavModel } from 'app/core/selectors/navModel'; @@ -18,7 +17,8 @@ import { NavToolbarActions } from './NavToolbarActions'; export function DashboardSceneRenderer({ model }: SceneComponentProps) { const { controls, overlay, editview, editPanel, isEmpty, meta } = model.useState(); - const styles = useStyles2(getStyles); + const headerHeight = useChromeHeaderHeight(); + const styles = useStyles2(getStyles, headerHeight); const location = useLocation(); const navIndex = useSelector((state) => state.navIndex); const pageNav = model.getPageNav(location, navIndex); @@ -59,57 +59,43 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps {editPanel && } {!editPanel && ( -
- - {controls && ( -
- -
- )} - + +
+ + {controls && ( +
+ +
+ )}
{body}
- -
+
+ )} {overlay && } ); } -function getStyles(theme: GrafanaTheme2) { +function getStyles(theme: GrafanaTheme2, headerHeight: number | undefined) { return { - pageContainer: css( - { - display: 'grid', - gridTemplateAreas: ` + pageContainer: css({ + display: 'grid', + gridTemplateAreas: ` "panels"`, - gridTemplateColumns: `1fr`, - gridTemplateRows: '1fr', - height: '100%', - [theme.breakpoints.down('sm')]: { - display: 'flex', - flexDirection: 'column', - }, + gridTemplateColumns: `1fr`, + gridTemplateRows: '1fr', + flexGrow: 1, + [theme.breakpoints.down('sm')]: { + display: 'flex', + flexDirection: 'column', }, - config.featureToggles.bodyScrolling && { - position: 'absolute', - width: '100%', - } - ), + }), pageContainerWithControls: css({ gridTemplateAreas: ` "controls" "panels"`, gridTemplateRows: 'auto 1fr', }), - panelsContainer: css({ - gridArea: 'panels', - }), controlsWrapper: css({ display: 'flex', flexDirection: 'column', @@ -119,6 +105,13 @@ function getStyles(theme: GrafanaTheme2) { ':empty': { display: 'none', }, + // Make controls sticky on larger screens (> mobile) + [theme.breakpoints.up('md')]: { + position: 'sticky', + zIndex: theme.zIndex.activePanel, + background: theme.colors.background.canvas, + top: headerHeight, + }, }), canvasContent: css({ label: 'canvas-content', @@ -126,7 +119,9 @@ function getStyles(theme: GrafanaTheme2) { flexDirection: 'column', padding: theme.spacing(0, 2), flexBasis: '100%', + gridArea: 'panels', flexGrow: 1, + minWidth: 0, }), body: css({ label: 'body', @@ -141,34 +136,3 @@ function getStyles(theme: GrafanaTheme2) { }), }; } - -interface PanelsContainerProps { - id: string; - children: React.ReactNode; - className?: string; - testId?: string; -} -/** - * Removes the scrollbar on mobile and uses a custom scrollbar on desktop - */ -const PanelsContainer = ({ id, children, className, testId }: PanelsContainerProps) => { - const theme = useTheme2(); - const isMobile = useMedia(`(max-width: ${theme.breakpoints.values.sm}px)`); - const styles = useStyles2(() => ({ - nonScrollable: css({ - height: '100%', - display: 'flex', - flexDirection: 'column', - }), - })); - - return isMobile ? ( -
- {children} -
- ) : ( - - {children} - - ); -}; diff --git a/public/app/features/scopes/scopes.test.tsx b/public/app/features/scopes/scopes.test.tsx index 1b8d9116609..50e014c53a6 100644 --- a/public/app/features/scopes/scopes.test.tsx +++ b/public/app/features/scopes/scopes.test.tsx @@ -1,7 +1,8 @@ import { act, cleanup, waitFor } from '@testing-library/react'; import userEvents from '@testing-library/user-event'; -import { config, locationService } from '@grafana/runtime'; +import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks'; +import { config, locationService, setPluginImportUtils } from '@grafana/runtime'; import { sceneGraph } from '@grafana/scenes'; import { getDashboardAPI, setDashboardAPI } from 'app/features/dashboard/api/dashboard_api'; import { DashboardScene } from 'app/features/dashboard-scene/scene/DashboardScene'; @@ -62,16 +63,30 @@ import { getClosestScopesFacade } from './utils'; jest.mock('@grafana/runtime', () => ({ __esModule: true, ...jest.requireActual('@grafana/runtime'), + useChromeHeaderHeight: jest.fn(), getBackendSrv: () => ({ get: getMock, }), usePluginLinkExtensions: jest.fn().mockReturnValue({ extensions: [] }), })); +const panelPlugin = getPanelPlugin({ + id: 'table', + skipDataQuery: true, +}); + +config.panels['table'] = panelPlugin.meta; + +setPluginImportUtils({ + importPanelPlugin: (id: string) => Promise.resolve(panelPlugin), + getPanelPluginFromCache: (id: string) => undefined, +}); + describe('Scopes', () => { describe('Feature flag off', () => { beforeAll(() => { config.featureToggles.scopeFilters = false; + config.featureToggles.groupByVariable = true; initializeScopes(); }); @@ -88,6 +103,7 @@ describe('Scopes', () => { beforeAll(() => { config.featureToggles.scopeFilters = true; + config.featureToggles.groupByVariable = true; }); beforeEach(() => { diff --git a/yarn.lock b/yarn.lock index 75e03fb2f98..de44a6f730a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3739,9 +3739,9 @@ __metadata: languageName: unknown linkType: soft -"@grafana/scenes@npm:5.7.4": - version: 5.7.4 - resolution: "@grafana/scenes@npm:5.7.4" +"@grafana/scenes@npm:^5.8.0": + version: 5.8.0 + resolution: "@grafana/scenes@npm:5.8.0" dependencies: "@grafana/e2e-selectors": "npm:^11.0.0" "@leeoniya/ufuzzy": "npm:^1.0.14" @@ -3756,7 +3756,7 @@ __metadata: "@grafana/ui": ">=10.4" react: ^18.0.0 react-dom: ^18.0.0 - checksum: 10/e78f0d215e8a7a591689ce0b4672732bbf22ab38964e908cbf328125c855bcbf9569945dc2731a114e1a7244d2b7c7da98a1df2ac1ec4883c8201e0ab68de75f + checksum: 10/1c550dd5256371de0849ae64d167c4a9dbd99be0c03f15116ac213d3a9e2ec4b5248331086ca9d6616b5ad8f2be5be503772ac38a853e32da57f485ef3addb41 languageName: node linkType: hard @@ -18163,7 +18163,7 @@ __metadata: "@grafana/prometheus": "workspace:*" "@grafana/runtime": "workspace:*" "@grafana/saga-icons": "workspace:*" - "@grafana/scenes": "npm:5.7.4" + "@grafana/scenes": "npm:^5.8.0" "@grafana/schema": "workspace:*" "@grafana/sql": "workspace:*" "@grafana/tsconfig": "npm:^2.0.0"