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

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<Props> = ({ isOpen, title, body, confirmText, icon, onConfirm, onDismiss }) => {
+export const ConfirmModal: FC<Props> = ({
+  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<Props> = ({ isOpen, title, body, confirmText, icon
             {confirmText}
           </Button>
           <Button variant="inverse" onClick={onDismiss}>
-            Cancel
+            {dismissText}
           </Button>
         </div>
       </div>

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<Props> {
   }
 
   render() {
-    return ReactDOM.createPortal(this.props.children, this.node);
+    // Default z-index is high to make sure
+    return ReactDOM.createPortal(
+      <div style={{ zIndex: 1051, position: 'relative' }}>{this.props.children}</div>,
+      this.node
+    );
   }
 }

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<Props, State> {
-  state: State = {};
+  constructor(props: Props) {
-
+    super(props);
-  componentDidMount() {
+    const { panel } = props;
-    const { panel } = this.props;
+    const dirtyPanel = panel.getEditClone();
-    const dirtyPanel = new PanelModel(panel.getSaveModel());
+    this.state = { dirtyPanel };
-
-    this.setState({ 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<Props, State> {
     }
 
     return (
-      <div className={styles.wrapper}>
+      <>
-        <div className={styles.leftPane}>
+        <div className={styles.wrapper}>
-          <div className={styles.leftPaneViz}>
+          <div className={styles.leftPane}>
-            <DashboardPanel
+            <div className={styles.leftPaneViz}>
-              dashboard={dashboard}
+              <DashboardPanel
-              panel={dirtyPanel}
+                dashboard={dashboard}
-              isEditing={false}
+                panel={dirtyPanel}
-              isFullscreen={false}
+                isEditing={false}
-              isInView={true}
+                isInEditMode
-            />
+                isFullscreen={false}
+                isInView={true}
+              />
+            </div>
+            <div className={styles.leftPaneData}>
+              <QueriesTab panel={dirtyPanel} dashboard={dashboard} />
+            </div>
           </div>
-          <div className={styles.leftPaneData}>
+          <div className={styles.rightPane}>
-            <QueriesTab panel={dirtyPanel} dashboard={dashboard} />;
+            <Forms.Button variant="destructive" onClick={this.onDiscard}>
+              Discard
+            </Forms.Button>
+            <Forms.Button onClick={this.onPanelExit}>Exit</Forms.Button>
           </div>
         </div>
-        <div className={styles.rightPane}>Visualization settings</div>
+      </>
-      </div>
     );
   }
 }
+
+const mapStateToProps = (state: StoreState) => ({
+  location: state.location,
+});
+
+const mapDispatchToProps = {
+  updateLocation,
+};
+
+export default connect(mapStateToProps, mapDispatchToProps)(PanelEditor);

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 PanelEditor from '../components/PanelEditor/PanelEditor';
-import { CustomScrollbar, Alert } from '@grafana/ui';
+import { CustomScrollbar, Alert, Portal } from '@grafana/ui';
 
 // Redux
 import { initDashboard } from '../state/initDashboard';
@@ -325,7 +325,11 @@ export class DashboardPage extends PureComponent<Props, State> {
         </div>
 
         {inspectPanel && <PanelInspector dashboard={dashboard} panel={inspectPanel} selectedTab={inspectTab} />}
-        {editPanel && <PanelEditor dashboard={dashboard} panel={editPanel} />}
+        {editPanel && (
+          <Portal>
+            <PanelEditor dashboard={dashboard} panel={editPanel} />
+          </Portal>
+        )}
       </div>
     );
   }

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,
             }
           }

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<Props, State> {
   };
 
   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<Props, State> {
               dashboard={dashboard}
               isFullscreen={isFullscreen}
               isInView={isInView}
+              isInEditMode={isInEditMode}
               width={width}
               height={height}
             />

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<Props, State> {
   }
 
   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<Props, State> {
         },
         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<Props, State> {
   }
 
   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<Props, State> {
     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) => {

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",

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++;
+    }
+  };
 }

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();

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;
   }