From 1fc22f6e4df4de3740baa62c6efa46fa1749fc4a Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Thu, 23 Sep 2021 08:59:52 -0400 Subject: [PATCH] BarGauge: Set cell color to off if numeric value is NaN (#39574) (#39580) * BarGauge: Set cell color to off if numeric value is NaN * BarGauge: Pull getCellColor out into a pure function and add unit tests (cherry picked from commit 6948dbe5504fcfd7ad334104a626d64cdbf1e4e9) Co-authored-by: Ashley Harrison --- .../src/components/BarGauge/BarGauge.test.tsx | 65 ++++++++++++++++ .../src/components/BarGauge/BarGauge.tsx | 77 ++++++++++--------- 2 files changed, 105 insertions(+), 37 deletions(-) diff --git a/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx b/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx index 340845b1436..307473a6924 100644 --- a/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx +++ b/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx @@ -4,6 +4,7 @@ import { DisplayValue, VizOrientation, ThresholdsMode, + FALLBACK_COLOR, Field, FieldType, getDisplayProcessor, @@ -12,6 +13,7 @@ import { import { BarGauge, Props, + getCellColor, getValueColor, getBasicAndGradientStyles, getBarGradient, @@ -74,6 +76,69 @@ function getValue(value: number, title?: string): DisplayValue { } describe('BarGauge', () => { + describe('getCellColor', () => { + it('returns a fallback if the positionValue is null', () => { + const props = getProps(); + expect(getCellColor(null, props.value, props.display)).toEqual({ + background: FALLBACK_COLOR, + border: FALLBACK_COLOR, + }); + }); + + it('does not show as lit if the value is null (somehow)', () => { + const props = getProps(); + expect(getCellColor(1, (null as unknown) as DisplayValue, props.display)).toEqual( + expect.objectContaining({ + isLit: false, + }) + ); + }); + + it('does not show as lit if the numeric value is NaN', () => { + const props = getProps(); + expect( + getCellColor( + 1, + { + numeric: NaN, + text: '0', + }, + props.display + ) + ).toEqual( + expect.objectContaining({ + isLit: false, + }) + ); + }); + + it('does not show as lit if the positionValue is greater than the numeric value', () => { + const props = getProps(); + expect(getCellColor(75, props.value, props.display)).toEqual( + expect.objectContaining({ + isLit: false, + }) + ); + }); + + it('shows as lit otherwise', () => { + const props = getProps(); + expect(getCellColor(1, props.value, props.display)).toEqual( + expect.objectContaining({ + isLit: true, + }) + ); + }); + + it('returns a fallback if there is no display processor', () => { + const props = getProps(); + expect(getCellColor(null, props.value, undefined)).toEqual({ + background: FALLBACK_COLOR, + border: FALLBACK_COLOR, + }); + }); + }); + describe('Get value color', () => { it('should get the threshold color if value is same as a threshold', () => { const props = getProps(); diff --git a/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx b/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx index c196fd13544..8a444b61352 100644 --- a/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx +++ b/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx @@ -122,43 +122,8 @@ export class BarGauge extends PureComponent { ); } - getCellColor(positionValue: TimeSeriesValue): CellColors { - const { value, display } = this.props; - if (positionValue === null) { - return { - background: FALLBACK_COLOR, - border: FALLBACK_COLOR, - }; - } - - const color = display ? display(positionValue).color : null; - - if (color) { - // if we are past real value the cell is not "on" - if (value === null || (positionValue !== null && positionValue > value.numeric)) { - return { - background: tinycolor(color).setAlpha(0.18).toRgbString(), - border: 'transparent', - isLit: false, - }; - } else { - return { - background: tinycolor(color).setAlpha(0.95).toRgbString(), - backgroundShade: tinycolor(color).setAlpha(0.55).toRgbString(), - border: tinycolor(color).setAlpha(0.9).toRgbString(), - isLit: true, - }; - } - } - - return { - background: FALLBACK_COLOR, - border: FALLBACK_COLOR, - }; - } - renderRetroBars(): ReactNode { - const { field, value, itemSpacing, alignmentFactors, orientation, lcdCellWidth, text } = this.props; + const { display, field, value, itemSpacing, alignmentFactors, orientation, lcdCellWidth, text } = this.props; const { valueHeight, valueWidth, @@ -200,7 +165,7 @@ export class BarGauge extends PureComponent { for (let i = 0; i < cellCount; i++) { const currentValue = minValue + (valueRange / cellCount) * i; - const cellColor = this.getCellColor(currentValue); + const cellColor = getCellColor(currentValue, value, display); const cellStyles: CSSProperties = { borderRadius: '2px', }; @@ -425,6 +390,44 @@ export function calculateBarAndValueDimensions(props: Props): BarAndValueDimensi }; } +export function getCellColor( + positionValue: TimeSeriesValue, + value: Props['value'], + display: Props['display'] +): CellColors { + if (positionValue === null) { + return { + background: FALLBACK_COLOR, + border: FALLBACK_COLOR, + }; + } + + const color = display ? display(positionValue).color : null; + + if (color) { + // if we are past real value the cell is not "on" + if (value === null || isNaN(value.numeric) || (positionValue !== null && positionValue > value.numeric)) { + return { + background: tinycolor(color).setAlpha(0.18).toRgbString(), + border: 'transparent', + isLit: false, + }; + } else { + return { + background: tinycolor(color).setAlpha(0.95).toRgbString(), + backgroundShade: tinycolor(color).setAlpha(0.55).toRgbString(), + border: tinycolor(color).setAlpha(0.9).toRgbString(), + isLit: true, + }; + } + } + + return { + background: FALLBACK_COLOR, + border: FALLBACK_COLOR, + }; +} + export function getValuePercent(value: number, minValue: number, maxValue: number): number { return Math.min((value - minValue) / (maxValue - minValue), 1); }