From a67eaf6b6276537ad55b34e2aaaa854981143172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 7 Jul 2021 18:39:45 +0200 Subject: [PATCH] DashboardPage: Refactor state to fix state timing bugs and reduce unnecessary re-renders (#36460) * DashboardPage: Refactoring state handling to improve performance and fix bugs with state out of sync * Fixed exit panel editor timing issues * New tests in RTL * Updated comment * Removed unused imports --- .../DashboardPrompt/DashboardPrompt.tsx | 7 +- .../components/PanelEditor/PanelEditor.tsx | 5 +- .../PanelEditor/state/actions.test.ts | 8 +- .../components/PanelEditor/state/actions.ts | 13 +- .../containers/DashboardPage.test.tsx | 321 +++-- .../dashboard/containers/DashboardPage.tsx | 153 +-- .../dashboard/containers/SoloPanelPage.tsx | 7 +- .../__snapshots__/DashboardPage.test.tsx.snap | 1075 ----------------- .../dashboard/dashgrid/DashboardGrid.tsx | 1 - .../features/dashboard/services/TimeSrv.ts | 30 +- .../dashboard/state/DashboardModel.ts | 9 +- public/sass/_grafana.scss | 1 - public/sass/components/_panel_editor.scss | 18 - 13 files changed, 261 insertions(+), 1387 deletions(-) delete mode 100644 public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap delete mode 100644 public/sass/components/_panel_editor.scss diff --git a/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx index 849de4be7eb..05e04573036 100644 --- a/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx +++ b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx @@ -11,7 +11,7 @@ import * as H from 'history'; import { SaveLibraryPanelModal } from 'app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal'; import { PanelModelWithLibraryPanel } from 'app/features/library-panels/types'; import { useDispatch } from 'react-redux'; -import { discardPanelChanges } from '../PanelEditor/state/actions'; +import { discardPanelChanges, exitPanelEditor } from '../PanelEditor/state/actions'; import { DashboardSavedEvent } from 'app/types/events'; export interface Props { @@ -77,6 +77,11 @@ export const DashboardPrompt = React.memo(({ dashboard }: Props) => { // Are we still on the same dashboard? if (originalPath === location.pathname || !original) { + // This is here due to timing reasons we want the exit panel editor state changes to happen before router update + if (panelInEdit && !search.has('editPanel')) { + dispatch(exitPanelEditor()); + } + return true; } diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx index c1fcc9931ca..7ea794d65bd 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx @@ -28,7 +28,7 @@ import { SplitPaneWrapper } from 'app/core/components/SplitPaneWrapper/SplitPane import { SaveDashboardModalProxy } from '../SaveDashboard/SaveDashboardModalProxy'; import { DashboardPanel } from '../../dashgrid/DashboardPanel'; -import { discardPanelChanges, initPanelEditor, panelEditorCleanUp, updatePanelEditorUIState } from './state/actions'; +import { discardPanelChanges, initPanelEditor, updatePanelEditorUIState } from './state/actions'; import { updateTimeZoneForSession } from 'app/features/profile/state/reducers'; import { toggleTableView } from './state/reducers'; @@ -78,7 +78,6 @@ const mapStateToProps = (state: StoreState) => { const mapDispatchToProps = { initPanelEditor, - panelEditorCleanUp, discardPanelChanges, updatePanelEditorUIState, updateTimeZoneForSession, @@ -115,7 +114,7 @@ export class PanelEditorUnconnected extends PureComponent { } componentWillUnmount() { - this.props.panelEditorCleanUp(); + // redux action exitPanelEditor is called on location change from DashboardPrompt this.eventSubs?.unsubscribe(); } diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts index 5b4b2fd56ed..94c8279c301 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts @@ -1,6 +1,6 @@ import { thunkTester } from '../../../../../../test/core/thunk/thunkTester'; import { closeCompleted, initialState, PanelEditorState } from './reducers'; -import { initPanelEditor, panelEditorCleanUp } from './actions'; +import { initPanelEditor, exitPanelEditor } from './actions'; import { cleanUpEditPanel, panelModelAndPluginReady } from '../../../state/reducers'; import { DashboardModel, PanelModel } from '../../../state'; import { getPanelPlugin } from 'app/features/plugins/__mocks__/pluginMocks'; @@ -48,7 +48,7 @@ describe('panelEditor actions', () => { getModel: () => dashboard, }, }) - .givenThunk(panelEditorCleanUp) + .givenThunk(exitPanelEditor) .whenThunkIsDispatched(); expect(dispatchedActions.length).toBe(2); @@ -83,7 +83,7 @@ describe('panelEditor actions', () => { getModel: () => dashboard, }, }) - .givenThunk(panelEditorCleanUp) + .givenThunk(exitPanelEditor) .whenThunkIsDispatched(); expect(dispatchedActions.length).toBe(3); @@ -118,7 +118,7 @@ describe('panelEditor actions', () => { getModel: () => dashboard, }, }) - .givenThunk(panelEditorCleanUp) + .givenThunk(exitPanelEditor) .whenThunkIsDispatched(); expect(dispatchedActions.length).toBe(2); diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.ts index 08bf2eaf49d..a815337e1db 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.ts @@ -11,7 +11,6 @@ import { import { cleanUpEditPanel, panelModelAndPluginReady } from '../../../state/reducers'; import store from 'app/core/store'; import { pick } from 'lodash'; -import { locationService } from '@grafana/runtime'; export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardModel): ThunkResult { return (dispatch) => { @@ -34,12 +33,6 @@ export function discardPanelChanges(): ThunkResult { }; } -export function exitPanelEditor(): ThunkResult { - return async (dispatch, getStore) => { - locationService.partial({ editPanel: null, tab: null }); - }; -} - function updateDuplicateLibraryPanels(modifiedPanel: PanelModel, dashboard: DashboardModel | null, dispatch: any) { if (modifiedPanel.libraryPanel?.uid === undefined || !dashboard) { return; @@ -74,8 +67,8 @@ function updateDuplicateLibraryPanels(modifiedPanel: PanelModel, dashboard: Dash } } -export function panelEditorCleanUp(): ThunkResult { - return (dispatch, getStore) => { +export function exitPanelEditor(): ThunkResult { + return async (dispatch, getStore) => { const dashboard = getStore().dashboard.getModel(); const { getPanel, getSourcePanel, shouldDiscardChanges } = getStore().panelEditor; @@ -98,7 +91,7 @@ export function panelEditorCleanUp(): ThunkResult { sourcePanel.plugin = panel.plugin; if (panelTypeChanged) { - dispatch(panelModelAndPluginReady({ panelId: sourcePanel.id, plugin: panel.plugin! })); + await dispatch(panelModelAndPluginReady({ panelId: sourcePanel.id, plugin: panel.plugin! })); } // Resend last query result on source panel query runner diff --git a/public/app/features/dashboard/containers/DashboardPage.test.tsx b/public/app/features/dashboard/containers/DashboardPage.test.tsx index cebc986639a..4be727e8075 100644 --- a/public/app/features/dashboard/containers/DashboardPage.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.test.tsx @@ -1,23 +1,43 @@ import React from 'react'; -import { shallow, ShallowWrapper } from 'enzyme'; -import { UnthemedDashboardPage, mapStateToProps, Props, State } from './DashboardPage'; +import { Provider } from 'react-redux'; +import { render, screen } from '@testing-library/react'; +import { UnthemedDashboardPage, Props } from './DashboardPage'; +import { Router } from 'react-router-dom'; +import { locationService } from '@grafana/runtime'; import { DashboardModel } from '../state'; +import { configureStore } from '../../../store/configureStore'; import { mockToolkitActionCreator } from 'test/core/redux/mocks'; import { DashboardInitPhase, DashboardRoutes } from 'app/types'; import { notifyApp } from 'app/core/actions'; -import { cleanUpDashboardAndVariables } from '../state/actions'; import { selectors } from '@grafana/e2e-selectors'; import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps'; import { createTheme } from '@grafana/data'; -jest.mock('app/features/dashboard/components/DashboardSettings/GeneralSettings', () => ({})); +jest.mock('app/features/dashboard/components/DashboardSettings/GeneralSettings', () => { + class GeneralSettings extends React.Component<{}, {}> { + render() { + return <>general settings; + } + } + + return { GeneralSettings }; +}); + +jest.mock('app/core/core', () => ({ + appEvents: { + subscribe: () => { + return { unsubscribe: () => {} }; + }, + }, +})); interface ScenarioContext { - cleanUpDashboardAndVariablesMock: typeof cleanUpDashboardAndVariables; dashboard?: DashboardModel | null; - setDashboardProp: (overrides?: any, metaOverrides?: any) => void; - wrapper?: ShallowWrapper; + container?: HTMLElement; mount: (propOverrides?: Partial) => void; + unmount: () => void; + props: Props; + rerender: (propOverrides?: Partial) => void; setup: (fn: () => void) => void; } @@ -28,8 +48,8 @@ function getTestDashboard(overrides?: any, metaOverrides?: any): DashboardModel panels: [ { id: 1, - type: 'graph', - title: 'My graph', + type: 'timeseries', + title: 'My panel title', gridPos: { x: 0, y: 0, w: 1, h: 1 }, }, ], @@ -46,15 +66,11 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo let setupFn: () => void; const ctx: ScenarioContext = { - cleanUpDashboardAndVariablesMock: jest.fn(), setup: (fn) => { setupFn = fn; }, - setDashboardProp: (overrides?: any, metaOverrides?: any) => { - ctx.dashboard = getTestDashboard(overrides, metaOverrides); - ctx.wrapper?.setProps({ dashboard: ctx.dashboard }); - }, mount: (propOverrides?: Partial) => { + const store = configureStore(); const props: Props = { ...getRouteComponentProps({ match: { params: { slug: 'my-dash', uid: '11' } } as any, @@ -64,7 +80,7 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo isInitSlow: false, initDashboard: jest.fn(), notifyApp: mockToolkitActionCreator(notifyApp), - cleanUpDashboardAndVariables: ctx.cleanUpDashboardAndVariablesMock, + cleanUpDashboardAndVariables: jest.fn(), cancelVariables: jest.fn(), templateVarsChangedInUrl: jest.fn(), dashboard: null, @@ -73,9 +89,36 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo Object.assign(props, propOverrides); + ctx.props = props; ctx.dashboard = props.dashboard; - ctx.wrapper = shallow(); + + const { container, rerender, unmount } = render( + + + + + + ); + + ctx.container = container; + + ctx.rerender = (newProps?: Partial) => { + Object.assign(props, newProps); + + rerender( + + + + + + ); + }; + + ctx.unmount = unmount; }, + props: {} as Props, + rerender: () => {}, + unmount: () => {}, }; beforeEach(() => { @@ -92,226 +135,140 @@ describe('DashboardPage', () => { ctx.mount(); }); - it('Should render nothing', () => { - expect(ctx.wrapper).toMatchSnapshot(); - }); - }); - - dashboardPageScenario('Dashboard is fetching slowly', (ctx) => { - ctx.setup(() => { - ctx.mount(); - ctx.wrapper?.setProps({ - isInitSlow: true, - initPhase: DashboardInitPhase.Fetching, + it('Should call initDashboard on mount', () => { + expect(ctx.props.initDashboard).toBeCalledWith({ + fixUrl: true, + routeName: 'normal-dashboard', + urlSlug: 'my-dash', + urlUid: '11', }); - }); - - it('Should render slow init state', () => { - expect(ctx.wrapper).toMatchSnapshot(); + expect(ctx.container).toBeEmptyDOMElement(); }); }); - dashboardPageScenario('Dashboard init completed ', (ctx) => { + dashboardPageScenario('Given dashboard slow loading state', (ctx) => { ctx.setup(() => { ctx.mount(); - ctx.setDashboardProp(); + ctx.rerender({ isInitSlow: true }); + }); + + it('Should show spinner', () => { + expect(screen.getByText('Cancel loading dashboard')).toBeInTheDocument(); + }); + }); + + dashboardPageScenario('Given a simple dashboard', (ctx) => { + ctx.setup(() => { + ctx.mount(); + ctx.rerender({ dashboard: getTestDashboard() }); + }); + + it('Should render panels', () => { + expect(screen.getByText('My panel title')).toBeInTheDocument(); }); it('Should update title', () => { expect(document.title).toBe('My dashboard - Grafana'); }); + }); - it('Should render dashboard grid', () => { - expect(ctx.wrapper).toMatchSnapshot(); + dashboardPageScenario('When going into view mode', (ctx) => { + ctx.setup(() => { + ctx.mount({ + dashboard: getTestDashboard(), + queryParams: { viewPanel: '1' }, + }); + }); + + it('Should render panel in view mode', () => { + expect(ctx.dashboard?.panelInView).toBeDefined(); + expect(ctx.dashboard?.panels[0].isViewing).toBe(true); + }); + + it('Should reset state when leaving', () => { + ctx.rerender({ queryParams: {} }); + + expect(ctx.dashboard?.panelInView).toBeUndefined(); + expect(ctx.dashboard?.panels[0].isViewing).toBe(false); }); }); - dashboardPageScenario('When user goes into panel edit', (ctx) => { + dashboardPageScenario('When going into edit mode', (ctx) => { ctx.setup(() => { - ctx.mount(); - ctx.setDashboardProp(); - ctx.wrapper?.setProps({ + ctx.mount({ + dashboard: getTestDashboard(), queryParams: { editPanel: '1' }, }); }); - it('Should update component state to fullscreen and edit', () => { - const state = ctx.wrapper?.state(); - expect(state).not.toBe(null); - expect(state?.editPanel).toBeDefined(); - }); - }); - - dashboardPageScenario('When user goes into panel edit but has no edit permissions', (ctx) => { - ctx.setup(() => { - ctx.mount(); - ctx.setDashboardProp({}, { canEdit: false }); - ctx.wrapper?.setProps({ - queryParams: { editPanel: '1' }, - }); + it('Should render panel in edit mode', () => { + expect(ctx.dashboard?.panelInEdit).toBeDefined(); }); - it('Should update component state to fullscreen and edit', () => { - const state = ctx.wrapper?.state(); - expect(state?.editPanel).toBe(null); - }); - }); - dashboardPageScenario('When user goes back to dashboard from edit panel', (ctx) => { - ctx.setup(() => { - ctx.mount(); - ctx.setDashboardProp(); - ctx.wrapper?.setState({ scrollTop: 100 }); - ctx.wrapper?.setProps({ - queryParams: { editPanel: '1' }, - }); - ctx.wrapper?.setProps({ - queryParams: {}, - }); + it('Should render panel editor', () => { + expect(screen.getByTitle('Apply changes and go back to dashboard')).toBeInTheDocument(); }); - it('Should update model state normal state', () => { - expect(ctx.dashboard).toBeDefined(); - // @ts-ignore typescript doesn't understand that dashboard must be defined to reach the row below - expect(ctx.dashboard.panelInEdit).toBeUndefined(); - }); - - it('Should update component state to normal and restore scrollTop', () => { - const state = ctx.wrapper?.state(); - expect(ctx.wrapper).not.toBe(null); - expect(state).not.toBe(null); - expect(state?.editPanel).toBe(null); - expect(state?.scrollTop).toBe(100); - }); - }); - - dashboardPageScenario('When dashboard has editview url state', (ctx) => { - ctx.setup(() => { - ctx.mount(); - ctx.setDashboardProp(); - ctx.wrapper?.setProps({ - queryParams: { editview: 'settings' }, - }); - }); - - it('should render settings view', () => { - expect(ctx.wrapper).toMatchSnapshot(); - }); - }); - - dashboardPageScenario('When adding panel', (ctx) => { - ctx.setup(() => { - ctx.mount(); - ctx.setDashboardProp(); - ctx.wrapper?.setState({ scrollTop: 100 }); - ctx.wrapper?.instance().onAddPanel(); - }); - - it('should set scrollTop to 0', () => { - expect(ctx.wrapper).not.toBe(null); - expect(ctx.wrapper?.state()).not.toBe(null); - expect(ctx.wrapper?.state().updateScrollTop).toBe(0); - }); - - it('should add panel widget to dashboard panels', () => { - expect(ctx.dashboard).not.toBe(null); - expect(ctx.dashboard?.panels[0].type).toBe('add-panel'); - }); - }); - - dashboardPageScenario('Given panel with id 0', (ctx) => { - ctx.setup(() => { - ctx.mount(); - ctx.setDashboardProp({ - panels: [{ id: 0, type: 'graph' }], - schemaVersion: 17, - }); - ctx.wrapper?.setProps({ - queryParams: { editPanel: '0' }, - }); - }); - - it('Should go into edit mode', () => { - const state = ctx.wrapper?.state(); - expect(ctx.wrapper).not.toBe(null); - expect(state).not.toBe(null); - expect(state?.editPanel).not.toBe(null); + it('Should reset state when leaving', () => { + ctx.rerender({ queryParams: {} }); + expect(screen.queryByTitle('Apply changes and go back to dashboard')).not.toBeInTheDocument(); }); }); dashboardPageScenario('When dashboard unmounts', (ctx) => { ctx.setup(() => { ctx.mount(); - ctx.setDashboardProp({ - panels: [{ id: 0, type: 'graph' }], - schemaVersion: 17, - }); - ctx.wrapper?.unmount(); + ctx.rerender({ dashboard: getTestDashboard() }); + ctx.unmount(); }); - it('Should call clean up action', () => { - expect(ctx.cleanUpDashboardAndVariablesMock).toHaveBeenCalledTimes(1); + it('Should call close action', () => { + expect(ctx.props.cleanUpDashboardAndVariables).toHaveBeenCalledTimes(1); }); }); - dashboardPageScenario('Kiosk mode none', (ctx) => { + dashboardPageScenario('When dashboard changes', (ctx) => { ctx.setup(() => { - ctx.mount({ - queryParams: {}, - }); - ctx.setDashboardProp({ - panels: [{ id: 0, type: 'graph' }], - schemaVersion: 17, + ctx.mount(); + ctx.rerender({ dashboard: getTestDashboard() }); + ctx.rerender({ + match: { + params: { uid: 'new-uid' }, + } as any, + dashboard: getTestDashboard({ title: 'Another dashboard' }), }); }); - it('should not render dashboard navigation ', () => { - expect(ctx.wrapper?.find(`[aria-label="${selectors.pages.Dashboard.DashNav.nav}"]`)).toHaveLength(1); - expect(ctx.wrapper?.find(`[aria-label="${selectors.pages.Dashboard.SubMenu.submenu}"]`)).toHaveLength(1); + it('Should call clean up action and init', () => { + expect(ctx.props.cleanUpDashboardAndVariables).toHaveBeenCalledTimes(1); + expect(ctx.props.initDashboard).toHaveBeenCalledTimes(2); }); }); - dashboardPageScenario('Kiosk mode tv', (ctx) => { + dashboardPageScenario('No kiosk mode tv', (ctx) => { ctx.setup(() => { - ctx.mount({ - queryParams: { kiosk: 'tv' }, - }); - ctx.setDashboardProp({ - panels: [{ id: 0, type: 'graph' }], - schemaVersion: 17, - }); + ctx.mount({ dashboard: getTestDashboard() }); + ctx.rerender({ dashboard: ctx.dashboard }); }); - it('should not render dashboard navigation ', () => { - expect(ctx.wrapper?.find(`[aria-label="${selectors.pages.Dashboard.DashNav.nav}"]`)).toHaveLength(1); - expect(ctx.wrapper?.find(`[aria-label="${selectors.pages.Dashboard.SubMenu.submenu}"]`)).toHaveLength(0); + it('should render dashboard page toolbar and submenu', () => { + expect(screen.queryAllByLabelText(selectors.pages.Dashboard.DashNav.nav)).toHaveLength(1); + expect(screen.queryAllByLabelText(selectors.pages.Dashboard.SubMenu.submenu)).toHaveLength(1); }); }); - dashboardPageScenario('Kiosk mode full', (ctx) => { + dashboardPageScenario('When in full kiosk mode', (ctx) => { ctx.setup(() => { ctx.mount({ queryParams: { kiosk: true }, + dashboard: getTestDashboard(), }); - ctx.setDashboardProp({ - panels: [{ id: 0, type: 'graph' }], - schemaVersion: 17, - }); + ctx.rerender({ dashboard: ctx.dashboard }); }); - it('should not render dashboard navigation and submenu', () => { - expect(ctx.wrapper?.find(`[aria-label="${selectors.pages.Dashboard.DashNav.nav}"]`)).toHaveLength(0); - expect(ctx.wrapper?.find(`[aria-label="${selectors.pages.Dashboard.SubMenu.submenu}"]`)).toHaveLength(0); + it('should not render page toolbar and submenu', () => { + expect(screen.queryAllByLabelText(selectors.pages.Dashboard.DashNav.nav)).toHaveLength(0); + expect(screen.queryAllByLabelText(selectors.pages.Dashboard.SubMenu.submenu)).toHaveLength(0); }); }); - - describe('mapStateToProps', () => { - const props = mapStateToProps({ - panelEditor: {}, - dashboard: { - getModel: () => ({} as DashboardModel), - }, - } as any); - - expect(props.dashboard).toBeDefined(); - }); }); diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index 7ec69c5fc52..13de5e89011 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -1,4 +1,3 @@ -import $ from 'jquery'; import React, { PureComponent } from 'react'; import { css } from 'emotion'; import { hot } from 'react-hot-loader'; @@ -30,6 +29,7 @@ import { GrafanaTheme2, UrlQueryValue } from '@grafana/data'; import { DashboardLoading } from '../components/DashboardLoading/DashboardLoading'; import { DashboardFailed } from '../components/DashboardLoading/DashboardFailed'; import { DashboardPrompt } from '../components/DashboardPrompt/DashboardPrompt'; +import classnames from 'classnames'; export interface DashboardPageRouteParams { uid?: string; @@ -72,6 +72,8 @@ export interface State { updateScrollTop?: number; rememberScrollTop: number; showLoadingState: boolean; + panelNotFound: boolean; + editPanelAccessDenied: boolean; } export class UnthemedDashboardPage extends PureComponent { @@ -85,6 +87,8 @@ export class UnthemedDashboardPage extends PureComponent { showLoadingState: false, scrollTop: 0, rememberScrollTop: 0, + panelNotFound: false, + editPanelAccessDenied: false, }; } @@ -99,7 +103,6 @@ export class UnthemedDashboardPage extends PureComponent { closeDashboard() { this.props.cleanUpDashboardAndVariables(); - this.setPanelFullscreenClass(false); this.setState(this.getCleanState()); } @@ -120,10 +123,8 @@ export class UnthemedDashboardPage extends PureComponent { }); } - componentDidUpdate(prevProps: Props) { - const { dashboard, match, queryParams, templateVarsChangedInUrl } = this.props; - const { editPanel, viewPanel } = this.state; - + componentDidUpdate(prevProps: Props, prevState: State) { + const { dashboard, match, templateVarsChangedInUrl } = this.props; const routeReloadCounter = (this.props.history.location.state as any)?.routeReloadCounter; if (!dashboard) { @@ -163,83 +164,88 @@ export class UnthemedDashboardPage extends PureComponent { } } - const urlEditPanelId = queryParams.editPanel; - const urlViewPanelId = queryParams.viewPanel; - // entering edit mode - if (!editPanel && urlEditPanelId) { + if (this.state.editPanel && !prevState.editPanel) { dashboardWatcher.setEditingState(true); - - this.getPanelByIdFromUrlParam(urlEditPanelId, (panel) => { - // if no edit permission show error - if (!dashboard.canEditPanel(panel)) { - this.props.notifyApp(createErrorNotification('Permission to edit panel denied')); - return; - } - - this.setState({ editPanel: panel }); - }); } // leaving edit mode - if (editPanel && !urlEditPanelId) { + if (!this.state.editPanel && prevState.editPanel) { dashboardWatcher.setEditingState(false); - this.setState({ editPanel: null }); } - // entering view mode - if (!viewPanel && urlViewPanelId) { - this.getPanelByIdFromUrlParam(urlViewPanelId, (panel) => { - this.setPanelFullscreenClass(true); - dashboard.initViewPanel(panel); - this.setState({ - viewPanel: panel, - rememberScrollTop: this.state.scrollTop, - updateScrollTop: 0, - }); - }); + if (this.state.editPanelAccessDenied) { + this.props.notifyApp(createErrorNotification('Permission to edit panel denied')); + locationService.partial({ editPanel: null }); } - // leaving view mode - if (viewPanel && !urlViewPanelId) { - this.setPanelFullscreenClass(false); - dashboard.exitViewPanel(viewPanel); - this.setState( - { viewPanel: null, updateScrollTop: this.state.rememberScrollTop }, - this.triggerPanelsRendering.bind(this) - ); - } - } - - getPanelByIdFromUrlParam(urlPanelId: string, callback: (panel: PanelModel) => void) { - const { dashboard } = this.props; - - const panelId = parseInt(urlPanelId!, 10); - dashboard!.expandParentRowFor(panelId); - const panel = dashboard!.getPanelById(panelId); - - if (!panel) { - // Panel not found - this.props.notifyApp(createErrorNotification(`Panel with ID ${urlPanelId} not found`)); - // Clear url state + if (this.state.panelNotFound) { + this.props.notifyApp(createErrorNotification(`Panel not found`)); locationService.partial({ editPanel: null, viewPanel: null }); - return; - } - - callback(panel); - } - - triggerPanelsRendering() { - try { - this.props.dashboard!.render(); - } catch (err) { - console.error(err); - this.props.notifyApp(createErrorNotification(`Panel rendering error`, err)); } } - setPanelFullscreenClass(isFullscreen: boolean) { - $('body').toggleClass('panel-in-fullscreen', isFullscreen); + static getDerivedStateFromProps(props: Props, state: State) { + const { dashboard, queryParams } = props; + + const urlEditPanelId = queryParams.editPanel; + const urlViewPanelId = queryParams.viewPanel; + + if (!dashboard) { + return state; + } + + // Entering edit mode + if (!state.editPanel && urlEditPanelId) { + const panel = dashboard.getPanelByUrlId(urlEditPanelId); + if (!panel) { + return { ...state, panelNotFound: true }; + } + + if (dashboard.canEditPanel(panel)) { + return { ...state, editPanel: panel }; + } else { + return { ...state, editPanelAccessDenied: true }; + } + } + // Leaving edit mode + else if (state.editPanel && !urlEditPanelId) { + return { ...state, editPanel: null }; + } + + // Entering view mode + if (!state.viewPanel && urlViewPanelId) { + const panel = dashboard.getPanelByUrlId(urlViewPanelId); + if (!panel) { + return { ...state, panelNotFound: urlEditPanelId }; + } + + // This mutable state feels wrong to have in getDerivedStateFromProps + // Should move this state out of dashboard in the future + dashboard.initViewPanel(panel); + + return { + ...state, + viewPanel: panel, + rememberScrollTop: state.scrollTop, + updateScrollTop: 0, + }; + } + // Leaving view mode + else if (state.viewPanel && !urlViewPanelId) { + // This mutable state feels wrong to have in getDerivedStateFromProps + // Should move this state out of dashboard in the future + dashboard.exitViewPanel(state.viewPanel); + + return { ...state, viewPanel: null, updateScrollTop: state.rememberScrollTop }; + } + + // if we removed url edit state, clear any panel not found state + if (state.panelNotFound || (state.editPanelAccessDenied && !urlEditPanelId)) { + return { ...state, panelNotFound: false, editPanelAccessDenied: false }; + } + + return state; } setScrollTop = ({ scrollTop }: ScrollbarPosition): void => { @@ -288,7 +294,7 @@ export class UnthemedDashboardPage extends PureComponent { } render() { - const { dashboard, isInitSlow, initError, isPanelEditorOpen, queryParams, theme } = this.props; + const { dashboard, isInitSlow, initError, queryParams, theme } = this.props; const { editPanel, viewPanel, scrollTop, updateScrollTop } = this.state; const kioskMode = getKioskMode(queryParams.kiosk); const styles = getStyles(theme, kioskMode); @@ -304,9 +310,12 @@ export class UnthemedDashboardPage extends PureComponent { // Only trigger render when the scroll has moved by 25 const approximateScrollTop = Math.round(scrollTop / 25) * 25; const inspectPanel = this.getInspectPanel(); + const containerClassNames = classnames(styles.dashboardContainer, { + 'panel-in-fullscreen': viewPanel, + }); return ( -
+
{kioskMode !== KioskMode.Full && (
{ viewPanel={viewPanel} editPanel={editPanel} scrollTop={approximateScrollTop} - isPanelEditorOpen={isPanelEditorOpen} />
@@ -363,7 +371,6 @@ export const mapStateToProps = (state: StoreState) => ({ isInitSlow: state.dashboard.isInitSlow, initError: state.dashboard.initError, dashboard: state.dashboard.getModel(), - isPanelEditorOpen: state.panelEditor.isOpen, }); const mapDispatchToProps = { diff --git a/public/app/features/dashboard/containers/SoloPanelPage.tsx b/public/app/features/dashboard/containers/SoloPanelPage.tsx index 16449e96d51..a2361dc71ed 100644 --- a/public/app/features/dashboard/containers/SoloPanelPage.tsx +++ b/public/app/features/dashboard/containers/SoloPanelPage.tsx @@ -55,12 +55,7 @@ export class SoloPanelPage extends Component { // we just got a new dashboard if (!prevProps.dashboard || prevProps.dashboard.uid !== dashboard.uid) { - const panelId = this.getPanelId(); - - // need to expand parent row if this panel is inside a row - dashboard.expandParentRowFor(panelId); - - const panel = dashboard.getPanelById(panelId); + const panel = dashboard.getPanelByUrlId(this.props.queryParams.panelId); if (!panel) { this.setState({ notFound: true }); diff --git a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap b/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap deleted file mode 100644 index 765373393fd..00000000000 --- a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap +++ /dev/null @@ -1,1075 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`DashboardPage Dashboard init completed Should render dashboard grid 1`] = ` -
-
- -
- -
- -
-
- -
- -
- -
-
-`; - -exports[`DashboardPage Dashboard is fetching slowly Should render slow init state 1`] = ` - -`; - -exports[`DashboardPage Given initial state Should render nothing 1`] = `""`; - -exports[`DashboardPage When dashboard has editview url state should render settings view 1`] = ` -
-
- -
- -
- -
-
- -
- -
- -
- -
-`; diff --git a/public/app/features/dashboard/dashgrid/DashboardGrid.tsx b/public/app/features/dashboard/dashgrid/DashboardGrid.tsx index 8e573758d2e..bf5c66dbd4f 100644 --- a/public/app/features/dashboard/dashgrid/DashboardGrid.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardGrid.tsx @@ -22,7 +22,6 @@ export interface Props { editPanel: PanelModel | null; viewPanel: PanelModel | null; scrollTop: number; - isPanelEditorOpen?: boolean; } export interface State { diff --git a/public/app/features/dashboard/services/TimeSrv.ts b/public/app/features/dashboard/services/TimeSrv.ts index 734af2c0f20..a33751f1f31 100644 --- a/public/app/features/dashboard/services/TimeSrv.ts +++ b/public/app/features/dashboard/services/TimeSrv.ts @@ -211,21 +211,29 @@ export class TimeSrv { this.stopAutoRefresh(); - if (interval) { - const validInterval = this.contextSrv.getValidInterval(interval); - const intervalMs = rangeUtil.intervalToMs(validInterval); + const currentUrlState = locationService.getSearchObject(); - this.refreshTimer = setTimeout(() => { - this.startNextRefreshTimer(intervalMs); - this.refreshDashboard(); - }, intervalMs); + if (!interval) { + // Clear URL state + if (currentUrlState.refresh) { + locationService.partial({ refresh: null }, true); + } + + return; } - if (interval) { - const refresh = this.contextSrv.getValidInterval(interval); + const validInterval = this.contextSrv.getValidInterval(interval); + const intervalMs = rangeUtil.intervalToMs(validInterval); + + this.refreshTimer = setTimeout(() => { + this.startNextRefreshTimer(intervalMs); + this.refreshDashboard(); + }, intervalMs); + + const refresh = this.contextSrv.getValidInterval(interval); + + if (currentUrlState.refresh !== refresh) { locationService.partial({ refresh }, true); - } else { - locationService.partial({ refresh: null }, true); } } diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index 462b9dfca0b..5a973508262 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -1051,17 +1051,22 @@ export class DashboardModel { this.events.emit(CoreEvents.templateVariableValueUpdated); } - expandParentRowFor(panelId: number) { + getPanelByUrlId(panelUrlId: string) { + const panelId = parseInt(panelUrlId ?? '0', 10); + + // First try to find it in a collapsed row and exand it for (const panel of this.panels) { if (panel.collapsed) { for (const rowPanel of panel.panels) { if (rowPanel.id === panelId) { this.toggleRow(panel); - return; + break; } } } } + + return this.getPanelById(panelId); } toggleLegendsForAll() { diff --git a/public/sass/_grafana.scss b/public/sass/_grafana.scss index 046d42b00e3..9a228e6874f 100644 --- a/public/sass/_grafana.scss +++ b/public/sass/_grafana.scss @@ -80,7 +80,6 @@ @import 'components/dashboard_list'; @import 'components/page_header'; @import 'components/dashboard_settings'; -@import 'components/panel_editor'; @import 'components/toolbar'; @import 'components/add_data_source.scss'; @import 'components/page_loader'; diff --git a/public/sass/components/_panel_editor.scss b/public/sass/components/_panel_editor.scss deleted file mode 100644 index 3f18767fb0e..00000000000 --- a/public/sass/components/_panel_editor.scss +++ /dev/null @@ -1,18 +0,0 @@ -.panel-editor__scroll { - flex-grow: 1; - min-width: 0; - display: flex; - min-height: 0; - height: 100%; - overflow: hidden; -} - -.panel-editor__content { - padding: 0 16px 16px 16px; -} - -.panel-in-fullscreen { - .search-container { - left: 0 !important; - } -}