DashboardScene: Panel edit toolbar actions (#82302)

* DashboardScene: Panel edit toolbar actions

* Make saving work

* Update public/app/features/dashboard-scene/panel-edit/PanelEditor.test.ts

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

---------

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

public/app/features/dashboard-scene/panel-edit/PanelEditor.test.ts

@@ -1,5 +1,5 @@
 import { PanelPlugin, PanelPluginMeta, PluginType } from '@grafana/data';
-import { SceneFlexItem, SplitLayout, VizPanel } from '@grafana/scenes';
+import { SceneFlexItem, SceneGridItem, SceneGridLayout, SplitLayout, VizPanel } from '@grafana/scenes';
 
 import { DashboardScene } from '../scene/DashboardScene';
 import { activateFullSceneTree } from '../utils/test-utils';
@@ -26,6 +26,37 @@ jest.mock('@grafana/runtime', () => ({
 }));
 
 describe('PanelEditor', () => {
+  describe('When closing editor', () => {
+    it('should apply changes automatically', () => {
+      pluginToLoad = getTestPanelPlugin({ id: 'text', skipDataQuery: true });
+
+      const panel = new VizPanel({
+        key: 'panel-1',
+        pluginId: 'text',
+      });
+
+      const editScene = buildPanelEditScene(panel);
+      const gridItem = new SceneGridItem({ body: panel });
+      const scene = new DashboardScene({
+        editPanel: editScene,
+        isEditing: true,
+        body: new SceneGridLayout({
+          children: [gridItem],
+        }),
+      });
+
+      const deactivate = activateFullSceneTree(scene);
+
+      const vizManager = editScene.state.panelRef.resolve();
+      vizManager.state.panel.setState({ title: 'changed title' });
+
+      deactivate();
+
+      const updatedPanel = gridItem.state.body as VizPanel;
+      expect(updatedPanel?.state.title).toBe('changed title');
+    });
+  });
+
   describe('PanelDataPane', () => {
     it('should not exist if panel is skipDataQuery', () => {
       pluginToLoad = getTestPanelPlugin({ id: 'text', skipDataQuery: true });
@@ -44,7 +75,7 @@ describe('PanelEditor', () => {
       expect(((editScene.state.body as SplitLayout).state.primary as SplitLayout).state.secondary).toBeUndefined();
     });
 
-    it('should  exist if panel is supporting querying', () => {
+    it('should exist if panel is supporting querying', () => {
       pluginToLoad = getTestPanelPlugin({ id: 'timeseries' });
 
       const panel = new VizPanel({

public/app/features/dashboard-scene/panel-edit/PanelEditor.tsx

@@ -14,7 +14,6 @@ import {
   VizPanel,
 } from '@grafana/scenes';
 
-import { getDashboardUrl } from '../utils/urlBuilders';
 import {
   findVizPanelByKey,
   getDashboardSceneFor,
@@ -38,8 +37,18 @@ export interface PanelEditorState extends SceneObjectState {
 export class PanelEditor extends SceneObjectBase<PanelEditorState> {
   static Component = PanelEditorRenderer;
 
+  private _discardChanges = false;
+
   public constructor(state: PanelEditorState) {
     super(state);
+
+    this.addActivationHandler(() => {
+      return () => {
+        if (!this._discardChanges) {
+          this.commitChanges();
+        }
+      };
+    });
   }
   public getUrlKey() {
     return this.state.panelId.toString();
@@ -55,23 +64,11 @@ export class PanelEditor extends SceneObjectBase<PanelEditorState> {
   }
 
   public onDiscard = () => {
-    // Open question on what to preserve when going back
+    this._discardChanges = true;
-    // Preserve time range, and variables state (that might have been changed while in panel edit)
+    locationService.partial({ editPanel: null });
-    // Preserve current panel data? (say if you just changed the time range and have new data)
-    this._navigateBackToDashboard();
   };
 
-  public onApply = () => {
+  public commitChanges() {
-    this._commitChanges();
-    this._navigateBackToDashboard();
-  };
-
-  public onSave = () => {
-    this._commitChanges();
-    // Open dashboard save drawer
-  };
-
-  private _commitChanges() {
     const dashboard = getDashboardSceneFor(this);
     const sourcePanel = findVizPanelByKey(dashboard.state.body, getVizPanelKeyForPanelId(this.state.panelId));
 
@@ -84,26 +81,6 @@ export class PanelEditor extends SceneObjectBase<PanelEditorState> {
     if (sourcePanel!.parent instanceof SceneGridItem) {
       sourcePanel!.parent.setState({ body: panelMngr.state.panel.clone() });
     }
-
-    dashboard.setState({
-      isDirty: true,
-    });
-  }
-
-  private _navigateBackToDashboard() {
-    const dashboard = getDashboardSceneFor(this);
-    locationService.push(
-      getDashboardUrl({
-        uid: dashboard.state.uid,
-        slug: dashboard.state.meta.slug,
-        currentQueryParams: locationService.getLocation().search,
-        updateQuery: {
-          editPanel: null,
-          // Clean the PanelEditor data pane tab query param
-          tab: null,
-        },
-      })
-    );
   }
 }
 

public/app/features/dashboard-scene/panel-edit/PanelEditorRenderer.tsx

@@ -3,10 +3,9 @@ import React from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { SceneComponentProps } from '@grafana/scenes';
-import { Button, useStyles2 } from '@grafana/ui';
+import { useStyles2 } from '@grafana/ui';
-import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate';
-import { NavToolbarSeparator } from 'app/core/components/AppChrome/NavToolbar/NavToolbarSeparator';
 
+import { NavToolbarActions } from '../scene/NavToolbarActions';
 import { getDashboardSceneFor } from '../utils/utils';
 
 import { PanelEditor } from './PanelEditor';
@@ -19,7 +18,7 @@ export function PanelEditorRenderer({ model }: SceneComponentProps<PanelEditor>)
 
   return (
     <>
-      <AppChromeUpdate actions={getToolbarActions(model)} />
+      <NavToolbarActions dashboard={dashboard} />
       <div className={styles.canvasContent}>
         {controls && (
           <div className={styles.controls}>
@@ -36,29 +35,6 @@ export function PanelEditorRenderer({ model }: SceneComponentProps<PanelEditor>)
   );
 }
 
-function getToolbarActions(editor: PanelEditor) {
-  return (
-    <>
-      <NavToolbarSeparator leftActionsSeparator key="separator" />
-
-      <Button
-        onClick={editor.onDiscard}
-        tooltip=""
-        key="panel-edit-discard"
-        variant="destructive"
-        fill="outline"
-        size="sm"
-      >
-        Discard
-      </Button>
-
-      <Button onClick={editor.onApply} tooltip="" key="panel-edit-apply" variant="primary" size="sm">
-        Apply
-      </Button>
-    </>
-  );
-}
-
 function getStyles(theme: GrafanaTheme2) {
   return {
     canvasContent: css({

public/app/features/dashboard-scene/saving/getSaveDashboardChange.test.ts

@@ -1,7 +1,9 @@
 import { MultiValueVariable, sceneGraph } from '@grafana/scenes';
 
+import { buildPanelEditScene } from '../panel-edit/PanelEditor';
 import { transformSaveModelToScene } from '../serialization/transformSaveModelToScene';
 import { transformSceneToSaveModel } from '../serialization/transformSceneToSaveModel';
+import { findVizPanelByKey } from '../utils/utils';
 
 import { getSaveDashboardChange } from './getSaveDashboardChange';
 
@@ -59,15 +61,43 @@ describe('getSaveDashboardChange', () => {
     expect(result.hasChanges).toBe(true);
     expect(result.diffCount).toBe(2);
   });
+
+  describe('Saving from panel edit', () => {
+    it('Should commit panel edit changes', () => {
+      const dashboard = setup();
+      const panel = findVizPanelByKey(dashboard, 'panel-1')!;
+      const editScene = buildPanelEditScene(panel);
+
+      dashboard.onEnterEditMode();
+      dashboard.setState({ editPanel: editScene });
+
+      const vizManager = editScene.state.panelRef.resolve();
+      vizManager.state.panel.setState({ title: 'changed title' });
+
+      const result = getSaveDashboardChange(dashboard, false, true);
+      const panelSaveModel = result.changedSaveModel.panels![0];
+      expect(panelSaveModel.title).toBe('changed title');
+    });
+  });
 });
 
-function setup() {
+interface ScenarioOptions {
+  fromPanelEdit?: boolean;
+}
+
+function setup(options: ScenarioOptions = {}) {
   const dashboard = transformSaveModelToScene({
     dashboard: {
       title: 'hello',
       uid: 'my-uid',
       schemaVersion: 30,
-      panels: [],
+      panels: [
+        {
+          id: 1,
+          title: 'Panel 1',
+          type: 'text',
+        },
+      ],
       version: 10,
       templating: {
         list: [

public/app/features/dashboard-scene/saving/getSaveDashboardChange.ts

@@ -15,6 +15,11 @@ export function getSaveDashboardChange(
   saveVariables?: boolean
 ): DashboardChangeInfo {
   const initialSaveModel = dashboard.getInitialSaveModel()!;
+
+  if (dashboard.state.editPanel) {
+    dashboard.state.editPanel.commitChanges();
+  }
+
   const changedSaveModel = transformSceneToSaveModel(dashboard);
   const hasTimeChanged = getHasTimeChanged(changedSaveModel, initialSaveModel);
 

public/app/features/dashboard-scene/scene/NavToolbarActions.tsx

@@ -36,14 +36,16 @@ NavToolbarActions.displayName = 'NavToolbarActions';
  * This part is split into a separate componet to help test this
  */
 export function ToolbarActions({ dashboard }: Props) {
-  const { isEditing, viewPanelScene, isDirty, uid, meta, editview } = dashboard.useState();
+  const { isEditing, viewPanelScene, isDirty, uid, meta, editview, editPanel } = dashboard.useState();
   const canSaveAs = contextSrv.hasEditPermissionInFolders;
   const toolbarActions: ToolbarAction[] = [];
   const buttonWithExtraMargin = useStyles2(getStyles);
+  const isEditingPanel = Boolean(editPanel);
+  const isViewingPanel = Boolean(viewPanelScene);
 
   toolbarActions.push({
     group: 'icon-actions',
-    condition: uid && !editview && Boolean(meta.canStar),
+    condition: uid && !editview && Boolean(meta.canStar) && !isEditingPanel,
     render: () => {
       let desc = meta.isStarred
         ? t('dashboard.toolbar.unmark-favorite', 'Unmark as favorite')
@@ -66,7 +68,7 @@ export function ToolbarActions({ dashboard }: Props) {
 
   toolbarActions.push({
     group: 'icon-actions',
-    condition: uid && !editview,
+    condition: uid && !editview && !isEditingPanel,
     render: () => (
       <ToolbarButton
         key="view-in-old-dashboard-button"
@@ -98,7 +100,7 @@ export function ToolbarActions({ dashboard }: Props) {
     ),
   });
 
-  if (dynamicDashNavActions.left.length > 0) {
+  if (dynamicDashNavActions.left.length > 0 && !isEditingPanel) {
     dynamicDashNavActions.left.map((action, index) => {
       const props = { dashboard: getDashboardSrv().getCurrent()! };
       if (action.show(props)) {
@@ -114,11 +116,11 @@ export function ToolbarActions({ dashboard }: Props) {
 
   toolbarActions.push({
     group: 'back-button',
-    condition: Boolean(viewPanelScene),
+    condition: isViewingPanel || isEditingPanel,
     render: () => (
       <Button
         onClick={() => {
-          locationService.partial({ viewPanel: null });
+          locationService.partial({ viewPanel: null, editPanel: null });
         }}
         tooltip=""
         key="back"
@@ -173,7 +175,7 @@ export function ToolbarActions({ dashboard }: Props) {
 
   toolbarActions.push({
     group: 'main-buttons',
-    condition: !isEditing && dashboard.canEditDashboard() && !viewPanelScene,
+    condition: !isEditing && dashboard.canEditDashboard() && !isViewingPanel && !isEditingPanel,
     render: () => (
       <Button
         onClick={() => {
@@ -192,7 +194,7 @@ export function ToolbarActions({ dashboard }: Props) {
 
   toolbarActions.push({
     group: 'settings',
-    condition: isEditing && dashboard.canEditDashboard() && !viewPanelScene && !editview,
+    condition: isEditing && dashboard.canEditDashboard() && !isViewingPanel && !isEditingPanel && !editview,
     render: () => (
       <Button
         onClick={() => {
@@ -211,7 +213,7 @@ export function ToolbarActions({ dashboard }: Props) {
 
   toolbarActions.push({
     group: 'main-buttons',
-    condition: isEditing && !editview && !meta.isNew,
+    condition: isEditing && !editview && !meta.isNew && !isViewingPanel && !isEditingPanel,
     render: () => (
       <Button
         onClick={() => dashboard.exitEditMode({ skipConfirm: false })}
@@ -226,6 +228,23 @@ export function ToolbarActions({ dashboard }: Props) {
     ),
   });
 
+  toolbarActions.push({
+    group: 'main-buttons',
+    condition: isEditingPanel && !editview && !meta.isNew && !isViewingPanel,
+    render: () => (
+      <Button
+        onClick={editPanel?.onDiscard}
+        tooltip="Discard panel changes"
+        size="sm"
+        key="discard"
+        fill="outline"
+        variant="destructive"
+      >
+        Discard panel changes
+      </Button>
+    ),
+  });
+
   toolbarActions.push({
     group: 'main-buttons',
     condition: isEditing && (meta.canSave || canSaveAs),