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

packages/grafana-data/src/types/legacyEvents.ts

@@ -22,7 +22,6 @@ export const PanelEvents = {
   dataSnapshotLoad: eventFactory<DataQueryResponseData[]>('data-snapshot-load'),
   editModeInitialized: eventFactory('init-edit-mode'),
   initPanelActions: eventFactory<AngularPanelMenuItem[]>('init-panel-actions'),
-  panelSizeChanged: eventFactory('panel-size-changed'),
   panelTeardown: eventFactory('panel-teardown'),
   render: eventFactory<any>('render'),
 };

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() {

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<Props, State> {
-  formattedJson: any;
+  private formattedJson: any;
-  clipboard: any;
+  private subs = new Subscription();
-  subscription?: Unsubscribable;
 
   constructor(props: Props) {
     super(props);
@@ -58,11 +58,15 @@ export class QueryInspector extends PureComponent<Props, State> {
   }
 
   componentDidMount() {
-    this.subscription = backendSrv.getInspectorStream().subscribe({
+    const { panel } = this.props;
-      next: response => this.onDataSourceResponse(response),
-    });
 
-    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<Props, State> {
   };
 
   componentWillUnmount() {
-    const { panel } = this.props;
+    this.subs.unsubscribe();
-
-    if (this.subscription) {
-      this.subscription.unsubscribe();
-    }
-
-    panel.events.off(PanelEvents.refresh, this.onPanelRefresh);
   }
 
   onPanelRefresh = () => {

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<Props> {
     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<Props> {
 
   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<Props> {
         isResizable={dashboard.meta.canEdit}
         isDraggable={dashboard.meta.canEdit}
         onLayoutChange={this.onLayoutChange}
-        onWidthChange={this.onWidthChange}
         onDragStop={this.onDragStop}
         onResize={this.onResize}
         onResizeStop={this.onResizeStop}

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<Props, State> {
-  readonly timeSrv: TimeSrv = getTimeSrv();
+  private readonly timeSrv: TimeSrv = getTimeSrv();
-
+  private subs = new Subscription();
-  querySubscription?: Unsubscribable;
 
   constructor(props: Props) {
     super(props);
@@ -75,8 +74,8 @@ export class PanelChrome extends Component<Props, State> {
     const { panel, dashboard } = this.props;
 
     // Subscribe to panel events
-    panel.events.on(PanelEvents.refresh, this.onRefresh);
+    this.subs.add(panel.events.subscribe(RefreshEvent, this.onRefresh));
-    panel.events.on(PanelEvents.render, this.onRender);
+    this.subs.add(panel.events.subscribe(RenderEvent, this.onRender));
 
     dashboard.panelInitialized(this.props.panel);
 
@@ -93,21 +92,18 @@ export class PanelChrome extends Component<Props, State> {
       this.setState({ isFirstLoad: false });
     }
 
-    this.querySubscription = panel
+    this.subs.add(
-      .getQueryRunner()
+      panel
-      .getData({ withTransforms: true, withFieldConfig: true })
+        .getQueryRunner()
-      .subscribe({
+        .getData({ withTransforms: true, withFieldConfig: true })
-        next: data => this.onDataUpdate(data),
+        .subscribe({
-      });
+          next: data => this.onDataUpdate(data),
+        })
+    );
   }
 
   componentWillUnmount() {
-    this.props.panel.events.off(PanelEvents.refresh, this.onRefresh);
+    this.subs.unsubscribe();
-    this.props.panel.events.off(PanelEvents.render, this.onRender);
-
-    if (this.querySubscription) {
-      this.querySubscription.unsubscribe();
-    }
   }
 
   componentDidUpdate(prevProps: Props) {

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<Props, State> {
   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<Props, State> {
     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({
+    this.subs.add(
-      next: (data: PanelData) => this.onPanelDataUpdate(data),
+      queryRunner.getData({ withTransforms: false, withFieldConfig: false }).subscribe({
-    });
+        next: (data: PanelData) => this.onPanelDataUpdate(data),
-  }
+      })
+    );
 
-  subscribeToRenderEvent() {
+    this.subs.add(panel.events.subscribe(RenderEvent, this.onPanelRenderEvent));
-    // 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);
   }
 
   onPanelRenderEvent = (payload?: any) => {
@@ -126,12 +124,7 @@ export class PanelChromeAngularUnconnected extends PureComponent<Props, State> {
 
   componentWillUnmount() {
     this.cleanUpAngularPanel();
-
+    this.subs.unsubscribe();
-    if (this.querySubscription) {
-      this.querySubscription.unsubscribe();
-    }
-
-    this.props.panel.events.off(PanelEvents.render, this.onPanelRenderEvent);
   }
 
   componentDidUpdate(prevProps: Props, prevState: State) {
@@ -146,7 +139,7 @@ export class PanelChromeAngularUnconnected extends PureComponent<Props, State> {
       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<Props, State> {
       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() {

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}
 >
   <div

public/app/features/dashboard/state/DashboardModel.ts

@@ -16,7 +16,6 @@ import {
   DateTimeInput,
   EventBusExtended,
   EventBusSrv,
-  PanelEvents,
   TimeRange,
   TimeZone,
   UrlQueryValue,
@@ -27,7 +26,7 @@ import { variableAdapters } from 'app/features/variables/adapters';
 import { onTimeRangeUpdated } from 'app/features/variables/state/actions';
 import { dispatch } from '../../../store/store';
 import { isAllVariable } from '../../variables/utils';
-import { DashboardPanelsChangedEvent } from 'app/types/events';
+import { DashboardPanelsChangedEvent, RefreshEvent, RenderEvent } from 'app/types/events';
 
 export interface CloneOptions {
   saveVariables?: boolean;
@@ -266,7 +265,7 @@ export class DashboardModel {
   }
 
   startRefresh() {
-    this.events.emit(PanelEvents.refresh);
+    this.events.publish(new RefreshEvent());
 
     if (this.panelInEdit) {
       this.panelInEdit.refresh();
@@ -281,8 +280,7 @@ export class DashboardModel {
   }
 
   render() {
-    this.events.emit(PanelEvents.render);
+    this.events.publish(new RenderEvent());
-
     for (const panel of this.panels) {
       panel.render();
     }
@@ -884,11 +882,15 @@ export class DashboardModel {
     return rowPanels;
   }
 
+  /** @deprecated */
   on<T>(event: AppEvent<T>, callback: (payload?: T) => void) {
+    console.log('DashboardModel.on is deprecated use events.subscribe');
     this.events.on(event, callback);
   }
 
+  /** @deprecated */
   off<T>(event: AppEvent<T>, callback: (payload?: T) => void) {
+    console.log('DashboardModel.off is deprecated');
     this.events.off(event, callback);
   }
 

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());
     }
   }
 

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];

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() {
+      function updateDimensionsFromParentScope() {
-        $timeout(() => {
-          resizeScrollableContent();
-          ctrl.render();
-        });
-      }
-
-      function onPanelModelRender(payload?: any) {
         ctrl.height = scope.$parent.$parent.size.height;
         ctrl.width = scope.$parent.$parent.size.width;
       }
 
-      function onPanelModelRefresh() {
+      // Pass PanelModel events down to angular controller event emitter
-        ctrl.height = scope.$parent.$parent.size.height;
+      subs.add(
-        ctrl.width = scope.$parent.$parent.size.width;
+        panel.events.subscribe(RefreshEvent, () => {
-      }
+          updateDimensionsFromParentScope();
+          ctrl.events.emit('refresh');
+        })
+      );
 
-      panel.events.on(PanelEvents.refresh, onPanelModelRefresh);
+      subs.add(
-      panel.events.on(PanelEvents.render, onPanelModelRender);
+        panel.events.subscribe(RenderEvent, () => {
-      panel.events.on(PanelEvents.panelSizeChanged, onPanelSizeChanged);
+          updateDimensionsFromParentScope();
+
+          $timeout(() => {
+            resizeScrollableContent();
+            ctrl.events.emit('render');
+          });
+        })
+      );
 
       scope.$on('$destroy', () => {
         elem.off();
 
-        panel.events.emit(PanelEvents.panelTeardown);
+        // Remove PanelModel.event subs
-        panel.events.removeAllListeners();
+        subs.unsubscribe();
+        // Remove Angular controller event subs
+        ctrl.events.emit(PanelEvents.panelTeardown);
+        ctrl.events.removeAllListeners();
 
         if (panelScrollbar) {
           panelScrollbar.dispose();

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);
           });
         });
       });

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';
+}