From 3111f23bf150b5e411393c19ed92322483b15c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 6 Sep 2024 16:00:59 +0200 Subject: [PATCH] ViewPanel: Refactoring to simplify logic by rendering the source panel instead of a clone (#93000) * ViewPanel: Refactoring to simplify logic by rendering the source panel instead of a clone * Moved util function to utils * Update --- .../scene/DashboardGridItem.tsx | 3 +- .../scene/DashboardSceneUrlSync.test.ts | 32 +------- .../scene/DashboardSceneUrlSync.ts | 17 +---- .../scene/ViewPanelScene.test.tsx | 35 ++++----- .../dashboard-scene/scene/ViewPanelScene.tsx | 76 ++----------------- .../features/dashboard-scene/utils/utils.ts | 30 +++++--- 6 files changed, 43 insertions(+), 150 deletions(-) diff --git a/public/app/features/dashboard-scene/scene/DashboardGridItem.tsx b/public/app/features/dashboard-scene/scene/DashboardGridItem.tsx index 9e69fc477d9..97e44cef66b 100644 --- a/public/app/features/dashboard-scene/scene/DashboardGridItem.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardGridItem.tsx @@ -58,7 +58,6 @@ export class DashboardGridItem extends SceneObjectBase i this._prevRepeatValues = undefined; } - this._oldBody = this.state.body; this.performRepeat(); } } @@ -117,7 +116,9 @@ export class DashboardGridItem extends SceneObjectBase i return; } + this._oldBody = this.state.body; this._prevRepeatValues = values; + const panelToRepeat = this.state.body; const repeatedPanels: VizPanel[] = []; diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts index 276c82e9d5c..c1a7172044f 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts +++ b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts @@ -1,11 +1,10 @@ import { AppEvents } from '@grafana/data'; -import { SceneGridLayout, SceneGridRow, SceneQueryRunner, VizPanel } from '@grafana/scenes'; +import { SceneGridLayout, SceneQueryRunner, VizPanel } from '@grafana/scenes'; import appEvents from 'app/core/app_events'; import { KioskMode } from 'app/types'; import { DashboardGridItem } from './DashboardGridItem'; import { DashboardScene } from './DashboardScene'; -import { RowRepeaterBehavior } from './RowRepeaterBehavior'; import { DashboardRepeatsProcessedEvent } from './types'; describe('DashboardSceneUrlSync', () => { @@ -109,35 +108,6 @@ describe('DashboardSceneUrlSync', () => { scene.publishEvent(new DashboardRepeatsProcessedEvent({ source: scene })); expect(scene.state.viewPanelScene?.getUrlKey()).toBe('panel-1-clone-1'); }); - - it('should subscribe and update view panel if panel is in a repeated row', () => { - const scene = buildTestScene(); - - // fake adding row panel - const layout = scene.state.body as SceneGridLayout; - layout.setState({ - children: [ - new SceneGridRow({ - $behaviors: [new RowRepeaterBehavior({ variableName: 'test' })], - children: [ - new VizPanel({ - title: 'Panel A', - key: 'panel-1', - pluginId: 'table', - }), - ], - }), - ], - }); - - scene.urlSync?.updateFromUrl({ viewPanel: 'panel-1' }); - - expect(scene.state.viewPanelScene?.getUrlKey()).toBeUndefined(); - - // Verify it subscribes to DashboardRepeatsProcessedEvent - scene.publishEvent(new DashboardRepeatsProcessedEvent({ source: scene })); - expect(scene.state.viewPanelScene?.getUrlKey()).toBe('panel-1'); - }); }); function buildTestScene() { diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts index ec868f4bf7b..53067b92a34 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts +++ b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts @@ -18,13 +18,7 @@ import { buildPanelEditScene } from '../panel-edit/PanelEditor'; import { createDashboardEditViewFor } from '../settings/utils'; import { ShareDrawer } from '../sharing/ShareDrawer/ShareDrawer'; import { ShareModal } from '../sharing/ShareModal'; -import { - findVizPanelByKey, - getDashboardSceneFor, - getLibraryPanelBehavior, - isPanelClone, - isWithinUnactivatedRepeatRow, -} from '../utils/utils'; +import { findVizPanelByKey, getDashboardSceneFor, getLibraryPanelBehavior, isPanelClone } from '../utils/utils'; import { DashboardScene, DashboardSceneState } from './DashboardScene'; import { LibraryPanelBehavior } from './LibraryPanelBehavior'; @@ -108,11 +102,6 @@ export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { return; } - if (isWithinUnactivatedRepeatRow(panel)) { - this._handleViewRepeatClone(values.viewPanel); - return; - } - update.viewPanelScene = new ViewPanelScene({ panelRef: panel.getRef() }); } else if (viewPanelScene && values.viewPanel === null) { update.viewPanelScene = undefined; @@ -236,10 +225,6 @@ class ResolveInspectPanelByKey extends SceneObjectBase ({ + ...jest.requireActual('@grafana/runtime'), + getPluginImportUtils: () => ({ + getPanelPluginFromCache: jest.fn(() => {}), + }), +})); + describe('ViewPanelScene', () => { - it('Should build scene on activate', () => { - const { viewPanelScene } = buildScene(); + it('Should activate panel parents', () => { + const { viewPanelScene, dashboard } = buildScene(); viewPanelScene.activate(); - expect(viewPanelScene.state.body).toBeDefined(); - }); - - it('Should look copy row variable scope', () => { - const { viewPanelScene } = buildScene({ rowVariables: true, panelVariables: true }); - viewPanelScene.activate(); - - const variables = viewPanelScene.state.body?.state.$variables; - expect(variables?.state.variables.length).toBe(2); + expect(viewPanelScene.state.panelRef.resolve().isActive).toBe(true); + expect(dashboard.state.body.isActive).toBe(true); }); }); @@ -29,11 +29,6 @@ function buildScene(options?: SceneOptions) { // builds a scene how it looks like after row and panel repeats are processed const panel = new VizPanel({ key: 'panel-22', - $variables: options?.panelVariables - ? new SceneVariableSet({ - variables: [new LocalValueVariable({ value: 'panel-var-value' })], - }) - : undefined, }); const dashboard = new DashboardScene({ @@ -43,11 +38,9 @@ function buildScene(options?: SceneOptions) { x: 0, y: 10, width: 24, - $variables: options?.rowVariables - ? new SceneVariableSet({ - variables: [new LocalValueVariable({ value: 'row-var-value' })], - }) - : undefined, + $variables: new SceneVariableSet({ + variables: [new LocalValueVariable({ value: 'row-var-value' })], + }), height: 1, children: [ new DashboardGridItem({ diff --git a/public/app/features/dashboard-scene/scene/ViewPanelScene.tsx b/public/app/features/dashboard-scene/scene/ViewPanelScene.tsx index 33885113f0a..969ed5f1358 100644 --- a/public/app/features/dashboard-scene/scene/ViewPanelScene.tsx +++ b/public/app/features/dashboard-scene/scene/ViewPanelScene.tsx @@ -1,20 +1,9 @@ -import { - SceneComponentProps, - SceneObjectBase, - SceneObjectRef, - SceneObjectState, - VizPanel, - sceneUtils, - SceneVariables, - SceneGridRow, - sceneGraph, - SceneVariableSet, - SceneVariable, -} from '@grafana/scenes'; +import { SceneComponentProps, SceneObjectBase, SceneObjectRef, SceneObjectState, VizPanel } from '@grafana/scenes'; + +import { activateInActiveParents } from '../utils/utils'; interface ViewPanelSceneState extends SceneObjectState { panelRef: SceneObjectRef; - body?: VizPanel; } export class ViewPanelScene extends SceneObjectBase { @@ -26,47 +15,7 @@ export class ViewPanelScene extends SceneObjectBase { public _activationHandler() { const panel = this.state.panelRef.resolve(); - const panelState = sceneUtils.cloneSceneObjectState(panel.state, { - key: panel.state.key + '-view', - $variables: this.getScopedVariables(panel), - }); - - const body = new VizPanel(panelState); - - this.setState({ body }); - - return () => { - // Make sure we preserve data state - if (body.state.$data) { - panel.setState({ $data: body.state.$data.clone() }); - } - }; - } - - // In case the panel is inside a repeated row - private getScopedVariables(panel: VizPanel): SceneVariables | undefined { - const row = tryGetParentRow(panel); - const variables: SceneVariable[] = []; - - // Because we are rendering the panel outside it's potential row context we need to copy the row (scoped) varables - if (row && row.state.$variables) { - for (const variable of row.state.$variables.state.variables) { - variables.push(variable.clone()); - } - } - - // If we have local scoped panel variables we need to add the row variables to it - if (panel.state.$variables) { - for (const variable of panel.state.$variables.state.variables) { - variables.push(variable.clone()); - } - } - - if (variables.length > 0) { - return new SceneVariableSet({ variables }); - } - - return undefined; + return activateInActiveParents(panel); } public getUrlKey() { @@ -74,20 +23,9 @@ export class ViewPanelScene extends SceneObjectBase { } public static Component = ({ model }: SceneComponentProps) => { - const { body } = model.useState(); + const { panelRef } = model.useState(); + const panel = panelRef.resolve(); - if (!body) { - return null; - } - - return ; + return ; }; } - -function tryGetParentRow(panel: VizPanel): SceneGridRow | undefined { - try { - return sceneGraph.getAncestor(panel, SceneGridRow); - } catch { - return undefined; - } -} diff --git a/public/app/features/dashboard-scene/utils/utils.ts b/public/app/features/dashboard-scene/utils/utils.ts index f80a8ee67af..004e152bc4c 100644 --- a/public/app/features/dashboard-scene/utils/utils.ts +++ b/public/app/features/dashboard-scene/utils/utils.ts @@ -1,6 +1,7 @@ import { getDataSourceRef, IntervalVariableModel } from '@grafana/data'; import { getDataSourceSrv } from '@grafana/runtime'; import { + CancelActivationHandler, CustomVariable, MultiValueVariable, SceneDataTransformer, @@ -18,7 +19,6 @@ import { DashboardScene } from '../scene/DashboardScene'; import { LibraryPanelBehavior } from '../scene/LibraryPanelBehavior'; import { VizPanelLinks, VizPanelLinksMenu } from '../scene/PanelLinks'; import { panelMenuBehavior } from '../scene/PanelMenuBehavior'; -import { RowRepeaterBehavior } from '../scene/RowRepeaterBehavior'; import { RowActions } from '../scene/row-actions/RowActions'; import { dashboardSceneGraph } from './dashboardSceneGraph'; @@ -267,20 +267,26 @@ export function getLibraryPanelBehavior(vizPanel: VizPanel): LibraryPanelBehavio } /** - * If the panel is within a repeated row, it must wait until the row resolves the variables. - * This ensures that the scoped variable for the row is assigned and the panel is initialized with them. + * Activates any inactive parents of the scene object. + * Useful when rendering a scene object out of context of it's parent + * @returns */ -export function isWithinUnactivatedRepeatRow(panel: VizPanel): boolean { - let row; +export function activateInActiveParents(so: SceneObject): CancelActivationHandler | undefined { + let cancel: CancelActivationHandler | undefined; + let parentCancel: CancelActivationHandler | undefined; - try { - row = sceneGraph.getAncestor(panel, SceneGridRow); - } catch (err) { - return false; + if (so.isActive) { + return cancel; } - const hasBehavior = !!(row.state.$behaviors && row.state.$behaviors.find((b) => b instanceof RowRepeaterBehavior)); - const hasVariables = !!(row.state.$variables && row.state.$variables.state.variables.length > 0); + if (so.parent) { + parentCancel = activateInActiveParents(so.parent); + } - return hasBehavior && !hasVariables; + cancel = so.activate(); + + return () => { + parentCancel?.(); + cancel(); + }; }