From 87d6bebb9ef27229321dd202498256504642718b Mon Sep 17 00:00:00 2001 From: Oscar Kilhed Date: Mon, 11 Mar 2024 11:33:33 +0100 Subject: [PATCH] Dashboard scenes: Remove panel menu options that are dashboard editing activities when not in edit mode. (#84156) * remove panel menu options that are dasbhoard editing activities when the dashboard is not in edit mode * remove corresponding keybindings when not in edit mode * add keyboard shortcuts but inactivate them when not in edit mode * Add tests; fix tests --- .../scene/PanelMenuBehavior.test.tsx | 51 +++++++-- .../scene/PanelMenuBehavior.tsx | 100 ++++++++++-------- .../scene/keyboardShortcuts.ts | 8 +- 3 files changed, 103 insertions(+), 56 deletions(-) diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx index 815ddae7d53..48cc1fd7ea9 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx @@ -70,7 +70,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(8); + expect(menu.state.items?.length).toBe(6); // verify view panel url keeps url params and adds viewPanel= expect(menu.state.items?.[0].href).toBe('/d/dash-1?from=now-5m&to=now&viewPanel=panel-12'); // verify edit url keeps url time range @@ -119,7 +119,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(9); + expect(menu.state.items?.length).toBe(7); const extensionsSubMenu = menu.state.items?.find((i) => i.text === 'Extensions')?.subMenu; @@ -158,7 +158,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(9); + expect(menu.state.items?.length).toBe(7); const extensionsSubMenu = menu.state.items?.find((i) => i.text === 'Extensions')?.subMenu; @@ -199,7 +199,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(9); + expect(menu.state.items?.length).toBe(7); const extensionsSubMenu = menu.state.items?.find((i) => i.text === 'Extensions')?.subMenu; const menuItem = extensionsSubMenu?.find((i) => (i.text = 'Declare incident when...')); @@ -347,7 +347,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(9); + expect(menu.state.items?.length).toBe(7); const extensionsSubMenu = menu.state.items?.find((i) => i.text === 'Extensions')?.subMenu; @@ -392,7 +392,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(9); + expect(menu.state.items?.length).toBe(7); const extensionsSubMenu = menu.state.items?.find((i) => i.text === 'Extensions')?.subMenu; @@ -445,7 +445,7 @@ describe('panelMenuBehavior', () => { await new Promise((r) => setTimeout(r, 1)); - expect(menu.state.items?.length).toBe(9); + expect(menu.state.items?.length).toBe(7); const extensionsSubMenu = menu.state.items?.find((i) => i.text === 'Extensions')?.subMenu; @@ -470,6 +470,43 @@ describe('panelMenuBehavior', () => { ); }); + it('it should not contain remove and duplicate menu items when not in edit mode', async () => { + const { menu, panel } = await buildTestScene({}); + + panel.getPlugin = () => getPanelPlugin({ skipDataQuery: false }); + + mocks.contextSrv.hasAccessToExplore.mockReturnValue(true); + mocks.getExploreUrl.mockReturnValue(Promise.resolve('/explore')); + + menu.activate(); + + await new Promise((r) => setTimeout(r, 1)); + + expect(menu.state.items?.find((i) => i.text === 'Remove')).toBeUndefined(); + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + expect(moreMenu?.find((i) => i.text === 'Duplicate')).toBeUndefined(); + expect(moreMenu?.find((i) => i.text === 'Create library panel')).toBeUndefined(); + }); + + it('it should contain remove and duplicate menu items when in edit mode', async () => { + const { scene, menu, panel } = await buildTestScene({}); + scene.setState({ isEditing: true }); + + panel.getPlugin = () => getPanelPlugin({ skipDataQuery: false }); + + mocks.contextSrv.hasAccessToExplore.mockReturnValue(true); + mocks.getExploreUrl.mockReturnValue(Promise.resolve('/explore')); + + menu.activate(); + + await new Promise((r) => setTimeout(r, 1)); + + expect(menu.state.items?.find((i) => i.text === 'Remove')).toBeDefined(); + const moreMenu = menu.state.items?.find((i) => i.text === 'More...')?.subMenu; + expect(moreMenu?.find((i) => i.text === 'Duplicate')).toBeDefined(); + expect(moreMenu?.find((i) => i.text === 'Create library panel')).toBeDefined(); + }); + it('should only contain explore when embedded', async () => { const { menu, panel } = await buildTestScene({ isEmbedded: true }); diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx index 323c89e7608..61d958afcea 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx @@ -86,14 +86,16 @@ export function panelMenuBehavior(menu: VizPanelMenu) { shortcut: 'p s', }); - moreSubMenu.push({ - text: t('panel.header-menu.duplicate', `Duplicate`), - onClick: () => { - DashboardInteractions.panelMenuItemClicked('duplicate'); - dashboard.duplicatePanel(panel); - }, - shortcut: 'p d', - }); + if (dashboard.state.isEditing) { + moreSubMenu.push({ + text: t('panel.header-menu.duplicate', `Duplicate`), + onClick: () => { + DashboardInteractions.panelMenuItemClicked('duplicate'); + dashboard.duplicatePanel(panel); + }, + shortcut: 'p d', + }); + } moreSubMenu.push({ text: t('panel.header-menu.copy', `Copy`), @@ -103,32 +105,34 @@ export function panelMenuBehavior(menu: VizPanelMenu) { }, }); - if (parent instanceof LibraryVizPanel) { - moreSubMenu.push({ - text: t('panel.header-menu.unlink-library-panel', `Unlink library panel`), - onClick: () => { - DashboardInteractions.panelMenuItemClicked('unlinkLibraryPanel'); - dashboard.showModal( - new UnlinkLibraryPanelModal({ - panelRef: parent.getRef(), - }) - ); - }, - }); - } else { - moreSubMenu.push({ - text: t('panel.header-menu.create-library-panel', `Create library panel`), - onClick: () => { - DashboardInteractions.panelMenuItemClicked('createLibraryPanel'); - dashboard.showModal( - new ShareModal({ - panelRef: panel.getRef(), - dashboardRef: dashboard.getRef(), - activeTab: shareDashboardType.libraryPanel, - }) - ); - }, - }); + if (dashboard.state.isEditing) { + if (parent instanceof LibraryVizPanel) { + moreSubMenu.push({ + text: t('panel.header-menu.unlink-library-panel', `Unlink library panel`), + onClick: () => { + DashboardInteractions.panelMenuItemClicked('unlinkLibraryPanel'); + dashboard.showModal( + new UnlinkLibraryPanelModal({ + panelRef: parent.getRef(), + }) + ); + }, + }); + } else { + moreSubMenu.push({ + text: t('panel.header-menu.create-library-panel', `Create library panel`), + onClick: () => { + DashboardInteractions.panelMenuItemClicked('createLibraryPanel'); + dashboard.showModal( + new ShareModal({ + panelRef: panel.getRef(), + dashboardRef: dashboard.getRef(), + activeTab: shareDashboardType.libraryPanel, + }) + ); + }, + }); + } } moreSubMenu.push({ @@ -196,20 +200,22 @@ export function panelMenuBehavior(menu: VizPanelMenu) { }); } - items.push({ - text: '', - type: 'divider', - }); + if (dashboard.state.isEditing) { + items.push({ + text: '', + type: 'divider', + }); - items.push({ - text: t('panel.header-menu.remove', `Remove`), - iconClassName: 'trash-alt', - onClick: () => { - DashboardInteractions.panelMenuItemClicked('remove'); - onRemovePanel(dashboard, panel); - }, - shortcut: 'p r', - }); + items.push({ + text: t('panel.header-menu.remove', `Remove`), + iconClassName: 'trash-alt', + onClick: () => { + DashboardInteractions.panelMenuItemClicked('remove'); + onRemovePanel(dashboard, panel); + }, + shortcut: 'p r', + }); + } menu.setState({ items }); }; diff --git a/public/app/features/dashboard-scene/scene/keyboardShortcuts.ts b/public/app/features/dashboard-scene/scene/keyboardShortcuts.ts index e1bb7aea75d..99123eaf9ca 100644 --- a/public/app/features/dashboard-scene/scene/keyboardShortcuts.ts +++ b/public/app/features/dashboard-scene/scene/keyboardShortcuts.ts @@ -120,7 +120,9 @@ export function setupKeyboardShortcuts(scene: DashboardScene) { keybindings.addBinding({ key: 'p r', onTrigger: withFocusedPanel(scene, (vizPanel: VizPanel) => { - onRemovePanel(scene, vizPanel); + if (scene.state.isEditing) { + onRemovePanel(scene, vizPanel); + } }), }); @@ -128,7 +130,9 @@ export function setupKeyboardShortcuts(scene: DashboardScene) { keybindings.addBinding({ key: 'p d', onTrigger: withFocusedPanel(scene, (vizPanel: VizPanel) => { - scene.duplicatePanel(vizPanel); + if (scene.state.isEditing) { + scene.duplicatePanel(vizPanel); + } }), });