From 5b5831ae34bc9ff6a68e9557b48f80d9fef61caf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 22 Jan 2025 12:25:04 +0100 Subject: [PATCH] Dashboard: Simplify repeating logic and panel menu interaction (#99352) * Dashboard: Simplify repeating logic and panel menu interaction * Update * Remove unused behavior --- .../scene/PanelMenuBehavior.tsx | 23 +++++--- .../scene/RowRepeaterBehavior.test.tsx | 57 +++---------------- .../scene/RowRepeaterBehavior.ts | 12 +--- .../layout-default/DashboardGridItem.test.tsx | 4 ++ .../layout-default/DashboardGridItem.tsx | 7 --- .../features/dashboard-scene/utils/utils.ts | 20 +++++++ 6 files changed, 48 insertions(+), 75 deletions(-) diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx index 4268276f60d..0080b57e228 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx @@ -32,7 +32,13 @@ import { ShareDrawer } from '../sharing/ShareDrawer/ShareDrawer'; import { ShareModal } from '../sharing/ShareModal'; import { DashboardInteractions } from '../utils/interactions'; import { getEditPanelUrl, getInspectUrl, getViewPanelUrl, tryGetExploreUrlForPanel } from '../utils/urlBuilders'; -import { getDashboardSceneFor, getPanelIdForVizPanel, getQueryRunnerFor, isLibraryPanel } from '../utils/utils'; +import { + getDashboardSceneFor, + getPanelIdForVizPanel, + getQueryRunnerFor, + isLibraryPanel, + isReadOnlyClone, +} from '../utils/utils'; import { DashboardScene } from './DashboardScene'; import { VizPanelLinks, VizPanelLinksMenu } from './PanelLinks'; @@ -41,7 +47,7 @@ import { UnlinkLibraryPanelModal } from './UnlinkLibraryPanelModal'; /** * Behavior is called when VizPanelMenu is activated (ie when it's opened). */ -export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { +export function panelMenuBehavior(menu: VizPanelMenu) { const asyncFunc = async () => { // hm.. add another generic param to SceneObject to specify parent type? // eslint-disable-next-line @typescript-eslint/consistent-type-assertions @@ -53,6 +59,7 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { const dashboard = getDashboardSceneFor(panel); const { isEmbedded } = dashboard.state.meta; const exploreMenuItem = await getExploreMenuItem(panel); + const isReadOnlyRepeat = isReadOnlyClone(panel); // For embedded dashboards we only have explore action for now if (isEmbedded) { @@ -72,7 +79,7 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { }); } - if (dashboard.canEditDashboard() && dashboard.state.editable && !isRepeat && !isEditingPanel) { + if (dashboard.canEditDashboard() && dashboard.state.editable && !isReadOnlyRepeat && !isEditingPanel) { // We could check isEditing here but I kind of think this should always be in the menu, // and going into panel edit should make the dashboard go into edit mode is it's not already items.push({ @@ -167,7 +174,7 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { }); } - if (dashboard.state.isEditing && !isRepeat && !isEditingPanel) { + if (dashboard.state.isEditing && !isReadOnlyRepeat && !isEditingPanel) { moreSubMenu.push({ text: t('panel.header-menu.duplicate', `Duplicate`), iconClassName: 'file-copy-alt', @@ -188,7 +195,7 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { }); } - if (dashboard.state.isEditing && !isRepeat && !isEditingPanel) { + if (dashboard.state.isEditing && !isReadOnlyRepeat && !isEditingPanel) { if (isLibraryPanel(panel)) { moreSubMenu.push({ text: t('panel.header-menu.unlink-library-panel', `Unlink library panel`), @@ -263,7 +270,7 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { }); } - if (dashboard.canEditDashboard() && plugin && !plugin.meta.skipDataQuery && !isRepeat) { + if (dashboard.canEditDashboard() && plugin && !plugin.meta.skipDataQuery && !isReadOnlyRepeat) { moreSubMenu.push({ text: t('panel.header-menu.get-help', 'Get help'), iconClassName: 'question-circle', @@ -313,7 +320,7 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { }); } - if (dashboard.state.isEditing && !isRepeat && !isEditingPanel) { + if (dashboard.state.isEditing && !isReadOnlyRepeat && !isEditingPanel) { items.push({ text: '', type: 'divider', @@ -335,8 +342,6 @@ export function panelMenuBehavior(menu: VizPanelMenu, isRepeat = false) { asyncFunc(); } -export const repeatPanelMenuBehavior = (menu: VizPanelMenu) => panelMenuBehavior(menu, true); - async function getExploreMenuItem(panel: VizPanel): Promise { const exploreUrl = await tryGetExploreUrlForPanel(panel); if (!exploreUrl) { diff --git a/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.test.tsx b/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.test.tsx index 2e5778070f0..021a1448827 100644 --- a/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.test.tsx +++ b/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.test.tsx @@ -10,17 +10,15 @@ import { SceneVariableSet, TestVariable, VariableValueOption, - VizPanel, - VizPanelMenu, } from '@grafana/scenes'; import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE } from 'app/features/variables/constants'; import { activateFullSceneTree } from '../utils/test-utils'; +import { isReadOnlyClone } from '../utils/utils'; import { DashboardScene } from './DashboardScene'; -import { panelMenuBehavior, repeatPanelMenuBehavior } from './PanelMenuBehavior'; import { RowRepeaterBehavior } from './RowRepeaterBehavior'; -import { DashboardGridItem, RepeatDirection } from './layout-default/DashboardGridItem'; +import { RepeatDirection } from './layout-default/DashboardGridItem'; import { DefaultGridLayoutManager } from './layout-default/DefaultGridLayoutManager'; import { RowActions } from './row-actions/RowActions'; @@ -69,6 +67,13 @@ describe('RowRepeaterBehavior', () => { expect(gridItem.state.body?.state.key).toBe('canvas-1-clone-B1'); }); + it('Repeated rows should be read only', () => { + const row1 = grid.state.children[1] as SceneGridRow; + const row2 = grid.state.children[2] as SceneGridRow; + expect(isReadOnlyClone(row1)).toBe(false); + expect(isReadOnlyClone(row2)).toBe(true); + }); + it('Should update all rows when a panel is added to a clone', async () => { const originalRow = grid.state.children[1] as SceneGridRow; const clone1 = grid.state.children[2] as SceneGridRow; @@ -168,50 +173,6 @@ describe('RowRepeaterBehavior', () => { }); }); - describe('Given scene with DashboardGridItem', () => { - let scene: DashboardScene; - let grid: SceneGridLayout; - let rowToRepeat: SceneGridRow; - - beforeEach(async () => { - const menu = new VizPanelMenu({ - $behaviors: [panelMenuBehavior], - }); - - ({ scene, grid, rowToRepeat } = buildScene({ variableQueryTime: 0 })); - const panel = new VizPanel({ pluginId: 'text', menu }); - panel.getPlugin = () => getPanelPlugin({ skipDataQuery: false }); - - rowToRepeat.setState({ - children: [ - new DashboardGridItem({ - body: panel, - }), - ], - }); - - activateFullSceneTree(scene); - await new Promise((r) => setTimeout(r, 1)); - }); - - it('Should set repeat specific panel menu for repeated rows but not original one', () => { - const row1 = grid.state.children[1] as SceneGridRow; - const row2 = grid.state.children[2] as SceneGridRow; - const panelMenuBehaviorOriginal = ( - ((row1.state.children[0] as DashboardGridItem).state.body as VizPanel).state.menu as VizPanelMenu - ).state.$behaviors; - const panelMenuBehaviorClone = ( - ((row2.state.children[0] as DashboardGridItem).state.body as VizPanel).state.menu as VizPanelMenu - ).state.$behaviors; - - expect(panelMenuBehaviorOriginal).toBeDefined(); - expect(panelMenuBehaviorOriginal![0]).toBe(panelMenuBehavior); - - expect(panelMenuBehaviorClone).toBeDefined(); - expect(panelMenuBehaviorClone![0]).toBe(repeatPanelMenuBehavior); - }); - }); - describe('Given scene empty row', () => { let scene: DashboardScene; let grid: SceneGridLayout; diff --git a/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.ts b/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.ts index 4f10cc0f665..26ce6387627 100644 --- a/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.ts +++ b/public/app/features/dashboard-scene/scene/RowRepeaterBehavior.ts @@ -13,12 +13,10 @@ import { SceneVariableSet, VariableDependencyConfig, VariableValueSingle, - VizPanelMenu, } from '@grafana/scenes'; import { getMultiVariableValues, getQueryRunnerFor } from '../utils/utils'; -import { repeatPanelMenuBehavior } from './PanelMenuBehavior'; import { DashboardGridItem } from './layout-default/DashboardGridItem'; import { DashboardRepeatsProcessedEvent } from './types'; @@ -192,15 +190,7 @@ export class RowRepeaterBehavior extends SceneObjectBase { expect(panel1.state.$variables?.state.variables[0].getValue()).toBe('1'); expect(panel1.state.$variables?.state.variables[0].getValueText?.()).toBe('A'); expect(panel2.state.$variables?.state.variables[0].getValue()).toBe('2'); + + expect(isReadOnlyClone(panel1)).toBe(false); + expect(isReadOnlyClone(panel2)).toBe(true); }); it('Should wait for variable to load', async () => { diff --git a/public/app/features/dashboard-scene/scene/layout-default/DashboardGridItem.tsx b/public/app/features/dashboard-scene/scene/layout-default/DashboardGridItem.tsx index 80d919226ff..b2ad2918ca9 100644 --- a/public/app/features/dashboard-scene/scene/layout-default/DashboardGridItem.tsx +++ b/public/app/features/dashboard-scene/scene/layout-default/DashboardGridItem.tsx @@ -15,7 +15,6 @@ import { MultiValueVariable, LocalValueVariable, CustomVariable, - VizPanelMenu, VizPanelState, VariableValueSingle, SceneVariable, @@ -25,7 +24,6 @@ import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN, GRID_COLUMN_COUNT } from 'app/core import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor'; import { getMultiVariableValues, getQueryRunnerFor } from '../../utils/utils'; -import { repeatPanelMenuBehavior } from '../PanelMenuBehavior'; import { DashboardLayoutItem, DashboardRepeatsProcessedEvent } from '../types'; import { getDashboardGridItemOptions } from './DashboardGridItemEditor'; @@ -142,11 +140,6 @@ export class DashboardGridItem }), key: `${panelToRepeat.state.key}-clone-${index}`, }; - if (index > 0) { - cloneState.menu = new VizPanelMenu({ - $behaviors: [repeatPanelMenuBehavior], - }); - } const clone = panelToRepeat.clone(cloneState); repeatedPanels.push(clone); } diff --git a/public/app/features/dashboard-scene/utils/utils.ts b/public/app/features/dashboard-scene/utils/utils.ts index 55a9c16cdd2..8a288f9b1a4 100644 --- a/public/app/features/dashboard-scene/utils/utils.ts +++ b/public/app/features/dashboard-scene/utils/utils.ts @@ -216,6 +216,26 @@ export function isPanelClone(key: string) { return key.includes('clone'); } +/** + * Recursivly check the scene graph up until it finds a read only clone. + * If the key contains clone-0 it is the reference object and can be edited + */ +export function isReadOnlyClone(sceneObject: SceneObject): boolean { + const key = sceneObject.state.key!; + + // Regular expression to match 'clone-' followed by a number, but not 'clone-0' as the is the reference object + const pattern = /clone-(?!0)/; + if (pattern.test(key)) { + return true; + } + + if (sceneObject.parent) { + return isReadOnlyClone(sceneObject.parent); + } + + return false; +} + export function getDefaultVizPanel(): VizPanel { return new VizPanel({ title: 'Panel Title',