From f95359fb344ca94545d9d234cf7c43c289448358 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Wed, 19 Dec 2018 12:58:10 +0100 Subject: [PATCH 1/9] Notify user on query error --- .../features/dashboard/dashgrid/DataPanel.tsx | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/DataPanel.tsx b/public/app/features/dashboard/dashgrid/DataPanel.tsx index 9926410f40d..72f405dcbe7 100644 --- a/public/app/features/dashboard/dashgrid/DataPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DataPanel.tsx @@ -1,5 +1,6 @@ // Library import React, { Component } from 'react'; +import Tooltip from 'app/core/components/Tooltip/Tooltip'; // Services import { getDatasourceSrv, DatasourceSrv } from 'app/features/plugins/datasource_srv'; @@ -138,7 +139,7 @@ export class DataPanel extends Component { const timeSeries = response.data; if (isFirstLoad && loading === LoadingState.Loading) { - return this.renderLoadingSpinner(); + return this.renderLoadingState(); } if (!queries.length) { @@ -151,7 +152,7 @@ export class DataPanel extends Component { return ( <> - {this.renderLoadingSpinner()} + {this.renderLoadingState()} {this.props.children({ timeSeries, loading, @@ -160,15 +161,26 @@ export class DataPanel extends Component { ); } - private renderLoadingSpinner(): JSX.Element { + private renderLoadingState(): JSX.Element { const { loading } = this.state; - if (loading === LoadingState.Loading) { return (
); + } else if (loading === LoadingState.Error) { + return ( + + + + + ); } return null; From 5143a2669dcf7a118bd0283f6ba0ce310a927711 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Mon, 7 Jan 2019 13:05:27 +0100 Subject: [PATCH 2/9] feat: Add "theme" to Tooltip --- public/app/core/components/Tooltip/Popper.tsx | 12 +++++++-- .../core/components/Tooltip/withPopper.tsx | 3 ++- .../features/dashboard/dashgrid/DataPanel.tsx | 8 +++--- public/sass/_variables.dark.scss | 6 +++-- public/sass/_variables.light.scss | 6 +++-- public/sass/components/_popper.scss | 26 ++++++++++++------- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/public/app/core/components/Tooltip/Popper.tsx b/public/app/core/components/Tooltip/Popper.tsx index 36cf0fe837e..d5ea1d93091 100644 --- a/public/app/core/components/Tooltip/Popper.tsx +++ b/public/app/core/components/Tooltip/Popper.tsx @@ -21,13 +21,21 @@ interface Props { placement?: any; content: string | ((props: any) => JSX.Element); refClassName?: string; + theme?: string; +} + +export enum Themes { + Default = 'popper__background--default', + Error = 'popper__background--error', } class Popper extends PureComponent { render() { - const { children, renderContent, show, placement, refClassName } = this.props; + const { children, renderContent, show, placement, refClassName, theme } = this.props; const { content } = this.props; + const popperBackgroundClassName = 'popper__background' + (theme ? ' ' + theme : ''); + return ( @@ -53,7 +61,7 @@ class Popper extends PureComponent { data-placement={placement} className="popper" > -
+
{renderContent(content)}
diff --git a/public/app/core/components/Tooltip/withPopper.tsx b/public/app/core/components/Tooltip/withPopper.tsx index 4ba05937531..7be12748247 100644 --- a/public/app/core/components/Tooltip/withPopper.tsx +++ b/public/app/core/components/Tooltip/withPopper.tsx @@ -9,6 +9,7 @@ export interface UsingPopperProps { content: string | ((props: any) => JSX.Element); className?: string; refClassName?: string; + theme?: string; } interface Props { @@ -16,6 +17,7 @@ interface Props { className?: string; refClassName?: string; content: string | ((props: any) => JSX.Element); + theme?: string; } interface State { @@ -71,7 +73,6 @@ export default function withPopper(WrappedComponent) { render() { const { show, placement } = this.state; const className = this.props.className || ''; - return ( { const timeSeries = response.data; if (isFirstLoad && loading === LoadingState.Loading) { - return this.renderLoadingState(); + return this.renderLoadingStates(); } if (!queries.length) { @@ -152,7 +153,7 @@ export class DataPanel extends Component { return ( <> - {this.renderLoadingState()} + {this.renderLoadingStates()} {this.props.children({ timeSeries, loading, @@ -161,7 +162,7 @@ export class DataPanel extends Component { ); } - private renderLoadingState(): JSX.Element { + private renderLoadingStates(): JSX.Element { const { loading } = this.state; if (loading === LoadingState.Loading) { return ( @@ -176,6 +177,7 @@ export class DataPanel extends Component { className="popper__manager--block" refClassName={`panel-info-corner panel-info-corner--error`} placement="bottom-start" + theme={Themes.Error} > diff --git a/public/sass/_variables.dark.scss b/public/sass/_variables.dark.scss index 70db51a0fb2..8769164d5ec 100644 --- a/public/sass/_variables.dark.scss +++ b/public/sass/_variables.dark.scss @@ -302,12 +302,14 @@ $popover-error-bg: $btn-danger-bg; // Tooltips and popovers // ------------------------- $tooltipColor: $popover-help-color; -$tooltipBackground: $popover-help-bg; $tooltipArrowWidth: 5px; -$tooltipArrowColor: $tooltipBackground; $tooltipLinkColor: $link-color; $graph-tooltip-bg: $dark-1; +$tooltipBackground: $popover-help-bg; +$tooltipArrowColor: $tooltipBackground; +$tooltipBackgroundError: $brand-danger; + // images $checkboxImageUrl: '../img/checkbox.png'; diff --git a/public/sass/_variables.light.scss b/public/sass/_variables.light.scss index 6afd087a849..be2d05e2441 100644 --- a/public/sass/_variables.light.scss +++ b/public/sass/_variables.light.scss @@ -307,12 +307,14 @@ $popover-error-bg: $btn-danger-bg; // Tooltips and popovers // ------------------------- $tooltipColor: $popover-help-color; -$tooltipBackground: $popover-help-bg; $tooltipArrowWidth: 5px; -$tooltipArrowColor: $tooltipBackground; $tooltipLinkColor: lighten($popover-help-color, 5%); $graph-tooltip-bg: $gray-5; +$tooltipBackground: $popover-help-bg; +$tooltipArrowColor: $tooltipBackground; // Used by Angular tooltip +$tooltipBackgroundError: $brand-danger; + // images $checkboxImageUrl: '../img/checkbox_white.png'; diff --git a/public/sass/components/_popper.scss b/public/sass/components/_popper.scss index d869d52b92f..afa629d4043 100644 --- a/public/sass/components/_popper.scss +++ b/public/sass/components/_popper.scss @@ -8,7 +8,22 @@ $popper-margin-from-ref: 5px; text-align: center; } -.popper .popper__arrow { +.popper__background { + background: $tooltipBackground; + border-radius: $border-radius; + box-shadow: 0 0 2px rgba(0, 0, 0, 0.5); + padding: 10px; + + // Themes + &.popper__background--error { + background: $tooltipBackgroundError; + .popper__arrow { + border-color: $tooltipBackgroundError; + } + } +} + +.popper__arrow { width: 0; height: 0; border-style: solid; @@ -16,17 +31,10 @@ $popper-margin-from-ref: 5px; margin: 0px; } -.popper .popper__arrow { +.popper__arrow { border-color: $tooltipBackground; } -.popper__background { - background: $tooltipBackground; - border-radius: $border-radius; - box-shadow: 0 0 2px rgba(0, 0, 0, 0.5); - padding: 10px; -} - // Top .popper[data-placement^='top'] { padding-bottom: $popper-margin-from-ref; From dadbaccfeb0bef811129fd07acfcba286832cd3b Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Wed, 19 Dec 2018 12:58:10 +0100 Subject: [PATCH 3/9] Notify user on query error --- .../features/dashboard/dashgrid/DataPanel.tsx | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/DataPanel.tsx b/public/app/features/dashboard/dashgrid/DataPanel.tsx index 30a939b50aa..7d752da95a8 100644 --- a/public/app/features/dashboard/dashgrid/DataPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DataPanel.tsx @@ -1,5 +1,6 @@ // Library import React, { Component } from 'react'; +import Tooltip from 'app/core/components/Tooltip/Tooltip'; // Services import { getDatasourceSrv, DatasourceSrv } from 'app/features/plugins/datasource_srv'; @@ -139,7 +140,7 @@ export class DataPanel extends Component { const timeSeries = response.data; if (isFirstLoad && loading === LoadingState.Loading) { - return this.renderLoadingSpinner(); + return this.renderLoadingState(); } if (!queries.length) { @@ -152,7 +153,7 @@ export class DataPanel extends Component { return ( <> - {this.renderLoadingSpinner()} + {this.renderLoadingState()} {this.props.children({ timeSeries, loading, @@ -161,15 +162,26 @@ export class DataPanel extends Component { ); } - private renderLoadingSpinner(): JSX.Element { + private renderLoadingState(): JSX.Element { const { loading } = this.state; - if (loading === LoadingState.Loading) { return (
); + } else if (loading === LoadingState.Error) { + return ( + + + + + ); } return null; From 79fd8a7eda2e53685f52129175ce15e35af273fe Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Mon, 7 Jan 2019 13:05:27 +0100 Subject: [PATCH 4/9] feat: Add "theme" to Tooltip --- public/app/core/components/Tooltip/Popper.tsx | 12 +++++++-- .../core/components/Tooltip/withPopper.tsx | 3 ++- .../features/dashboard/dashgrid/DataPanel.tsx | 8 +++--- public/sass/_variables.dark.scss | 6 +++-- public/sass/_variables.light.scss | 6 +++-- public/sass/components/_popper.scss | 26 ++++++++++++------- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/public/app/core/components/Tooltip/Popper.tsx b/public/app/core/components/Tooltip/Popper.tsx index 36cf0fe837e..d5ea1d93091 100644 --- a/public/app/core/components/Tooltip/Popper.tsx +++ b/public/app/core/components/Tooltip/Popper.tsx @@ -21,13 +21,21 @@ interface Props { placement?: any; content: string | ((props: any) => JSX.Element); refClassName?: string; + theme?: string; +} + +export enum Themes { + Default = 'popper__background--default', + Error = 'popper__background--error', } class Popper extends PureComponent { render() { - const { children, renderContent, show, placement, refClassName } = this.props; + const { children, renderContent, show, placement, refClassName, theme } = this.props; const { content } = this.props; + const popperBackgroundClassName = 'popper__background' + (theme ? ' ' + theme : ''); + return ( @@ -53,7 +61,7 @@ class Popper extends PureComponent { data-placement={placement} className="popper" > -
+
{renderContent(content)}
diff --git a/public/app/core/components/Tooltip/withPopper.tsx b/public/app/core/components/Tooltip/withPopper.tsx index 4ba05937531..7be12748247 100644 --- a/public/app/core/components/Tooltip/withPopper.tsx +++ b/public/app/core/components/Tooltip/withPopper.tsx @@ -9,6 +9,7 @@ export interface UsingPopperProps { content: string | ((props: any) => JSX.Element); className?: string; refClassName?: string; + theme?: string; } interface Props { @@ -16,6 +17,7 @@ interface Props { className?: string; refClassName?: string; content: string | ((props: any) => JSX.Element); + theme?: string; } interface State { @@ -71,7 +73,6 @@ export default function withPopper(WrappedComponent) { render() { const { show, placement } = this.state; const className = this.props.className || ''; - return ( { const timeSeries = response.data; if (isFirstLoad && loading === LoadingState.Loading) { - return this.renderLoadingState(); + return this.renderLoadingStates(); } if (!queries.length) { @@ -153,7 +154,7 @@ export class DataPanel extends Component { return ( <> - {this.renderLoadingState()} + {this.renderLoadingStates()} {this.props.children({ timeSeries, loading, @@ -162,7 +163,7 @@ export class DataPanel extends Component { ); } - private renderLoadingState(): JSX.Element { + private renderLoadingStates(): JSX.Element { const { loading } = this.state; if (loading === LoadingState.Loading) { return ( @@ -177,6 +178,7 @@ export class DataPanel extends Component { className="popper__manager--block" refClassName={`panel-info-corner panel-info-corner--error`} placement="bottom-start" + theme={Themes.Error} > diff --git a/public/sass/_variables.dark.scss b/public/sass/_variables.dark.scss index 70db51a0fb2..8769164d5ec 100644 --- a/public/sass/_variables.dark.scss +++ b/public/sass/_variables.dark.scss @@ -302,12 +302,14 @@ $popover-error-bg: $btn-danger-bg; // Tooltips and popovers // ------------------------- $tooltipColor: $popover-help-color; -$tooltipBackground: $popover-help-bg; $tooltipArrowWidth: 5px; -$tooltipArrowColor: $tooltipBackground; $tooltipLinkColor: $link-color; $graph-tooltip-bg: $dark-1; +$tooltipBackground: $popover-help-bg; +$tooltipArrowColor: $tooltipBackground; +$tooltipBackgroundError: $brand-danger; + // images $checkboxImageUrl: '../img/checkbox.png'; diff --git a/public/sass/_variables.light.scss b/public/sass/_variables.light.scss index 6afd087a849..be2d05e2441 100644 --- a/public/sass/_variables.light.scss +++ b/public/sass/_variables.light.scss @@ -307,12 +307,14 @@ $popover-error-bg: $btn-danger-bg; // Tooltips and popovers // ------------------------- $tooltipColor: $popover-help-color; -$tooltipBackground: $popover-help-bg; $tooltipArrowWidth: 5px; -$tooltipArrowColor: $tooltipBackground; $tooltipLinkColor: lighten($popover-help-color, 5%); $graph-tooltip-bg: $gray-5; +$tooltipBackground: $popover-help-bg; +$tooltipArrowColor: $tooltipBackground; // Used by Angular tooltip +$tooltipBackgroundError: $brand-danger; + // images $checkboxImageUrl: '../img/checkbox_white.png'; diff --git a/public/sass/components/_popper.scss b/public/sass/components/_popper.scss index d869d52b92f..afa629d4043 100644 --- a/public/sass/components/_popper.scss +++ b/public/sass/components/_popper.scss @@ -8,7 +8,22 @@ $popper-margin-from-ref: 5px; text-align: center; } -.popper .popper__arrow { +.popper__background { + background: $tooltipBackground; + border-radius: $border-radius; + box-shadow: 0 0 2px rgba(0, 0, 0, 0.5); + padding: 10px; + + // Themes + &.popper__background--error { + background: $tooltipBackgroundError; + .popper__arrow { + border-color: $tooltipBackgroundError; + } + } +} + +.popper__arrow { width: 0; height: 0; border-style: solid; @@ -16,17 +31,10 @@ $popper-margin-from-ref: 5px; margin: 0px; } -.popper .popper__arrow { +.popper__arrow { border-color: $tooltipBackground; } -.popper__background { - background: $tooltipBackground; - border-radius: $border-radius; - box-shadow: 0 0 2px rgba(0, 0, 0, 0.5); - padding: 10px; -} - // Top .popper[data-placement^='top'] { padding-bottom: $popper-margin-from-ref; From 0b89f81609fadab4d60aa0b47ca9c8a8f9124f55 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Mon, 7 Jan 2019 13:56:36 +0100 Subject: [PATCH 5/9] fix: Light theme corner bg color update --- public/sass/_variables.dark.scss | 1 + public/sass/_variables.light.scss | 1 + public/sass/pages/_dashboard.scss | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/public/sass/_variables.dark.scss b/public/sass/_variables.dark.scss index 8769164d5ec..ded17e6ecfe 100644 --- a/public/sass/_variables.dark.scss +++ b/public/sass/_variables.dark.scss @@ -103,6 +103,7 @@ $panel-bg: #212124; $panel-border-color: $dark-1; $panel-border: solid 1px $panel-border-color; $panel-header-hover-bg: $dark-4; +$panel-corner: $panel-bg; // page header $page-header-bg: linear-gradient(90deg, #292a2d, black); diff --git a/public/sass/_variables.light.scss b/public/sass/_variables.light.scss index be2d05e2441..1e7a2e9cfbe 100644 --- a/public/sass/_variables.light.scss +++ b/public/sass/_variables.light.scss @@ -102,6 +102,7 @@ $panel-bg: $white; $panel-border-color: $gray-5; $panel-border: solid 1px $panel-border-color; $panel-header-hover-bg: $gray-6; +$panel-corner: $gray-4; // Page header $page-header-bg: linear-gradient(90deg, $white, $gray-7); diff --git a/public/sass/pages/_dashboard.scss b/public/sass/pages/_dashboard.scss index 589012bff3f..81698d396f8 100644 --- a/public/sass/pages/_dashboard.scss +++ b/public/sass/pages/_dashboard.scss @@ -214,7 +214,7 @@ div.flot-text { &--info { display: block; - @include panel-corner-color(lighten($panel-bg, 4%)); + @include panel-corner-color(lighten($panel-corner, 4%)); .fa:before { content: '\f129'; } @@ -222,7 +222,7 @@ div.flot-text { &--links { display: block; - @include panel-corner-color(lighten($panel-bg, 4%)); + @include panel-corner-color(lighten($panel-corner, 4%)); .fa { left: 4px; } From 4f943687d84c66165511547eb888607c219bbe9f Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Tue, 8 Jan 2019 09:55:23 +0100 Subject: [PATCH 6/9] feat: Display error when plot fail to render --- packages/grafana-ui/src/types/panel.ts | 1 + .../src/visualizations/Graph/Graph.tsx | 4 ++- public/app/core/components/Tooltip/Popper.tsx | 12 ++++----- .../core/components/Tooltip/withPopper.tsx | 6 ++--- .../features/dashboard/dashgrid/DataPanel.tsx | 26 ++++++++++++++++--- .../dashboard/dashgrid/PanelChrome.tsx | 4 +-- .../app/plugins/panel/graph2/GraphPanel.tsx | 7 ++--- public/sass/pages/_dashboard.scss | 2 +- 8 files changed, 42 insertions(+), 20 deletions(-) diff --git a/packages/grafana-ui/src/types/panel.ts b/packages/grafana-ui/src/types/panel.ts index 44336555a81..cf808f514de 100644 --- a/packages/grafana-ui/src/types/panel.ts +++ b/packages/grafana-ui/src/types/panel.ts @@ -9,6 +9,7 @@ export interface PanelProps { renderCounter: number; width: number; height: number; + onRenderError: () => void; } export interface PanelOptionsProps { diff --git a/packages/grafana-ui/src/visualizations/Graph/Graph.tsx b/packages/grafana-ui/src/visualizations/Graph/Graph.tsx index 51afb33802d..b3f2120639c 100644 --- a/packages/grafana-ui/src/visualizations/Graph/Graph.tsx +++ b/packages/grafana-ui/src/visualizations/Graph/Graph.tsx @@ -13,6 +13,7 @@ interface GraphProps { showBars?: boolean; width: number; height: number; + onRenderError: () => void; } export class Graph extends PureComponent { @@ -37,7 +38,7 @@ export class Graph extends PureComponent { return; } - const { width, timeSeries, timeRange, showLines, showBars, showPoints } = this.props; + const { width, timeSeries, timeRange, showLines, showBars, showPoints, onRenderError } = this.props; if (!width) { return; @@ -98,6 +99,7 @@ export class Graph extends PureComponent { $.plot(this.element, timeSeries, flotOptions); } catch (err) { console.log('Graph rendering error', err, flotOptions, timeSeries); + onRenderError(); } } diff --git a/public/app/core/components/Tooltip/Popper.tsx b/public/app/core/components/Tooltip/Popper.tsx index d5ea1d93091..65ef510ba8f 100644 --- a/public/app/core/components/Tooltip/Popper.tsx +++ b/public/app/core/components/Tooltip/Popper.tsx @@ -3,6 +3,11 @@ import Portal from 'app/core/components/Portal/Portal'; import { Manager, Popper as ReactPopper, Reference } from 'react-popper'; import Transition from 'react-transition-group/Transition'; +export enum Themes { + Default = 'popper__background--default', + Error = 'popper__background--error', +} + const defaultTransitionStyles = { transition: 'opacity 200ms linear', opacity: 0, @@ -21,12 +26,7 @@ interface Props { placement?: any; content: string | ((props: any) => JSX.Element); refClassName?: string; - theme?: string; -} - -export enum Themes { - Default = 'popper__background--default', - Error = 'popper__background--error', + theme?: Themes; } class Popper extends PureComponent { diff --git a/public/app/core/components/Tooltip/withPopper.tsx b/public/app/core/components/Tooltip/withPopper.tsx index 7be12748247..3766b78f0f6 100644 --- a/public/app/core/components/Tooltip/withPopper.tsx +++ b/public/app/core/components/Tooltip/withPopper.tsx @@ -1,5 +1,5 @@ import React from 'react'; - +import { Themes } from './Popper'; export interface UsingPopperProps { showPopper: (prevState: object) => void; hidePopper: (prevState: object) => void; @@ -9,7 +9,7 @@ export interface UsingPopperProps { content: string | ((props: any) => JSX.Element); className?: string; refClassName?: string; - theme?: string; + theme?: Themes; } interface Props { @@ -17,7 +17,7 @@ interface Props { className?: string; refClassName?: string; content: string | ((props: any) => JSX.Element); - theme?: string; + theme?: Themes; } interface State { diff --git a/public/app/features/dashboard/dashgrid/DataPanel.tsx b/public/app/features/dashboard/dashgrid/DataPanel.tsx index d57b787bf57..4cd9460802a 100644 --- a/public/app/features/dashboard/dashgrid/DataPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DataPanel.tsx @@ -16,6 +16,7 @@ import { Themes } from 'app/core/components/Tooltip/Popper'; interface RenderProps { loading: LoadingState; timeSeries: TimeSeries[]; + onRenderError: () => void; } export interface Props { @@ -35,6 +36,7 @@ export interface Props { export interface State { isFirstLoad: boolean; loading: LoadingState; + errorMessage: string; response: DataQueryResponse; } @@ -53,6 +55,7 @@ export class DataPanel extends Component { this.state = { loading: LoadingState.NotStarted, + errorMessage: '', response: { data: [], }, @@ -92,7 +95,7 @@ export class DataPanel extends Component { return; } - this.setState({ loading: LoadingState.Loading }); + this.setState({ loading: LoadingState.Loading, errorMessage: '' }); try { const ds = await this.dataSourceSrv.get(datasource); @@ -130,10 +133,24 @@ export class DataPanel extends Component { }); } catch (err) { console.log('Loading error', err); - this.setState({ loading: LoadingState.Error, isFirstLoad: false }); + this.onError('Request Error'); } }; + onError = (errorMessage: string) => { + if (this.state.loading !== LoadingState.Error || this.state.errorMessage !== errorMessage) { + this.setState({ + loading: LoadingState.Error, + isFirstLoad: false, + errorMessage: errorMessage + }); + } + } + + onRenderError = () => { + this.onError('Error rendering panel'); + } + render() { const { queries } = this.props; const { response, loading, isFirstLoad } = this.state; @@ -158,13 +175,14 @@ export class DataPanel extends Component { {this.props.children({ timeSeries, loading, + onRenderError: this.onRenderError })} ); } private renderLoadingStates(): JSX.Element { - const { loading } = this.state; + const { loading, errorMessage } = this.state; if (loading === LoadingState.Loading) { return (
@@ -174,7 +192,7 @@ export class DataPanel extends Component { } else if (loading === LoadingState.Error) { return ( { const { datasource, targets, transparent } = panel; const PanelComponent = plugin.exports.Panel; const containerClassNames = `panel-container panel-container--absolute ${transparent ? 'panel-transparent' : ''}`; - return ( {({ width, height }) => { @@ -115,7 +114,7 @@ export class PanelChrome extends PureComponent { widthPixels={width} refreshCounter={refreshCounter} > - {({ loading, timeSeries }) => { + {({ loading, timeSeries, onRenderError }) => { return (
{ width={width} height={height - PANEL_HEADER_HEIGHT} renderCounter={renderCounter} + onRenderError={onRenderError} />
); diff --git a/public/app/plugins/panel/graph2/GraphPanel.tsx b/public/app/plugins/panel/graph2/GraphPanel.tsx index a08276e5179..1a23fba0c27 100644 --- a/public/app/plugins/panel/graph2/GraphPanel.tsx +++ b/public/app/plugins/panel/graph2/GraphPanel.tsx @@ -1,6 +1,6 @@ // Libraries import _ from 'lodash'; -import React, { PureComponent } from 'react'; +import React, { Component } from 'react'; import colors from 'app/core/utils/colors'; // Components & Types @@ -9,13 +9,13 @@ import { Options } from './types'; interface Props extends PanelProps {} -export class GraphPanel extends PureComponent { +export class GraphPanel extends Component { constructor(props) { super(props); } render() { - const { timeSeries, timeRange, width, height } = this.props; + const { timeSeries, timeRange, width, height, onRenderError } = this.props; const { showLines, showBars, showPoints } = this.props.options; const vmSeries = processTimeSeries({ @@ -33,6 +33,7 @@ export class GraphPanel extends PureComponent { showBars={showBars} width={width} height={height} + onRenderError={onRenderError} /> ); } diff --git a/public/sass/pages/_dashboard.scss b/public/sass/pages/_dashboard.scss index 81698d396f8..a0ff9fd877c 100644 --- a/public/sass/pages/_dashboard.scss +++ b/public/sass/pages/_dashboard.scss @@ -233,7 +233,7 @@ div.flot-text { &--error { display: block; - color: $text-color; + color: $white; @include panel-corner-color($popover-error-bg); .fa:before { content: '\f12a'; From bcb94cc0ece63fef6c03870d17887d66c12a741a Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Tue, 8 Jan 2019 10:30:00 +0100 Subject: [PATCH 7/9] fix: GraphPanel should be a PureComponent --- public/app/plugins/panel/graph2/GraphPanel.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/app/plugins/panel/graph2/GraphPanel.tsx b/public/app/plugins/panel/graph2/GraphPanel.tsx index 1a23fba0c27..49d2ac3e4e4 100644 --- a/public/app/plugins/panel/graph2/GraphPanel.tsx +++ b/public/app/plugins/panel/graph2/GraphPanel.tsx @@ -1,6 +1,6 @@ // Libraries import _ from 'lodash'; -import React, { Component } from 'react'; +import React, { PureComponent } from 'react'; import colors from 'app/core/utils/colors'; // Components & Types @@ -9,7 +9,7 @@ import { Options } from './types'; interface Props extends PanelProps {} -export class GraphPanel extends Component { +export class GraphPanel extends PureComponent { constructor(props) { super(props); } From f428db282c277e70ccf192c3593feba1f98d3695 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Tue, 8 Jan 2019 13:32:08 +0100 Subject: [PATCH 8/9] fix: Remove the onRenderError prop and add an ErrorBoundary component --- packages/grafana-ui/src/types/panel.ts | 1 - .../src/visualizations/Graph/Graph.tsx | 5 +- .../ErrorBoundary/ErrorBoundary.tsx | 47 +++++++++++++++++++ .../features/dashboard/dashgrid/DataPanel.tsx | 29 ++++++++---- .../dashboard/dashgrid/PanelChrome.tsx | 3 +- .../app/plugins/panel/graph2/GraphPanel.tsx | 5 +- 6 files changed, 71 insertions(+), 19 deletions(-) create mode 100644 public/app/core/components/ErrorBoundary/ErrorBoundary.tsx diff --git a/packages/grafana-ui/src/types/panel.ts b/packages/grafana-ui/src/types/panel.ts index cf808f514de..44336555a81 100644 --- a/packages/grafana-ui/src/types/panel.ts +++ b/packages/grafana-ui/src/types/panel.ts @@ -9,7 +9,6 @@ export interface PanelProps { renderCounter: number; width: number; height: number; - onRenderError: () => void; } export interface PanelOptionsProps { diff --git a/packages/grafana-ui/src/visualizations/Graph/Graph.tsx b/packages/grafana-ui/src/visualizations/Graph/Graph.tsx index b3f2120639c..ad038cebcda 100644 --- a/packages/grafana-ui/src/visualizations/Graph/Graph.tsx +++ b/packages/grafana-ui/src/visualizations/Graph/Graph.tsx @@ -13,7 +13,6 @@ interface GraphProps { showBars?: boolean; width: number; height: number; - onRenderError: () => void; } export class Graph extends PureComponent { @@ -38,7 +37,7 @@ export class Graph extends PureComponent { return; } - const { width, timeSeries, timeRange, showLines, showBars, showPoints, onRenderError } = this.props; + const { width, timeSeries, timeRange, showLines, showBars, showPoints } = this.props; if (!width) { return; @@ -99,7 +98,7 @@ export class Graph extends PureComponent { $.plot(this.element, timeSeries, flotOptions); } catch (err) { console.log('Graph rendering error', err, flotOptions, timeSeries); - onRenderError(); + throw new Error('Error rendering panel'); } } diff --git a/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx b/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx new file mode 100644 index 00000000000..ed068819adc --- /dev/null +++ b/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx @@ -0,0 +1,47 @@ +import React, { Component } from 'react'; + +interface ErrorInfo { + componentStack: string; +} + +interface RenderProps { + error: Error; + errorInfo: ErrorInfo; +} + +interface Props { + children: (r: RenderProps) => JSX.Element; +} + +interface State { + error: Error; + errorInfo: ErrorInfo; +} + +class ErrorBoundary extends Component { + constructor(props) { + super(props); + this.state = { error: null, errorInfo: null }; + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo) { + this.setState({ + error: error, + errorInfo: errorInfo + }); + } + + render() { + const { error, errorInfo } = this.state; + return ( + <> + {this.props.children({ + error, + errorInfo, + })} + + ); + } +} + +export default ErrorBoundary; diff --git a/public/app/features/dashboard/dashgrid/DataPanel.tsx b/public/app/features/dashboard/dashgrid/DataPanel.tsx index 4cd9460802a..fa21276d7a2 100644 --- a/public/app/features/dashboard/dashgrid/DataPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DataPanel.tsx @@ -1,6 +1,7 @@ // Library import React, { Component } from 'react'; import Tooltip from 'app/core/components/Tooltip/Tooltip'; +import ErrorBoundary from 'app/core/components/ErrorBoundary/ErrorBoundary'; // Services import { getDatasourceSrv, DatasourceSrv } from 'app/features/plugins/datasource_srv'; @@ -13,10 +14,11 @@ import { DataQueryOptions, DataQueryResponse } from 'app/types'; import { TimeRange, TimeSeries, LoadingState } from '@grafana/ui'; import { Themes } from 'app/core/components/Tooltip/Popper'; +const DEFAULT_PLUGIN_ERROR = 'Error in plugin'; + interface RenderProps { loading: LoadingState; timeSeries: TimeSeries[]; - onRenderError: () => void; } export interface Props { @@ -147,10 +149,6 @@ export class DataPanel extends Component { } } - onRenderError = () => { - this.onError('Error rendering panel'); - } - render() { const { queries } = this.props; const { response, loading, isFirstLoad } = this.state; @@ -172,11 +170,22 @@ export class DataPanel extends Component { return ( <> {this.renderLoadingStates()} - {this.props.children({ - timeSeries, - loading, - onRenderError: this.onRenderError - })} + + {({error, errorInfo}) => { + if (errorInfo) { + this.onError(error.message || DEFAULT_PLUGIN_ERROR); + return null; + } + return ( + <> + {this.props.children({ + timeSeries, + loading, + })} + + ); + }} + ); } diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 90d66a14cd3..84e11511453 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -114,7 +114,7 @@ export class PanelChrome extends PureComponent { widthPixels={width} refreshCounter={refreshCounter} > - {({ loading, timeSeries, onRenderError }) => { + {({ loading, timeSeries }) => { return (
{ width={width} height={height - PANEL_HEADER_HEIGHT} renderCounter={renderCounter} - onRenderError={onRenderError} />
); diff --git a/public/app/plugins/panel/graph2/GraphPanel.tsx b/public/app/plugins/panel/graph2/GraphPanel.tsx index 49d2ac3e4e4..2c21c7f0e15 100644 --- a/public/app/plugins/panel/graph2/GraphPanel.tsx +++ b/public/app/plugins/panel/graph2/GraphPanel.tsx @@ -10,12 +10,12 @@ import { Options } from './types'; interface Props extends PanelProps {} export class GraphPanel extends PureComponent { - constructor(props) { + constructor(props: Props) { super(props); } render() { - const { timeSeries, timeRange, width, height, onRenderError } = this.props; + const { timeSeries, timeRange, width, height } = this.props; const { showLines, showBars, showPoints } = this.props.options; const vmSeries = processTimeSeries({ @@ -33,7 +33,6 @@ export class GraphPanel extends PureComponent { showBars={showBars} width={width} height={height} - onRenderError={onRenderError} /> ); } From 12a3edd6e501e1e92ab222e37bdfd8c7d2a299b5 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Tue, 8 Jan 2019 16:04:46 +0100 Subject: [PATCH 9/9] fix: Clean up per PR feedback. Thanks @dprokop --- .../ErrorBoundary/ErrorBoundary.tsx | 23 ++++++++----------- .../app/plugins/panel/graph2/GraphPanel.tsx | 4 ---- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx b/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx index ed068819adc..188750b0fef 100644 --- a/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx +++ b/public/app/core/components/ErrorBoundary/ErrorBoundary.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import { Component } from 'react'; interface ErrorInfo { componentStack: string; @@ -19,10 +19,10 @@ interface State { } class ErrorBoundary extends Component { - constructor(props) { - super(props); - this.state = { error: null, errorInfo: null }; - } + readonly state: State = { + error: null, + errorInfo: null, + }; componentDidCatch(error: Error, errorInfo: ErrorInfo) { this.setState({ @@ -32,15 +32,12 @@ class ErrorBoundary extends Component { } render() { + const { children } = this.props; const { error, errorInfo } = this.state; - return ( - <> - {this.props.children({ - error, - errorInfo, - })} - - ); + return children({ + error, + errorInfo, + }); } } diff --git a/public/app/plugins/panel/graph2/GraphPanel.tsx b/public/app/plugins/panel/graph2/GraphPanel.tsx index 2c21c7f0e15..020c33f7d38 100644 --- a/public/app/plugins/panel/graph2/GraphPanel.tsx +++ b/public/app/plugins/panel/graph2/GraphPanel.tsx @@ -10,10 +10,6 @@ import { Options } from './types'; interface Props extends PanelProps {} export class GraphPanel extends PureComponent { - constructor(props: Props) { - super(props); - } - render() { const { timeSeries, timeRange, width, height } = this.props; const { showLines, showBars, showPoints } = this.props.options;