From e375fb0f2b05e614087ac0c5189e73236405985a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 23 Dec 2020 10:45:31 +0100 Subject: [PATCH] PanelEvents: Isolating angular panel events into it's own event bus + more event refactoring (#29904) * PanelEvents: Isolate angular events from panel model events, use new Event patterns * Removing some events that are not longer needed * Updated DashboardModel * Update * Review fixes --- .../grafana-data/src/types/legacyEvents.ts | 1 - .../features/annotations/annotations_srv.ts | 4 +- .../components/Inspector/QueryInspector.tsx | 30 ++++++------- .../dashboard/dashgrid/DashboardGrid.tsx | 11 ----- .../dashboard/dashgrid/PanelChrome.tsx | 34 +++++++-------- .../dashboard/dashgrid/PanelChromeAngular.tsx | 34 ++++++--------- .../__snapshots__/DashboardGrid.test.tsx.snap | 1 - .../dashboard/state/DashboardModel.ts | 12 +++--- .../features/dashboard/state/PanelModel.ts | 26 ++++------- public/app/features/panel/panel_ctrl.ts | 11 ++++- public/app/features/panel/panel_directive.ts | 43 +++++++++++-------- .../app/features/plugins/plugin_component.ts | 4 +- public/app/types/events.ts | 8 ++++ 13 files changed, 104 insertions(+), 115 deletions(-) diff --git a/packages/grafana-data/src/types/legacyEvents.ts b/packages/grafana-data/src/types/legacyEvents.ts index a0040e67d57..868830de83c 100644 --- a/packages/grafana-data/src/types/legacyEvents.ts +++ b/packages/grafana-data/src/types/legacyEvents.ts @@ -22,7 +22,6 @@ export const PanelEvents = { dataSnapshotLoad: eventFactory('data-snapshot-load'), editModeInitialized: eventFactory('init-edit-mode'), initPanelActions: eventFactory('init-panel-actions'), - panelSizeChanged: eventFactory('panel-size-changed'), panelTeardown: eventFactory('panel-teardown'), render: eventFactory('render'), }; diff --git a/public/app/features/annotations/annotations_srv.ts b/public/app/features/annotations/annotations_srv.ts index 3a8588503ac..d5c34c08d92 100644 --- a/public/app/features/annotations/annotations_srv.ts +++ b/public/app/features/annotations/annotations_srv.ts @@ -14,7 +14,6 @@ import { CoreApp, DataQueryRequest, DataSourceApi, - PanelEvents, rangeUtil, ScopedVars, } from '@grafana/data'; @@ -26,6 +25,7 @@ import { map, mergeMap } from 'rxjs/operators'; import { AnnotationQueryOptions, AnnotationQueryResponse } from './types'; import { standardAnnotationSupport } from './standardAnnotationSupport'; import { runRequest } from '../query/state/runRequest'; +import { RefreshEvent } from 'app/types/events'; let counter = 100; function getNextRequestId() { @@ -41,7 +41,7 @@ export class AnnotationsSrv { // always clearPromiseCaches when loading new dashboard this.clearPromiseCaches(); // clear promises on refresh events - dashboard.on(PanelEvents.refresh, this.clearPromiseCaches.bind(this)); + dashboard.events.subscribe(RefreshEvent, this.clearPromiseCaches.bind(this)); } clearPromiseCaches() { diff --git a/public/app/features/dashboard/components/Inspector/QueryInspector.tsx b/public/app/features/dashboard/components/Inspector/QueryInspector.tsx index e68a1e441b7..74ac26b9474 100644 --- a/public/app/features/dashboard/components/Inspector/QueryInspector.tsx +++ b/public/app/features/dashboard/components/Inspector/QueryInspector.tsx @@ -1,7 +1,7 @@ import React, { PureComponent } from 'react'; import { Button, JSONFormatter, LoadingPlaceholder } from '@grafana/ui'; import { selectors } from '@grafana/e2e-selectors'; -import { AppEvents, PanelEvents, DataFrame } from '@grafana/data'; +import { AppEvents, DataFrame } from '@grafana/data'; import appEvents from 'app/core/app_events'; import { CopyToClipboard } from 'app/core/components/CopyToClipboard/CopyToClipboard'; @@ -10,8 +10,9 @@ import { getPanelInspectorStyles } from './styles'; import { supportsDataQuery } from '../PanelEditor/utils'; import { config } from '@grafana/runtime'; import { css } from 'emotion'; -import { Unsubscribable } from 'rxjs'; +import { Subscription } from 'rxjs'; import { backendSrv } from 'app/core/services/backend_srv'; +import { RefreshEvent } from 'app/types/events'; interface DsQuery { isLoading: boolean; @@ -39,9 +40,8 @@ interface State { } export class QueryInspector extends PureComponent { - formattedJson: any; - clipboard: any; - subscription?: Unsubscribable; + private formattedJson: any; + private subs = new Subscription(); constructor(props: Props) { super(props); @@ -58,11 +58,15 @@ export class QueryInspector extends PureComponent { } componentDidMount() { - this.subscription = backendSrv.getInspectorStream().subscribe({ - next: response => this.onDataSourceResponse(response), - }); + const { panel } = this.props; - this.props.panel.events.on(PanelEvents.refresh, this.onPanelRefresh); + this.subs.add( + backendSrv.getInspectorStream().subscribe({ + next: response => this.onDataSourceResponse(response), + }) + ); + + this.subs.add(panel.events.subscribe(RefreshEvent, this.onPanelRefresh)); this.updateQueryList(); } @@ -112,13 +116,7 @@ export class QueryInspector extends PureComponent { }; componentWillUnmount() { - const { panel } = this.props; - - if (this.subscription) { - this.subscription.unsubscribe(); - } - - panel.events.off(PanelEvents.refresh, this.onPanelRefresh); + this.subs.unsubscribe(); } onPanelRefresh = () => { diff --git a/public/app/features/dashboard/dashgrid/DashboardGrid.tsx b/public/app/features/dashboard/dashgrid/DashboardGrid.tsx index e36d56060cc..ecdcbcdf4f9 100644 --- a/public/app/features/dashboard/dashgrid/DashboardGrid.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardGrid.tsx @@ -28,7 +28,6 @@ interface GridWrapperProps { onDragStop: ItemCallback; onResize: ItemCallback; onResizeStop: ItemCallback; - onWidthChange: () => void; className: string; isResizable?: boolean; isDraggable?: boolean; @@ -43,7 +42,6 @@ function GridWrapper({ onDragStop, onResize, onResizeStop, - onWidthChange, className, isResizable, isDraggable, @@ -56,7 +54,6 @@ function GridWrapper({ if (ignoreNextWidthChange) { ignoreNextWidthChange = false; } else if (!viewPanel && Math.abs(width - lastGridWidth) > 8) { - onWidthChange(); lastGridWidth = width; } } @@ -164,12 +161,6 @@ export class DashboardGrid extends PureComponent { this.forceUpdate(); }; - onWidthChange = () => { - for (const panel of this.props.dashboard.panels) { - panel.resizeDone(); - } - }; - updateGridPos = (item: ReactGridLayout.Layout, layout: ReactGridLayout.Layout[]) => { this.panelMap[item.i!].updateGridPos(item); @@ -184,7 +175,6 @@ export class DashboardGrid extends PureComponent { onResizeStop: ItemCallback = (layout, oldItem, newItem) => { this.updateGridPos(newItem, layout); - this.panelMap[newItem.i!].resizeDone(); }; onDragStop: ItemCallback = (layout, oldItem, newItem) => { @@ -277,7 +267,6 @@ export class DashboardGrid extends PureComponent { isResizable={dashboard.meta.canEdit} isDraggable={dashboard.meta.canEdit} onLayoutChange={this.onLayoutChange} - onWidthChange={this.onWidthChange} onDragStop={this.onDragStop} onResize={this.onResize} onResizeStop={this.onResizeStop} diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index a625e429ab8..f2d979a8007 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -1,7 +1,7 @@ // Libraries import React, { Component } from 'react'; import classNames from 'classnames'; -import { Unsubscribable } from 'rxjs'; +import { Subscription } from 'rxjs'; // Components import { PanelHeader } from './PanelHeader/PanelHeader'; import { ErrorBoundary } from '@grafana/ui'; @@ -20,7 +20,6 @@ import { DefaultTimeRange, toUtc, toDataFrameDTO, - PanelEvents, PanelData, PanelPlugin, FieldConfigSource, @@ -28,6 +27,7 @@ import { } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { loadSnapshotData } from '../utils/loadSnapshotData'; +import { RefreshEvent, RenderEvent } from 'app/types/events'; const DEFAULT_PLUGIN_ERROR = 'Error in plugin'; @@ -52,9 +52,8 @@ export interface State { } export class PanelChrome extends Component { - readonly timeSrv: TimeSrv = getTimeSrv(); - - querySubscription?: Unsubscribable; + private readonly timeSrv: TimeSrv = getTimeSrv(); + private subs = new Subscription(); constructor(props: Props) { super(props); @@ -75,8 +74,8 @@ export class PanelChrome extends Component { const { panel, dashboard } = this.props; // Subscribe to panel events - panel.events.on(PanelEvents.refresh, this.onRefresh); - panel.events.on(PanelEvents.render, this.onRender); + this.subs.add(panel.events.subscribe(RefreshEvent, this.onRefresh)); + this.subs.add(panel.events.subscribe(RenderEvent, this.onRender)); dashboard.panelInitialized(this.props.panel); @@ -93,21 +92,18 @@ export class PanelChrome extends Component { this.setState({ isFirstLoad: false }); } - this.querySubscription = panel - .getQueryRunner() - .getData({ withTransforms: true, withFieldConfig: true }) - .subscribe({ - next: data => this.onDataUpdate(data), - }); + this.subs.add( + panel + .getQueryRunner() + .getData({ withTransforms: true, withFieldConfig: true }) + .subscribe({ + next: data => this.onDataUpdate(data), + }) + ); } componentWillUnmount() { - this.props.panel.events.off(PanelEvents.refresh, this.onRefresh); - this.props.panel.events.off(PanelEvents.render, this.onRender); - - if (this.querySubscription) { - this.querySubscription.unsubscribe(); - } + this.subs.unsubscribe(); } componentDidUpdate(prevProps: Props) { diff --git a/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx b/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx index 2eb10235bf4..709043836d3 100644 --- a/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx @@ -1,7 +1,7 @@ // Libraries import React, { PureComponent } from 'react'; import classNames from 'classnames'; -import { Unsubscribable } from 'rxjs'; +import { Subscription } from 'rxjs'; import { connect, MapDispatchToProps, MapStateToProps } from 'react-redux'; // Components import { PanelHeader } from './PanelHeader/PanelHeader'; @@ -13,10 +13,11 @@ import config from 'app/core/config'; // Types import { DashboardModel, PanelModel } from '../state'; import { StoreState } from 'app/types'; -import { DefaultTimeRange, LoadingState, PanelData, PanelEvents, PanelPlugin } from '@grafana/data'; +import { DefaultTimeRange, LoadingState, PanelData, PanelPlugin } from '@grafana/data'; import { updateLocation } from 'app/core/actions'; import { PANEL_BORDER } from 'app/core/constants'; import { selectors } from '@grafana/e2e-selectors'; +import { RenderEvent } from 'app/types/events'; interface OwnProps { panel: PanelModel; @@ -59,7 +60,7 @@ export class PanelChromeAngularUnconnected extends PureComponent { element: HTMLElement | null = null; timeSrv: TimeSrv = getTimeSrv(); scopeProps?: AngularScopeProps; - querySubscription: Unsubscribable; + subs = new Subscription(); constructor(props: Props) { super(props); @@ -80,16 +81,13 @@ export class PanelChromeAngularUnconnected extends PureComponent { const queryRunner = panel.getQueryRunner(); // we are not displaying any of this data so no need for transforms or field config - this.querySubscription = queryRunner.getData({ withTransforms: false, withFieldConfig: false }).subscribe({ - next: (data: PanelData) => this.onPanelDataUpdate(data), - }); - } + this.subs.add( + queryRunner.getData({ withTransforms: false, withFieldConfig: false }).subscribe({ + next: (data: PanelData) => this.onPanelDataUpdate(data), + }) + ); - subscribeToRenderEvent() { - // Subscribe to render event, this is as far as I know only needed for changes to title & transparent - // These changes are modified in the model and only way to communicate that change is via this event - // Need to find another solution for this in tthe future (panel title in redux?) - this.props.panel.events.on(PanelEvents.render, this.onPanelRenderEvent); + this.subs.add(panel.events.subscribe(RenderEvent, this.onPanelRenderEvent)); } onPanelRenderEvent = (payload?: any) => { @@ -126,12 +124,7 @@ export class PanelChromeAngularUnconnected extends PureComponent { componentWillUnmount() { this.cleanUpAngularPanel(); - - if (this.querySubscription) { - this.querySubscription.unsubscribe(); - } - - this.props.panel.events.off(PanelEvents.render, this.onPanelRenderEvent); + this.subs.unsubscribe(); } componentDidUpdate(prevProps: Props, prevState: State) { @@ -146,7 +139,7 @@ export class PanelChromeAngularUnconnected extends PureComponent { if (this.scopeProps) { this.scopeProps.size.height = this.getInnerPanelHeight(); this.scopeProps.size.width = this.getInnerPanelWidth(); - panel.events.emit(PanelEvents.panelSizeChanged); + panel.render(); } } } @@ -189,9 +182,6 @@ export class PanelChromeAngularUnconnected extends PureComponent { panelId: panel.id, angularComponent: loader.load(this.element, this.scopeProps, template), }); - - // need to to this every time we load an angular as all events are unsubscribed when panel is destroyed - this.subscribeToRenderEvent(); } cleanUpAngularPanel() { diff --git a/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap b/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap index bd55179e3ac..0a8084cac8e 100644 --- a/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap +++ b/public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap @@ -41,7 +41,6 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = ` onLayoutChange={[Function]} onResize={[Function]} onResizeStop={[Function]} - onWidthChange={[Function]} viewPanel={null} >
(event: AppEvent, callback: (payload?: T) => void) { + console.log('DashboardModel.on is deprecated use events.subscribe'); this.events.on(event, callback); } + /** @deprecated */ off(event: AppEvent, callback: (payload?: T) => void) { + console.log('DashboardModel.off is deprecated'); this.events.off(event, callback); } diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index ca5872e4a4a..ed6abe527f9 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -28,7 +28,13 @@ import { import { EDIT_PANEL_ID } from 'app/core/constants'; import config from 'app/core/config'; import { PanelQueryRunner } from '../../query/state/PanelQueryRunner'; -import { PanelOptionsChangedEvent, PanelQueriesChangedEvent, PanelTransformationsChangedEvent } from 'app/types/events'; +import { + PanelOptionsChangedEvent, + PanelQueriesChangedEvent, + PanelTransformationsChangedEvent, + RefreshEvent, + RenderEvent, +} from 'app/types/events'; import { getTimeSrv } from '../services/TimeSrv'; import { getAllVariableValuesForUrl } from '../../variables/getAllVariableValuesForUrl'; @@ -258,36 +264,22 @@ export class PanelModel implements DataConfigSource { } updateGridPos(newPos: GridPos) { - let sizeChanged = false; - - if (this.gridPos.w !== newPos.w || this.gridPos.h !== newPos.h) { - sizeChanged = true; - } - this.gridPos.x = newPos.x; this.gridPos.y = newPos.y; this.gridPos.w = newPos.w; this.gridPos.h = newPos.h; - - if (sizeChanged) { - this.events.emit(PanelEvents.panelSizeChanged); - } - } - - resizeDone() { - this.events.emit(PanelEvents.panelSizeChanged); } refresh() { this.hasRefreshed = true; - this.events.emit(PanelEvents.refresh); + this.events.publish(new RefreshEvent()); } render() { if (!this.hasRefreshed) { this.refresh(); } else { - this.events.emit(PanelEvents.render); + this.events.publish(new RenderEvent()); } } diff --git a/public/app/features/panel/panel_ctrl.ts b/public/app/features/panel/panel_ctrl.ts index 97c48ca392b..1942fb80a51 100644 --- a/public/app/features/panel/panel_ctrl.ts +++ b/public/app/features/panel/panel_ctrl.ts @@ -2,7 +2,14 @@ import _ from 'lodash'; import config from 'app/core/config'; import { profiler } from 'app/core/core'; import { auto } from 'angular'; -import { AppEvent, PanelEvents, PanelPluginMeta, AngularPanelMenuItem, EventBusExtended } from '@grafana/data'; +import { + AppEvent, + PanelEvents, + PanelPluginMeta, + AngularPanelMenuItem, + EventBusExtended, + EventBusSrv, +} from '@grafana/data'; import { DashboardModel } from '../dashboard/state'; export class PanelCtrl { @@ -30,7 +37,7 @@ export class PanelCtrl { this.$scope = $scope; this.$timeout = $injector.get('$timeout'); this.editorTabs = []; - this.events = this.panel.events; + this.events = new EventBusSrv(); this.timing = {}; // not used but here to not break plugins const plugin = config.panels[this.panel.type]; diff --git a/public/app/features/panel/panel_directive.ts b/public/app/features/panel/panel_directive.ts index 07b46b76ee0..0a8d070a247 100644 --- a/public/app/features/panel/panel_directive.ts +++ b/public/app/features/panel/panel_directive.ts @@ -4,6 +4,8 @@ import baron from 'baron'; import { PanelEvents } from '@grafana/data'; import { PanelModel } from '../dashboard/state'; import { PanelCtrl } from './panel_ctrl'; +import { Subscription } from 'rxjs'; +import { RefreshEvent, RenderEvent } from 'app/types/events'; const module = angular.module('grafana.directives'); @@ -20,6 +22,7 @@ module.directive('grafanaPanel', ($rootScope, $document, $timeout) => { link: (scope: any, elem) => { const ctrl: PanelCtrl = scope.ctrl; const panel: PanelModel = scope.ctrl.panel; + const subs = new Subscription(); let panelScrollbar: any; @@ -58,32 +61,38 @@ module.directive('grafanaPanel', ($rootScope, $document, $timeout) => { } }); - function onPanelSizeChanged() { - $timeout(() => { - resizeScrollableContent(); - ctrl.render(); - }); - } - - function onPanelModelRender(payload?: any) { + function updateDimensionsFromParentScope() { ctrl.height = scope.$parent.$parent.size.height; ctrl.width = scope.$parent.$parent.size.width; } - function onPanelModelRefresh() { - ctrl.height = scope.$parent.$parent.size.height; - ctrl.width = scope.$parent.$parent.size.width; - } + // Pass PanelModel events down to angular controller event emitter + subs.add( + panel.events.subscribe(RefreshEvent, () => { + updateDimensionsFromParentScope(); + ctrl.events.emit('refresh'); + }) + ); - panel.events.on(PanelEvents.refresh, onPanelModelRefresh); - panel.events.on(PanelEvents.render, onPanelModelRender); - panel.events.on(PanelEvents.panelSizeChanged, onPanelSizeChanged); + subs.add( + panel.events.subscribe(RenderEvent, () => { + updateDimensionsFromParentScope(); + + $timeout(() => { + resizeScrollableContent(); + ctrl.events.emit('render'); + }); + }) + ); scope.$on('$destroy', () => { elem.off(); - panel.events.emit(PanelEvents.panelTeardown); - panel.events.removeAllListeners(); + // Remove PanelModel.event subs + subs.unsubscribe(); + // Remove Angular controller event subs + ctrl.events.emit(PanelEvents.panelTeardown); + ctrl.events.removeAllListeners(); if (panelScrollbar) { panelScrollbar.dispose(); diff --git a/public/app/features/plugins/plugin_component.ts b/public/app/features/plugins/plugin_component.ts index 043f5638275..b3240e8787a 100644 --- a/public/app/features/plugins/plugin_component.ts +++ b/public/app/features/plugins/plugin_component.ts @@ -4,7 +4,7 @@ import _ from 'lodash'; import config from 'app/core/config'; import coreModule from 'app/core/core_module'; -import { DataSourceApi } from '@grafana/data'; +import { DataSourceApi, PanelEvents } from '@grafana/data'; import { importPanelPlugin, importDataSourcePlugin, importAppPlugin } from './plugin_loader'; import DatasourceSrv from './datasource_srv'; import { GrafanaRootScope } from 'app/routes/GrafanaCtrl'; @@ -229,7 +229,7 @@ function pluginDirectiveLoader( elem.append(child); setTimeout(() => { scope.$applyAsync(() => { - scope.$broadcast('component-did-mount'); + scope.$broadcast(PanelEvents.componentDidMount.name); }); }); }); diff --git a/public/app/types/events.ts b/public/app/types/events.ts index c1b402c0cd3..ee323736397 100644 --- a/public/app/types/events.ts +++ b/public/app/types/events.ts @@ -148,3 +148,11 @@ export class PanelOptionsChangedEvent extends BusEventBase { export class DashboardPanelsChangedEvent extends BusEventBase { static type = 'dashboard-panels-changed'; } + +export class RefreshEvent extends BusEventBase { + static type = 'refresh'; +} + +export class RenderEvent extends BusEventBase { + static type = 'render'; +}