From 8d96502c5b92db8a3bbcb35f0ff47c23e535647c Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 9 Jul 2019 06:58:46 -0700 Subject: [PATCH 1/4] fix: adjust domain & range for single value histogram --- src/lib/axes/axis_utils.ts | 17 ++++++++-- src/lib/series/scales.ts | 21 ++++++++++--- src/state/chart_state.ts | 1 + src/state/utils.ts | 2 +- stories/annotations.tsx | 64 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 8 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 6b104ee7a7..c19455cba4 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -61,12 +61,23 @@ export function computeAxisTicksDimensions( chartRotation: Rotation, axisConfig: AxisConfig, barsPadding?: number, + enableHistogramMode?: boolean, ): AxisTicksDimensions | null { if (axisSpec.hide) { return null; } - const scale = getScaleForAxisSpec(axisSpec, xDomain, yDomain, totalBarsInCluster, chartRotation, 0, 1, barsPadding); + const scale = getScaleForAxisSpec( + axisSpec, + xDomain, + yDomain, + totalBarsInCluster, + chartRotation, + 0, + 1, + barsPadding, + enableHistogramMode, + ); if (!scale) { throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } @@ -112,6 +123,7 @@ export function getScaleForAxisSpec( minRange: number, maxRange: number, barsPadding?: number, + enableHistogramMode?: boolean, ): Scale | null { const axisIsYDomain = isYDomain(axisSpec.position, chartRotation); @@ -122,7 +134,7 @@ export function getScaleForAxisSpec( } return null; } else { - return computeXScale(xDomain, totalBarsInCluster, minRange, maxRange, barsPadding); + return computeXScale(xDomain, totalBarsInCluster, minRange, maxRange, barsPadding, enableHistogramMode); } } @@ -580,6 +592,7 @@ export function getAxisTicksPositions( minMaxRanges.minRange, minMaxRanges.maxRange, barsPadding, + enableHistogramMode, ); if (!scale) { diff --git a/src/lib/series/scales.ts b/src/lib/series/scales.ts index 6b8ab4f8fc..fbfe812732 100644 --- a/src/lib/series/scales.ts +++ b/src/lib/series/scales.ts @@ -50,6 +50,7 @@ export function computeXScale( minRange: number, maxRange: number, barsPadding?: number, + enableHistogramMode?: boolean, ): Scale { const { scaleType, minInterval, domain, isBandScale, timeZone } = xDomain; const rangeDiff = Math.abs(maxRange - minRange); @@ -60,20 +61,30 @@ export function computeXScale( return new ScaleBand(domain, [minRange, maxRange], bandwidth, barsPadding); } else { if (isBandScale) { - const intervalCount = (domain[1] - domain[0]) / minInterval; - const bandwidth = rangeDiff / (intervalCount + 1); + const isSingleValueHistogram = enableHistogramMode && domain[1] - domain[0] === 0; + const domainMin = domain[0]; + const domainMax = isSingleValueHistogram ? domain[0] + minInterval : domain[1]; + const domainForScale = [domainMin, domainMax]; + + const intervalCount = (domainForScale[1] - domainForScale[0]) / minInterval; + const intervalCountOffest = isSingleValueHistogram ? 0 : 1; + const bandwidth = rangeDiff / (intervalCount + intervalCountOffest); const start = isInverse ? minRange - bandwidth : minRange; const end = isInverse ? maxRange : maxRange - bandwidth; - return new ScaleContinuous( + const rangeEnd = isSingleValueHistogram ? end + bandwidth : end; + + const scale = new ScaleContinuous( scaleType, - domain, - [start, end], + domainForScale, + [start, rangeEnd], bandwidth / totalBarsInCluster, minInterval, timeZone, totalBarsInCluster, barsPadding, ); + + return scale; } else { return new ScaleContinuous( scaleType, diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 55d2d73e86..acd111d8a1 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -836,6 +836,7 @@ export class ChartStore { this.chartRotation, this.chartTheme.axes, barsPadding, + this.enableHistogramMode.get(), ); if (dimensions) { this.axesTicksDimensions.set(id, dimensions); diff --git a/src/state/utils.ts b/src/state/utils.ts index b296d4b9e5..155bff5ef2 100644 --- a/src/state/utils.ts +++ b/src/state/utils.ts @@ -193,7 +193,7 @@ export function computeSeriesGeometries( const { stackedBarsInCluster, totalBarsInCluster } = countBarsInCluster(stacked, nonStacked); // compute scales - const xScale = computeXScale(xDomain, totalBarsInCluster, 0, width, barsPadding); + const xScale = computeXScale(xDomain, totalBarsInCluster, 0, width, barsPadding, enableHistogramMode); const yScales = computeYScales(yDomain, height, 0); // compute colors diff --git a/stories/annotations.tsx b/stories/annotations.tsx index 51ffe0e6cd..6329f87ca3 100644 --- a/stories/annotations.tsx +++ b/stories/annotations.tsx @@ -547,4 +547,68 @@ storiesOf('Annotations', module) /> ); + }) + .add('[test] line annotation single value histogram', () => { + const dataValues = [ + { + dataValue: 3.5, + }, + ]; + + const style = { + line: { + strokeWidth: 3, + stroke: '#f00', + opacity: 1, + }, + details: { + fontSize: 12, + fontFamily: 'Arial', + fontStyle: 'bold', + fill: 'gray', + padding: 0, + }, + }; + + const chartRotation = select( + 'chartRotation', + { + '0 deg': 0, + '90 deg': 90, + '-90 deg': -90, + '180 deg': 180, + }, + 0, + ); + + const isBottom = boolean('x domain axis is bottom', true); + const axisPosition = isBottom ? Position.Bottom : Position.Top; + + const xDomain = { + minInterval: 1, + // max: 4, + }; + + return ( + + + + + {/* */} + + + ); }); From 3fe0b07f26e43f815894acbe88c2fddf4dc6a119 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 16 Jul 2019 10:38:47 -0700 Subject: [PATCH 2/4] test: add tests for single value scale --- src/lib/series/scales.test.ts | 38 +++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/lib/series/scales.test.ts b/src/lib/series/scales.test.ts index 1f83ca5ca5..a89c1e8f7e 100644 --- a/src/lib/series/scales.test.ts +++ b/src/lib/series/scales.test.ts @@ -41,6 +41,44 @@ describe('Series scales', () => { expect(scale.scale(3)).toBe(expectedBandwidth * 0); }); + describe('computeXScale with single value domain', () => { + const maxRange = 120; + const singleDomainValue = 3; + const minInterval = 1; + + test('should return extended domain & range when in histogram mode', () => { + const xDomainSingleValue: XDomain = { + type: 'xDomain', + isBandScale: true, + domain: [singleDomainValue, singleDomainValue], + minInterval: minInterval, + scaleType: ScaleType.Linear, + }; + const enableHistogramMode = true; + + const scale = computeXScale(xDomainSingleValue, 1, 0, maxRange, 0, enableHistogramMode); + expect(scale.bandwidth).toBe(maxRange); + expect(scale.domain).toEqual([singleDomainValue, singleDomainValue + minInterval]); + expect(scale.range).toEqual([0, maxRange]); + }); + + test('should return unextended domain & range when not in histogram mode', () => { + const xDomainSingleValue: XDomain = { + type: 'xDomain', + isBandScale: true, + domain: [singleDomainValue, singleDomainValue], + minInterval: minInterval, + scaleType: ScaleType.Linear, + }; + const enableHistogramMode = false; + + const scale = computeXScale(xDomainSingleValue, 1, 0, maxRange, 0, enableHistogramMode); + expect(scale.bandwidth).toBe(maxRange); + expect(scale.domain).toEqual([singleDomainValue, singleDomainValue]); + expect(scale.range).toEqual([0, 0]); + }); + }); + test('should compute X Scale ordinal', () => { const nonZeroGroupScale = computeXScale(xDomainOrdinal, 1, 120, 0); const expectedBandwidth = 60; From f47ff21b39a5063e6fda0f18c27bea36c6e7152d Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 16 Jul 2019 11:00:15 -0700 Subject: [PATCH 3/4] refactor: clean up story & refactor with clearer variable names --- src/lib/series/scales.ts | 22 ++++++++++++---------- stories/annotations.tsx | 8 ++------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/lib/series/scales.ts b/src/lib/series/scales.ts index fbfe812732..f021bba8e6 100644 --- a/src/lib/series/scales.ts +++ b/src/lib/series/scales.ts @@ -61,22 +61,24 @@ export function computeXScale( return new ScaleBand(domain, [minRange, maxRange], bandwidth, barsPadding); } else { if (isBandScale) { - const isSingleValueHistogram = enableHistogramMode && domain[1] - domain[0] === 0; - const domainMin = domain[0]; - const domainMax = isSingleValueHistogram ? domain[0] + minInterval : domain[1]; - const domainForScale = [domainMin, domainMax]; + const [domainMin, domainMax] = domain; + const isSingleValueHistogram = enableHistogramMode && domainMax - domainMin === 0; - const intervalCount = (domainForScale[1] - domainForScale[0]) / minInterval; + const adjustedDomainMax = isSingleValueHistogram ? domainMin + minInterval : domainMax; + const adjustedDomain = [domainMin, adjustedDomainMax]; + + const intervalCount = (adjustedDomain[1] - adjustedDomain[0]) / minInterval; const intervalCountOffest = isSingleValueHistogram ? 0 : 1; const bandwidth = rangeDiff / (intervalCount + intervalCountOffest); - const start = isInverse ? minRange - bandwidth : minRange; - const end = isInverse ? maxRange : maxRange - bandwidth; - const rangeEnd = isSingleValueHistogram ? end + bandwidth : end; + + const rangeEndOffset = isSingleValueHistogram ? 0 : bandwidth; + const start = isInverse ? minRange - rangeEndOffset : minRange; + const end = isInverse ? maxRange : maxRange - rangeEndOffset; const scale = new ScaleContinuous( scaleType, - domainForScale, - [start, rangeEnd], + adjustedDomain, + [start, end], bandwidth / totalBarsInCluster, minInterval, timeZone, diff --git a/stories/annotations.tsx b/stories/annotations.tsx index 6329f87ca3..0156310769 100644 --- a/stories/annotations.tsx +++ b/stories/annotations.tsx @@ -581,12 +581,8 @@ storiesOf('Annotations', module) 0, ); - const isBottom = boolean('x domain axis is bottom', true); - const axisPosition = isBottom ? Position.Bottom : Position.Top; - const xDomain = { minInterval: 1, - // max: 4, }; return ( @@ -598,8 +594,8 @@ storiesOf('Annotations', module) dataValues={dataValues} style={style} /> - - {/* */} + + Date: Wed, 17 Jul 2019 09:21:38 -0700 Subject: [PATCH 4/4] refactor: abstract range computation to function --- src/lib/series/scales.ts | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/lib/series/scales.ts b/src/lib/series/scales.ts index f021bba8e6..61feca242a 100644 --- a/src/lib/series/scales.ts +++ b/src/lib/series/scales.ts @@ -38,6 +38,23 @@ export function countBarsInCluster( }; } +function getBandScaleRange( + isInverse: boolean, + isSingleValueHistogram: boolean, + minRange: number, + maxRange: number, + bandwidth: number, +): { + start: number; + end: number; +} { + const rangeEndOffset = isSingleValueHistogram ? 0 : bandwidth; + const start = isInverse ? minRange - rangeEndOffset : minRange; + const end = isInverse ? maxRange : maxRange - rangeEndOffset; + + return { start, end }; +} + /** * Compute the x scale used to align geometries to the x axis. * @param xDomain the x domain @@ -62,7 +79,7 @@ export function computeXScale( } else { if (isBandScale) { const [domainMin, domainMax] = domain; - const isSingleValueHistogram = enableHistogramMode && domainMax - domainMin === 0; + const isSingleValueHistogram = !!enableHistogramMode && domainMax - domainMin === 0; const adjustedDomainMax = isSingleValueHistogram ? domainMin + minInterval : domainMax; const adjustedDomain = [domainMin, adjustedDomainMax]; @@ -71,9 +88,7 @@ export function computeXScale( const intervalCountOffest = isSingleValueHistogram ? 0 : 1; const bandwidth = rangeDiff / (intervalCount + intervalCountOffest); - const rangeEndOffset = isSingleValueHistogram ? 0 : bandwidth; - const start = isInverse ? minRange - rangeEndOffset : minRange; - const end = isInverse ? maxRange : maxRange - rangeEndOffset; + const { start, end } = getBandScaleRange(isInverse, isSingleValueHistogram, minRange, maxRange, bandwidth); const scale = new ScaleContinuous( scaleType,