From 9b6a8431778c4313add25b4ba9b99744957cddc6 Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Fri, 7 Feb 2020 14:59:04 +0100 Subject: [PATCH] New panel edit: don't query when entering edit mode (#21921) * First attempt * Save confirmation with discard option, reusing queriess a little bit differently * simplify cloning panels and restoring last results in panel query runner * Remove save button * Update public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx * Exit/discard buttons * Update snaps * Review comments * Fix lint --- .../components/ConfirmModal/ConfirmModal.tsx | 14 ++- .../src/components/Portal/Portal.tsx | 8 +- .../components/PanelEditor/PanelEditor.tsx | 86 ++++++++++++++----- .../dashboard/containers/DashboardPage.tsx | 10 ++- .../__snapshots__/DashboardPage.test.tsx.snap | 14 +++ .../dashboard/dashgrid/DashboardPanel.tsx | 4 +- .../dashboard/dashgrid/PanelChrome.tsx | 21 ++++- .../__snapshots__/DashboardGrid.test.tsx.snap | 24 ++++++ .../dashboard/state/DashboardModel.ts | 16 +++- .../features/dashboard/state/PanelModel.ts | 15 +++- .../dashboard/state/PanelQueryRunner.ts | 10 ++- 11 files changed, 185 insertions(+), 37 deletions(-) diff --git a/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx b/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx index 8bddaf35677..ca3906e2fef 100644 --- a/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx +++ b/packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx @@ -35,13 +35,23 @@ interface Props { title: string; body: string; confirmText: string; + dismissText?: string; icon?: IconType; onConfirm(): void; onDismiss(): void; } -export const ConfirmModal: FC = ({ isOpen, title, body, confirmText, icon, onConfirm, onDismiss }) => { +export const ConfirmModal: FC = ({ + isOpen, + title, + body, + confirmText, + dismissText = 'Cancel', + icon, + onConfirm, + onDismiss, +}) => { const theme = useContext(ThemeContext); const styles = getStyles(theme); @@ -54,7 +64,7 @@ export const ConfirmModal: FC = ({ isOpen, title, body, confirmText, icon {confirmText} diff --git a/packages/grafana-ui/src/components/Portal/Portal.tsx b/packages/grafana-ui/src/components/Portal/Portal.tsx index bc6fa37f2d0..586001b936f 100644 --- a/packages/grafana-ui/src/components/Portal/Portal.tsx +++ b/packages/grafana-ui/src/components/Portal/Portal.tsx @@ -1,4 +1,4 @@ -import { PureComponent } from 'react'; +import React, { PureComponent } from 'react'; import ReactDOM from 'react-dom'; interface Props { @@ -27,6 +27,10 @@ export class Portal extends PureComponent { } render() { - return ReactDOM.createPortal(this.props.children, this.node); + // Default z-index is high to make sure + return ReactDOM.createPortal( +
{this.props.children}
, + this.node + ); } } diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx index 2d05f0425c8..d76c8d6f39e 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx @@ -1,13 +1,16 @@ import React, { PureComponent } from 'react'; import { css } from 'emotion'; import { GrafanaTheme } from '@grafana/data'; -import { stylesFactory } from '@grafana/ui'; +import { stylesFactory, Forms } from '@grafana/ui'; import config from 'app/core/config'; import { PanelModel } from '../../state/PanelModel'; import { DashboardModel } from '../../state/DashboardModel'; import { DashboardPanel } from '../../dashgrid/DashboardPanel'; import { QueriesTab } from '../../panel_editor/QueriesTab'; +import { StoreState } from '../../../../types/store'; +import { connect } from 'react-redux'; +import { updateLocation } from '../../../../core/reducers/location'; const getStyles = stylesFactory((theme: GrafanaTheme) => ({ wrapper: css` @@ -47,6 +50,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => ({ interface Props { dashboard: DashboardModel; panel: PanelModel; + updateLocation: typeof updateLocation; } interface State { @@ -54,15 +58,35 @@ interface State { } export class PanelEditor extends PureComponent { - state: State = {}; - - componentDidMount() { - const { panel } = this.props; - const dirtyPanel = new PanelModel(panel.getSaveModel()); - - this.setState({ dirtyPanel }); + constructor(props: Props) { + super(props); + const { panel } = props; + const dirtyPanel = panel.getEditClone(); + this.state = { dirtyPanel }; } + onPanelUpdate = () => { + const { dirtyPanel } = this.state; + const { dashboard } = this.props; + dashboard.updatePanel(dirtyPanel); + }; + + onPanelExit = () => { + const { updateLocation } = this.props; + this.onPanelUpdate(); + updateLocation({ + query: { editPanel: null }, + partial: true, + }); + }; + + onDiscard = () => { + this.props.updateLocation({ + query: { editPanel: null }, + partial: true, + }); + }; + render() { const { dashboard } = this.props; const { dirtyPanel } = this.state; @@ -74,23 +98,41 @@ export class PanelEditor extends PureComponent { } return ( -
-
-
- + <> +
+
+
+ +
+
+ +
-
- ; +
+ + Discard + + Exit
-
Visualization settings
-
+ ); } } + +const mapStateToProps = (state: StoreState) => ({ + location: state.location, +}); + +const mapDispatchToProps = { + updateLocation, +}; + +export default connect(mapStateToProps, mapDispatchToProps)(PanelEditor); diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index 9e592f13018..714875a99ff 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -12,8 +12,8 @@ import { DashboardGrid } from '../dashgrid/DashboardGrid'; import { DashNav } from '../components/DashNav'; import { SubMenu } from '../components/SubMenu'; import { DashboardSettings } from '../components/DashboardSettings'; -import { PanelEditor } from '../components/PanelEditor'; -import { CustomScrollbar, Alert } from '@grafana/ui'; +import PanelEditor from '../components/PanelEditor/PanelEditor'; +import { CustomScrollbar, Alert, Portal } from '@grafana/ui'; // Redux import { initDashboard } from '../state/initDashboard'; @@ -325,7 +325,11 @@ export class DashboardPage extends PureComponent {
{inspectPanel && } - {editPanel && } + {editPanel && ( + + + + )}
); } diff --git a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap b/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap index 8fe42b3af72..4761bef87da 100644 --- a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap +++ b/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap @@ -66,6 +66,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1` "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -93,6 +94,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -179,6 +181,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1` "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -206,6 +209,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -271,6 +275,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1` "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -298,6 +303,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -395,6 +401,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -422,6 +429,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -506,6 +514,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -533,6 +542,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -601,6 +611,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -628,6 +639,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -693,6 +705,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "y": 0, }, "id": 1, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -720,6 +733,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } diff --git a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx index 41baaad41e6..d82e46ce0d8 100644 --- a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx @@ -22,6 +22,7 @@ export interface Props { panel: PanelModel; dashboard: DashboardModel; isEditing: boolean; + isInEditMode?: boolean; isFullscreen: boolean; isInView: boolean; } @@ -130,7 +131,7 @@ export class DashboardPanel extends PureComponent { }; renderPanel() { - const { dashboard, panel, isFullscreen, isInView } = this.props; + const { dashboard, panel, isFullscreen, isInView, isInEditMode } = this.props; const { plugin } = this.state; if (plugin.angularPanelCtrl) { @@ -151,6 +152,7 @@ export class DashboardPanel extends PureComponent { dashboard={dashboard} isFullscreen={isFullscreen} isInView={isInView} + isInEditMode={isInEditMode} width={width} height={height} /> diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 481b8089463..613f6cd78ad 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -35,6 +35,7 @@ export interface Props { plugin: PanelPlugin; isFullscreen: boolean; isInView: boolean; + isInEditMode?: boolean; width: number; height: number; } @@ -69,7 +70,7 @@ export class PanelChrome extends PureComponent { } componentDidMount() { - const { panel, dashboard } = this.props; + const { panel, dashboard, isInEditMode } = this.props; panel.events.on(PanelEvents.refresh, this.onRefresh); panel.events.on(PanelEvents.render, this.onRender); dashboard.panelInitialized(this.props.panel); @@ -84,6 +85,12 @@ export class PanelChrome extends PureComponent { }, isFirstLoad: false, }); + } else if (isInEditMode && this.panelHasLastResult()) { + console.log('Reusing results!'); + const lastResult = panel.getQueryRunner().getLastResult(); + if (lastResult) { + this.onDataUpdate(lastResult); + } } else if (!this.wantsQueryExecution) { this.setState({ isFirstLoad: false }); } @@ -160,8 +167,8 @@ export class PanelChrome extends PureComponent { } onRefresh = () => { + // debugger const { panel, isInView, width } = this.props; - if (!isInView) { console.log('Refresh when panel is visible', panel.id); this.setState({ refreshWhenInView: true }); @@ -232,8 +239,16 @@ export class PanelChrome extends PureComponent { return panel.snapshotData && panel.snapshotData.length; } + panelHasLastResult = () => { + return !!this.props.panel.getQueryRunner().getLastResult(); + }; + get wantsQueryExecution() { - return !(this.props.plugin.meta.skipDataQuery || this.hasPanelSnapshot); + return !( + this.props.plugin.meta.skipDataQuery || + this.hasPanelSnapshot || + (this.props.isInEditMode && !this.panelHasLastResult()) + ); } onChangeTimeRange = (timeRange: AbsoluteTimeRange) => { diff --git a/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap b/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap index 774608195c4..3694703f37f 100644 --- a/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap +++ b/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap @@ -142,6 +142,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 1, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -168,6 +169,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 2, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -194,6 +196,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 3, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -220,6 +223,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 4, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -247,6 +251,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -269,6 +274,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 1, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -379,6 +385,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 1, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -405,6 +412,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 2, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -431,6 +439,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 3, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -457,6 +466,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 4, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -484,6 +494,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -506,6 +517,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 2, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -616,6 +628,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 1, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -642,6 +655,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 2, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -668,6 +682,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 3, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -694,6 +709,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 4, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -721,6 +737,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -743,6 +760,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 3, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -853,6 +871,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 1, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -879,6 +898,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 2, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -905,6 +925,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 3, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -931,6 +952,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 4, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", @@ -958,6 +980,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` "timezone": "", "title": "My dashboard", "uid": null, + "updatePanel": [Function], "version": 0, } } @@ -980,6 +1003,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` }, "id": 4, "isInView": false, + "restoreModel": [Function], "targets": Array [ Object { "refId": "A", diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index c1b26ee774a..56d8bd87a53 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -239,7 +239,9 @@ export class DashboardModel { panelInitialized(panel: PanelModel) { panel.initialized(); - if (!this.otherPanelInFullscreen(panel)) { + // In new panel edit there is no need to trigger refresh as editor retrieves last results from the query runner + // as an initial value + if (!this.otherPanelInFullscreen(panel) && !panel.isNewEdit) { panel.refresh(); } } @@ -947,4 +949,16 @@ export class DashboardModel { panel.render(); } } + + updatePanel = (panelModel: PanelModel) => { + let index = 0; + for (const panel of this.panels) { + if (panel.id === panelModel.id) { + this.panels[index].restoreModel(panelModel.getSaveModel()); + this.panels[index].getQueryRunner().pipeDataToSubject(panelModel.getQueryRunner().getLastResult()); + break; + } + index++; + } + }; } diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index 54689533bdd..7c353c3ca11 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -40,6 +40,7 @@ const notPersistedProperties: { [str: string]: boolean } = { cachedPluginOptions: true, plugin: true, queryRunner: true, + restoreModel: true, }; // For angular panels we need to clean up properties when changing type @@ -137,11 +138,14 @@ export class PanelModel { constructor(model: any) { this.events = new Emitter(); - // should not be part of defaults as defaults are removed in save model and // this should not be removed in save model as exporter needs to templatize it this.datasource = null; + this.restoreModel(model); + } + /** Given a persistened PanelModel restores property values */ + restoreModel = (model: any) => { // copy properties from persisted model for (const property in model) { (this as any)[property] = model[property]; @@ -152,7 +156,7 @@ export class PanelModel { // queries must have refId this.ensureQueryIds(); - } + }; ensureQueryIds() { if (this.targets && _.isArray(this.targets)) { @@ -343,6 +347,13 @@ export class PanelModel { }); } + getEditClone() { + const clone = new PanelModel(this.getSaveModel()); + clone.queryRunner = new PanelQueryRunner(this.queryRunner.getLastResult()); + clone.isNewEdit = true; + return clone; + } + getQueryRunner(): PanelQueryRunner { if (!this.queryRunner) { this.queryRunner = new PanelQueryRunner(); diff --git a/public/app/features/dashboard/state/PanelQueryRunner.ts b/public/app/features/dashboard/state/PanelQueryRunner.ts index 48d2bfe63a6..ccf39b9bb2e 100644 --- a/public/app/features/dashboard/state/PanelQueryRunner.ts +++ b/public/app/features/dashboard/state/PanelQueryRunner.ts @@ -56,8 +56,11 @@ export class PanelQueryRunner { private transformations?: DataTransformerConfig[]; private lastResult?: PanelData; - constructor() { + constructor(data?: PanelData) { this.subject = new ReplaySubject(1); + if (data) { + this.pipeDataToSubject(data); + } } /** @@ -169,6 +172,11 @@ export class PanelQueryRunner { }); } + pipeDataToSubject = (data: PanelData) => { + this.subject.next(data); + this.lastResult = data; + }; + setTransformations(transformations?: DataTransformerConfig[]) { this.transformations = transformations; }