From 7311b14da1f6ed6815fb47504f96027277523756 Mon Sep 17 00:00:00 2001 From: ryan Date: Tue, 12 Mar 2019 16:10:14 -0700 Subject: [PATCH 1/7] API to fix/update properties before load --- packages/grafana-ui/src/types/panel.ts | 13 +++++++++++++ .../dashboard/dashgrid/DashboardPanel.tsx | 14 +++++++++++++- public/app/plugins/panel/text2/module.tsx | 15 +++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/grafana-ui/src/types/panel.ts b/packages/grafana-ui/src/types/panel.ts index 260ff78df76..da0d399fe65 100644 --- a/packages/grafana-ui/src/types/panel.ts +++ b/packages/grafana-ui/src/types/panel.ts @@ -26,9 +26,18 @@ export interface PanelEditorProps { onOptionsChange: (options: T) => void; } +/** + * Checks the existing model before the component is loaded + * This is useful for fixing options as configuration changes + * The object passed in is the panel model.... but not typed + * since that is not in grafana ui + */ +export type PanelOptionsValidator = (panelModel: any) => T; + export class ReactPanelPlugin { panel: ComponentClass>; editor?: ComponentClass>; + optionsValidator?: PanelOptionsValidator; defaults?: TOptions; constructor(panel: ComponentClass>) { @@ -39,6 +48,10 @@ export class ReactPanelPlugin { this.editor = editor; } + setOptionsValidator(validator: PanelOptionsValidator) { + this.optionsValidator = validator; + } + setDefaults(defaults: TOptions) { this.defaults = defaults; } diff --git a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx index 9aeddd5a0d9..ef2edd0a13a 100644 --- a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx @@ -91,6 +91,14 @@ export class DashboardPanel extends PureComponent { this.setState({ plugin, angularPanel: null }); } + + // Clean the options when switching plugins + // ??? is there a better way that will make sure to call componentDidUpdate ??? + // The panel constructor may have already run + const { reactPanel } = plugin.exports; + if (reactPanel && reactPanel.optionsValidator) { + panel.options = reactPanel.optionsValidator(panel); + } } } @@ -98,8 +106,12 @@ export class DashboardPanel extends PureComponent { this.loadPlugin(this.props.panel.type); } - componentDidUpdate() { + componentDidUpdate(prevProps: Props, prevState: State) { if (!this.element || this.state.angularPanel) { + const { plugin } = this.state; + if (plugin && plugin !== prevState.plugin) { + console.log('PLUGIN Changed', plugin); + } return; } diff --git a/public/app/plugins/panel/text2/module.tsx b/public/app/plugins/panel/text2/module.tsx index bac292e8c29..ae94e16a766 100644 --- a/public/app/plugins/panel/text2/module.tsx +++ b/public/app/plugins/panel/text2/module.tsx @@ -3,8 +3,23 @@ import { ReactPanelPlugin } from '@grafana/ui'; import { TextPanelEditor } from './TextPanelEditor'; import { TextPanel } from './TextPanel'; import { TextOptions, defaults } from './types'; +import { PanelModel } from 'app/features/dashboard/state'; + +import get from 'lodash/get'; export const reactPanel = new ReactPanelPlugin(TextPanel); +const validator = (model: PanelModel): TextOptions => { + const options = model.options as TextOptions; + if (!options) { + const old = get(model, 'cachedPluginOptions.text'); + if (old) { + return old; + } + } + return options; +}; + reactPanel.setEditor(TextPanelEditor); reactPanel.setDefaults(defaults); +reactPanel.setOptionsValidator(validator); From 09ba8cd5d36debac5061a5fa3ecfe180b244b0e4 Mon Sep 17 00:00:00 2001 From: ryan Date: Tue, 12 Mar 2019 16:35:22 -0700 Subject: [PATCH 2/7] make sure the validator is called before setState --- .../dashboard/dashgrid/DashboardPanel.tsx | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx index ef2edd0a13a..fb2b2ab6471 100644 --- a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx @@ -81,37 +81,35 @@ export class DashboardPanel extends PureComponent { } if (plugin.exports) { + this.validateOptions(plugin, panel); this.setState({ plugin, angularPanel: null }); } else { try { plugin.exports = await importPluginModule(plugin.module); + this.validateOptions(plugin, panel); } catch (e) { plugin = getPanelPluginNotFound(pluginId); } this.setState({ plugin, angularPanel: null }); } - - // Clean the options when switching plugins - // ??? is there a better way that will make sure to call componentDidUpdate ??? - // The panel constructor may have already run - const { reactPanel } = plugin.exports; - if (reactPanel && reactPanel.optionsValidator) { - panel.options = reactPanel.optionsValidator(panel); - } } } + // This is be called before the plugin constructor, so the initial properties are valid + validateOptions = (plugin: PanelPlugin, panel: PanelModel) => { + const { reactPanel } = plugin.exports; + if (reactPanel && reactPanel.optionsValidator) { + panel.options = reactPanel.optionsValidator(panel); + } + }; + componentDidMount() { this.loadPlugin(this.props.panel.type); } - componentDidUpdate(prevProps: Props, prevState: State) { + componentDidUpdate() { if (!this.element || this.state.angularPanel) { - const { plugin } = this.state; - if (plugin && plugin !== prevState.plugin) { - console.log('PLUGIN Changed', plugin); - } return; } From 8cca61172717d44bd716fd4a05b14cf881d6fe77 Mon Sep 17 00:00:00 2001 From: ryan Date: Tue, 12 Mar 2019 21:25:35 -0700 Subject: [PATCH 3/7] comment cleanup --- packages/grafana-ui/src/types/panel.ts | 13 +++++++++---- .../features/dashboard/dashgrid/DashboardPanel.tsx | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/grafana-ui/src/types/panel.ts b/packages/grafana-ui/src/types/panel.ts index da0d399fe65..d66fa601a29 100644 --- a/packages/grafana-ui/src/types/panel.ts +++ b/packages/grafana-ui/src/types/panel.ts @@ -27,10 +27,15 @@ export interface PanelEditorProps { } /** - * Checks the existing model before the component is loaded - * This is useful for fixing options as configuration changes - * The object passed in is the panel model.... but not typed - * since that is not in grafana ui + * This function is called with the full panelModel before + * the pluginPanel is constructed. This gives you an opportunity + * to validate the panel settings before the panel loads. + * + * @param panelModel the whole panel object. including the configuration + * saved for other panels + * + * @returns the validated panel options that will be passed into the + * panel constructor */ export type PanelOptionsValidator = (panelModel: any) => T; diff --git a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx index fb2b2ab6471..faefa97a74e 100644 --- a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx @@ -96,7 +96,8 @@ export class DashboardPanel extends PureComponent { } } - // This is be called before the plugin constructor, so the initial properties are valid + // This is called before the plugin is added to the three, + // it allows plugins to update options before loading validateOptions = (plugin: PanelPlugin, panel: PanelModel) => { const { reactPanel } = plugin.exports; if (reactPanel && reactPanel.optionsValidator) { From 15c111b39d66e9136c6d74a074b6e18bb58b5c4d Mon Sep 17 00:00:00 2001 From: ryan Date: Tue, 12 Mar 2019 21:30:48 -0700 Subject: [PATCH 4/7] better comments --- public/app/plugins/panel/text2/module.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/public/app/plugins/panel/text2/module.tsx b/public/app/plugins/panel/text2/module.tsx index ae94e16a766..f2cf52fbd75 100644 --- a/public/app/plugins/panel/text2/module.tsx +++ b/public/app/plugins/panel/text2/module.tsx @@ -6,16 +6,15 @@ import { TextOptions, defaults } from './types'; import { PanelModel } from 'app/features/dashboard/state'; import get from 'lodash/get'; +import cloneDeep from 'lodash/cloneDeep'; export const reactPanel = new ReactPanelPlugin(TextPanel); const validator = (model: PanelModel): TextOptions => { const options = model.options as TextOptions; if (!options) { - const old = get(model, 'cachedPluginOptions.text'); - if (old) { - return old; - } + // Use the same settings from an existing 'text' panel + return cloneDeep(get(model, 'cachedPluginOptions.text')); } return options; }; From 75022ebd6317b1cd9fd05a642468c17065179a92 Mon Sep 17 00:00:00 2001 From: ryan Date: Wed, 13 Mar 2019 12:46:04 -0700 Subject: [PATCH 5/7] single hook --- packages/grafana-ui/src/types/panel.ts | 34 ++++++++----------- .../dashboard/dashgrid/DashboardPanel.tsx | 5 +-- .../features/dashboard/state/PanelModel.ts | 10 +++--- public/app/plugins/panel/bargauge/module.tsx | 4 +-- public/app/plugins/panel/gauge/module.tsx | 4 +-- public/app/plugins/panel/text2/module.tsx | 21 ++++-------- 6 files changed, 29 insertions(+), 49 deletions(-) diff --git a/packages/grafana-ui/src/types/panel.ts b/packages/grafana-ui/src/types/panel.ts index 0b9e5ab4b90..2ac38b6253a 100644 --- a/packages/grafana-ui/src/types/panel.ts +++ b/packages/grafana-ui/src/types/panel.ts @@ -27,26 +27,20 @@ export interface PanelEditorProps { } /** - * This function is called with the full panelModel before - * the pluginPanel is constructed. This gives you an opportunity - * to validate the panel settings before the panel loads. - * - * @param panelModel the whole panel object. including the configuration - * saved for other panels - * - * @returns the validated panel options that will be passed into the - * panel constructor + * Called before a panel is initalized */ -export type PanelOptionsValidator = (panelModel: any) => T; - -export type PreservePanelOptionsHandler = (pluginId: string, prevOptions: any) => Partial; +export type PanelTypeChangedHook = ( + options: TOptions, + prevPluginId?: string, + prevOptions?: any +) => TOptions; export class ReactPanelPlugin { panel: ComponentClass>; editor?: ComponentClass>; - optionsValidator?: PanelOptionsValidator; defaults?: TOptions; - preserveOptions?: PreservePanelOptionsHandler; + + panelTypeChangedHook?: PanelTypeChangedHook; constructor(panel: ComponentClass>) { this.panel = panel; @@ -56,16 +50,16 @@ export class ReactPanelPlugin { this.editor = editor; } - setOptionsValidator(validator: PanelOptionsValidator) { - this.optionsValidator = validator; - } - setDefaults(defaults: TOptions) { this.defaults = defaults; } - setPreserveOptionsHandler(handler: PreservePanelOptionsHandler) { - this.preserveOptions = handler; + /** + * Called when the visualization changes. + * Lets you keep whatever settings made sense in the previous panel + */ + setPanelTypeChangedHook(v: PanelTypeChangedHook) { + this.panelTypeChangedHook = v; } } diff --git a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx index c111349c6fa..d5dba7cfe06 100644 --- a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx @@ -92,10 +92,7 @@ export class DashboardPanel extends PureComponent { this.props.panel.changeType(pluginId); } else { const { reactPanel } = plugin.exports; - panel.changeType(pluginId, reactPanel.preserveOptions); - if (reactPanel && reactPanel.optionsValidator) { - panel.options = reactPanel.optionsValidator(panel); - } + panel.changeType(pluginId, reactPanel.panelTypeChangedHook); } } diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index 88065fdf208..ddbd2f79440 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -3,7 +3,7 @@ import _ from 'lodash'; // Types import { Emitter } from 'app/core/utils/emitter'; -import { DataQuery, TimeSeries, Threshold, ScopedVars } from '@grafana/ui'; +import { DataQuery, TimeSeries, Threshold, ScopedVars, PanelTypeChangedHook } from '@grafana/ui'; import { TableData } from '@grafana/ui/src'; export interface GridPos { @@ -237,7 +237,7 @@ export class PanelModel { }); } - changeType(pluginId: string, preserveOptions?: any) { + changeType(pluginId: string, hook?: PanelTypeChangedHook) { const oldOptions: any = this.getOptionsToRemember(); const oldPluginId = this.type; @@ -255,9 +255,9 @@ export class PanelModel { this.cachedPluginOptions[oldPluginId] = oldOptions; this.restorePanelOptions(pluginId); - if (preserveOptions && oldOptions) { - this.options = this.options || {}; - Object.assign(this.options, preserveOptions(oldPluginId, oldOptions.options)); + // Callback that can validate and migrate any existing settings + if (hook) { + Object.assign(this.options, hook(this.options || {}, oldPluginId, oldOptions.options)); } } diff --git a/public/app/plugins/panel/bargauge/module.tsx b/public/app/plugins/panel/bargauge/module.tsx index d5dcbabc34f..14fc7696d25 100644 --- a/public/app/plugins/panel/bargauge/module.tsx +++ b/public/app/plugins/panel/bargauge/module.tsx @@ -8,9 +8,7 @@ export const reactPanel = new ReactPanelPlugin(BarGaugePanel); reactPanel.setEditor(BarGaugePanelEditor); reactPanel.setDefaults(defaults); -reactPanel.setPreserveOptionsHandler((pluginId: string, prevOptions: any) => { - const options: Partial = {}; - +reactPanel.setPanelTypeChangedHook((options: BarGaugeOptions, prevPluginId: string, prevOptions: any) => { if (prevOptions.valueOptions) { options.valueOptions = prevOptions.valueOptions; options.thresholds = prevOptions.thresholds; diff --git a/public/app/plugins/panel/gauge/module.tsx b/public/app/plugins/panel/gauge/module.tsx index 95a6e29ae4d..8d179f7b403 100644 --- a/public/app/plugins/panel/gauge/module.tsx +++ b/public/app/plugins/panel/gauge/module.tsx @@ -8,9 +8,7 @@ export const reactPanel = new ReactPanelPlugin(GaugePanel); reactPanel.setEditor(GaugePanelEditor); reactPanel.setDefaults(defaults); -reactPanel.setPreserveOptionsHandler((pluginId: string, prevOptions: any) => { - const options: Partial = {}; - +reactPanel.setPanelTypeChangedHook((options: GaugeOptions, prevPluginId: string, prevOptions: any) => { if (prevOptions.valueOptions) { options.valueOptions = prevOptions.valueOptions; options.thresholds = prevOptions.thresholds; diff --git a/public/app/plugins/panel/text2/module.tsx b/public/app/plugins/panel/text2/module.tsx index f2cf52fbd75..b2e3057de34 100644 --- a/public/app/plugins/panel/text2/module.tsx +++ b/public/app/plugins/panel/text2/module.tsx @@ -3,22 +3,15 @@ import { ReactPanelPlugin } from '@grafana/ui'; import { TextPanelEditor } from './TextPanelEditor'; import { TextPanel } from './TextPanel'; import { TextOptions, defaults } from './types'; -import { PanelModel } from 'app/features/dashboard/state'; - -import get from 'lodash/get'; -import cloneDeep from 'lodash/cloneDeep'; export const reactPanel = new ReactPanelPlugin(TextPanel); -const validator = (model: PanelModel): TextOptions => { - const options = model.options as TextOptions; - if (!options) { - // Use the same settings from an existing 'text' panel - return cloneDeep(get(model, 'cachedPluginOptions.text')); - } - return options; -}; - reactPanel.setEditor(TextPanelEditor); reactPanel.setDefaults(defaults); -reactPanel.setOptionsValidator(validator); +reactPanel.setPanelTypeChangedHook((options: TextOptions, prevPluginId: string, prevOptions: any) => { + if (prevPluginId === 'text') { + return prevOptions as TextOptions; + } + + return options; +}); From 0c086795e3fb854098cae0552be29d739df83e41 Mon Sep 17 00:00:00 2001 From: ryan Date: Wed, 13 Mar 2019 13:07:01 -0700 Subject: [PATCH 6/7] check types better --- public/app/features/dashboard/state/PanelModel.ts | 5 ++++- public/app/plugins/panel/bargauge/module.tsx | 6 ++++-- public/app/plugins/panel/gauge/module.tsx | 6 ++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index ddbd2f79440..128bd8d0785 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -257,7 +257,10 @@ export class PanelModel { // Callback that can validate and migrate any existing settings if (hook) { - Object.assign(this.options, hook(this.options || {}, oldPluginId, oldOptions.options)); + this.options = this.options || {}; + const old = oldOptions ? oldOptions.options : null; + + Object.assign(this.options, hook(this.options, oldPluginId, old)); } } diff --git a/public/app/plugins/panel/bargauge/module.tsx b/public/app/plugins/panel/bargauge/module.tsx index 14fc7696d25..af757f756ac 100644 --- a/public/app/plugins/panel/bargauge/module.tsx +++ b/public/app/plugins/panel/bargauge/module.tsx @@ -8,8 +8,10 @@ export const reactPanel = new ReactPanelPlugin(BarGaugePanel); reactPanel.setEditor(BarGaugePanelEditor); reactPanel.setDefaults(defaults); -reactPanel.setPanelTypeChangedHook((options: BarGaugeOptions, prevPluginId: string, prevOptions: any) => { - if (prevOptions.valueOptions) { +reactPanel.setPanelTypeChangedHook((options: BarGaugeOptions, prevPluginId?: string, prevOptions?: any) => { + console.log('BAR Gauge', options, prevPluginId, prevOptions); + + if (prevOptions && prevOptions.valueOptions) { options.valueOptions = prevOptions.valueOptions; options.thresholds = prevOptions.thresholds; options.maxValue = prevOptions.maxValue; diff --git a/public/app/plugins/panel/gauge/module.tsx b/public/app/plugins/panel/gauge/module.tsx index 8d179f7b403..b9124aea8b9 100644 --- a/public/app/plugins/panel/gauge/module.tsx +++ b/public/app/plugins/panel/gauge/module.tsx @@ -8,8 +8,10 @@ export const reactPanel = new ReactPanelPlugin(GaugePanel); reactPanel.setEditor(GaugePanelEditor); reactPanel.setDefaults(defaults); -reactPanel.setPanelTypeChangedHook((options: GaugeOptions, prevPluginId: string, prevOptions: any) => { - if (prevOptions.valueOptions) { +reactPanel.setPanelTypeChangedHook((options: GaugeOptions, prevPluginId?: string, prevOptions?: any) => { + console.log('BAR Gauge', options, prevPluginId, prevOptions); + + if (prevOptions && prevOptions.valueOptions) { options.valueOptions = prevOptions.valueOptions; options.thresholds = prevOptions.thresholds; options.maxValue = prevOptions.maxValue; From 68f7e046abde326ca4516e5d5228db3f2c66896f Mon Sep 17 00:00:00 2001 From: ryan Date: Wed, 13 Mar 2019 13:11:49 -0700 Subject: [PATCH 7/7] check types better --- public/app/features/dashboard/dashgrid/DashboardPanel.tsx | 8 ++++++-- public/app/plugins/panel/bargauge/module.tsx | 2 -- public/app/plugins/panel/gauge/module.tsx | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx index d5dba7cfe06..029fb0e8371 100644 --- a/public/app/features/dashboard/dashgrid/DashboardPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardPanel.tsx @@ -14,6 +14,7 @@ import { PanelEditor } from '../panel_editor/PanelEditor'; import { PanelModel, DashboardModel } from '../state'; import { PanelPlugin } from 'app/types'; import { PanelResizer } from './PanelResizer'; +import { PanelTypeChangedHook } from '@grafana/ui'; export interface Props { panel: PanelModel; @@ -91,8 +92,11 @@ export class DashboardPanel extends PureComponent { this.props.panel.changeType(pluginId); } else { - const { reactPanel } = plugin.exports; - panel.changeType(pluginId, reactPanel.panelTypeChangedHook); + let hook: PanelTypeChangedHook | null = null; + if (plugin.exports.reactPanel) { + hook = plugin.exports.reactPanel.panelTypeChangedHook; + } + panel.changeType(pluginId, hook); } } diff --git a/public/app/plugins/panel/bargauge/module.tsx b/public/app/plugins/panel/bargauge/module.tsx index af757f756ac..e7f2e2d7738 100644 --- a/public/app/plugins/panel/bargauge/module.tsx +++ b/public/app/plugins/panel/bargauge/module.tsx @@ -9,8 +9,6 @@ export const reactPanel = new ReactPanelPlugin(BarGaugePanel); reactPanel.setEditor(BarGaugePanelEditor); reactPanel.setDefaults(defaults); reactPanel.setPanelTypeChangedHook((options: BarGaugeOptions, prevPluginId?: string, prevOptions?: any) => { - console.log('BAR Gauge', options, prevPluginId, prevOptions); - if (prevOptions && prevOptions.valueOptions) { options.valueOptions = prevOptions.valueOptions; options.thresholds = prevOptions.thresholds; diff --git a/public/app/plugins/panel/gauge/module.tsx b/public/app/plugins/panel/gauge/module.tsx index b9124aea8b9..b8e90a27bff 100644 --- a/public/app/plugins/panel/gauge/module.tsx +++ b/public/app/plugins/panel/gauge/module.tsx @@ -9,8 +9,6 @@ export const reactPanel = new ReactPanelPlugin(GaugePanel); reactPanel.setEditor(GaugePanelEditor); reactPanel.setDefaults(defaults); reactPanel.setPanelTypeChangedHook((options: GaugeOptions, prevPluginId?: string, prevOptions?: any) => { - console.log('BAR Gauge', options, prevPluginId, prevOptions); - if (prevOptions && prevOptions.valueOptions) { options.valueOptions = prevOptions.valueOptions; options.thresholds = prevOptions.thresholds;