Skip to content

Commit

Permalink
fix: ensure Band's mainColumnName cannot be reused for lower and upper (
Browse files Browse the repository at this point in the history
  • Loading branch information
TCL735 authored Feb 10, 2022
1 parent 19eb200 commit 14b1a0e
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 15 deletions.
2 changes: 1 addition & 1 deletion giraffe/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@influxdata/giraffe",
"version": "2.24.1",
"version": "2.24.2",
"main": "dist/index.js",
"module": "dist/index.js",
"license": "MIT",
Expand Down
4 changes: 2 additions & 2 deletions giraffe/src/components/BandHoverLayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ export const BandHoverLayer: FunctionComponent<Props> = ({
y: yColKey,
fill: fillColKeys,
lineWidth,
lowerColumnName = '',
lowerColumnName,
mainColumnName: rowColumnName,
shadeOpacity,
upperColumnName = '',
upperColumnName,
} = config

const xColData = spec.table.getColumn(xColKey, 'number')
Expand Down
4 changes: 2 additions & 2 deletions giraffe/src/components/BandLayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ export const BandLayer: FunctionComponent<Props> = props => {
} = props

const {
lowerColumnName = '',
lowerColumnName,
mainColumnName: rowColumnName,
upperColumnName = '',
upperColumnName,
} = config

const simplifiedLineData = useMemo(
Expand Down
15 changes: 8 additions & 7 deletions giraffe/src/components/PlotResizer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {LayerTypes, PlotDimensions, SizedConfig} from '../types'

import {get} from '../utils/get'
import {resizePlotWithStaticLegend} from '../utils/legend/resizePlot'
import {normalizeConfig} from '../utils/normalizeConfig'
import {usePlotEnv} from '../utils/usePlotEnv'

import {SizedPlot} from './SizedPlot'
Expand All @@ -28,16 +29,16 @@ export const PlotResizer: FC<PlotResizerProps> = props => {
config.staticLegend
)

const sizedConfig: SizedConfig = useMemo(
const normalizedConfig: SizedConfig = useMemo(
() => ({
...config,
...normalizeConfig(config),
height: resized.height,
width: resized.width,
}),
[config, height, width]
[config, height, width] // eslint-disable-line react-hooks/exhaustive-deps
)

const env = usePlotEnv(sizedConfig)
const env = usePlotEnv(normalizedConfig)
const spec = env.getSpec(0)
const graphType = get(config, 'layers.0.type')

Expand All @@ -51,14 +52,14 @@ export const PlotResizer: FC<PlotResizerProps> = props => {
graphType === LayerTypes.Gauge ||
graphType === LayerTypes.SimpleTable
) {
return <SizedTable config={sizedConfig}>{children}</SizedTable>
return <SizedTable config={normalizedConfig}>{children}</SizedTable>
}

return (
<>
<SizedPlot
axesCanvasRef={axesCanvasRef}
config={sizedConfig}
config={normalizedConfig}
layerCanvasRef={layerCanvasRef}
env={env}
>
Expand All @@ -67,7 +68,7 @@ export const PlotResizer: FC<PlotResizerProps> = props => {
{config.staticLegend && !config.staticLegend.hide ? (
<StaticLegendBox
columnFormatter={env.getFormatterForColumn}
config={config}
config={normalizedConfig}
height={height - resized.height}
spec={spec}
top={resized.height}
Expand Down
4 changes: 2 additions & 2 deletions giraffe/src/transforms/band.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,9 @@ export const bandTransform = (
const yCol = table.getColumn(yColumnKey, 'number')
const bandLineMap = getBandLineMap(
fillColumnMap,
lowerColumnName || '',
lowerColumnName,
rowColumnName,
upperColumnName || ''
upperColumnName
)
const fillScale = range =>
getBandColorScale(bandLineMap, colors)(range * BAND_COLOR_SCALE_CONSTANT)
Expand Down
121 changes: 121 additions & 0 deletions giraffe/src/utils/normalizeConfig.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import {Config} from '../types'
import {normalizeConfig, normalizeLayers} from './normalizeConfig'

describe('normalizeConfig', () => {
it('handles unexpected arguments', () => {
const emptyConfig = {layers: []}

expect(normalizeConfig(undefined)).toEqual(emptyConfig)
expect(normalizeConfig(null)).toEqual(emptyConfig)
expect(normalizeConfig(emptyConfig)).toEqual(emptyConfig)
})

describe('config for Band Layer', () => {
it('returns the config when it has a proper band layer', () => {
const config = {
layers: [
{
type: 'band',
x: '_time',
y: '_value',
fill: ['result', '_field', '_measurement', 'cpu', 'host'],
mainColumnName: '_result',
},
],
} as Config
expect(normalizeConfig(config)).toEqual({
...config,
layers: [
{
...config.layers[0],
lowerColumnName: '',
upperColumnName: '',
},
],
})
})

it('overrides the "lowerColumnName" and "upperColumnName" when they match "mainColumnName"', () => {
const mainColumnName = 'mean'
const config = {
layers: [
{
type: 'band',
x: '_time',
y: '_value',
fill: ['result', '_field', '_measurement', 'cpu', 'host'],
upperColumnName: mainColumnName,
mainColumnName,
lowerColumnName: mainColumnName,
},
],
} as Config
expect(normalizeConfig(config)).toEqual({
...config,
layers: [
{
...config.layers[0],
upperColumnName: '',
mainColumnName,
lowerColumnName: '',
},
],
})
})

it('uses the "lowerColumnName" and "upperColumnName" when they do not match "mainColumnName"', () => {
const upperColumnName = 'max'
const mainColumnName = 'mean'
const lowerColumnName = 'min'
const config = {
layers: [
{
type: 'band',
x: '_time',
y: '_value',
fill: ['result', '_field', '_measurement', 'cpu', 'host'],
upperColumnName,
mainColumnName,
lowerColumnName,
},
],
} as Config
expect(normalizeConfig(config)).toEqual({
...config,
layers: [
{
...config.layers[0],
upperColumnName,
mainColumnName,
lowerColumnName,
},
],
})
})
})

/*
TODO: see https://github.com/influxdata/giraffe/issues/447
when ready add the tests for
- AnnotationLayerConfig
- CustomLayerConfig
- GaugeLayerConfig
- GeoLayerConfig
- HeatmapLayerConfig
- HistogramLayerConfig
- LineLayerConfig
- MosaicLayerConfig
- RawFluxDataTableLayerConfig
- ScatterLayerConfig
- SimpleTableLayerConfig
- SingleStatLayerConfig
- TableGraphLayerConfig
*/
})

describe('normalizeLayers', () => {
it('handles incorrect usage', () => {
expect(normalizeLayers(undefined)).toEqual([])
expect(normalizeLayers(null)).toEqual([])
})
})
22 changes: 22 additions & 0 deletions giraffe/src/utils/normalizeConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {Config, LayerConfig} from '../types'

// TODO: move all layer defaults here, see: https://github.com/influxdata/giraffe/issues/447
export const normalizeLayers = (layers: LayerConfig[]): LayerConfig[] =>
layers?.map(layerConfig => {
if (layerConfig.type === 'band') {
let {upperColumnName, lowerColumnName} = layerConfig
if (!upperColumnName || upperColumnName === layerConfig.mainColumnName) {
upperColumnName = ''
}
if (!lowerColumnName || lowerColumnName === layerConfig.upperColumnName) {
lowerColumnName = ''
}
return {...layerConfig, upperColumnName, lowerColumnName}
}
return layerConfig
}) || []

export const normalizeConfig = (config: Config): Config => ({
...config,
layers: normalizeLayers(config?.layers),
})
2 changes: 1 addition & 1 deletion stories/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@influxdata/giraffe-stories",
"version": "2.24.1",
"version": "2.24.2",
"license": "MIT",
"repository": {
"type": "git",
Expand Down

0 comments on commit 14b1a0e

Please sign in to comment.