From b0db741569a053789838ee5b893914b761d07f67 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Mon, 11 Mar 2019 14:57:31 -0700 Subject: [PATCH 01/24] refactor(domain): limit domain range with DomainRange interface --- src/lib/series/specs.ts | 11 ++++++-- stories/bar_chart.tsx | 61 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 34d281b5c8..0096bf4069 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -1,6 +1,6 @@ import { GridLineConfig } from '../themes/theme'; import { Accessor } from '../utils/accessor'; -import { Domain } from '../utils/domain'; +// import { Domain } from '../utils/domain'; import { AxisId, GroupId, SpecId } from '../utils/ids'; import { ScaleContinuousType, ScaleType } from '../utils/scales/scales'; import { CurveType } from './curves'; @@ -23,6 +23,11 @@ export interface GeomDatum { tooltipPosition: TooltipPosition; } +export interface DomainRange { + min: number; + max: number; +} + export interface SeriesSpec { /** The ID of the spec, generated via getSpecId method */ id: SpecId; @@ -33,9 +38,9 @@ export interface SeriesSpec { /** An array of data */ data: Datum[]; /** If specified, it constrant the x domain to these values */ - xDomain?: Domain; + xDomain?: DomainRange; /** If specified, it constrant the y Domain to these values */ - yDomain?: Domain; + yDomain?: DomainRange; /** The type of series you are looking to render */ seriesType: 'bar' | 'line' | 'area' | 'basic'; /** Custom colors for series */ diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx index f08f57471f..21b2370653 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -1,4 +1,4 @@ -import { boolean } from '@storybook/addon-knobs'; +import { boolean, number } from '@storybook/addon-knobs'; import { storiesOf } from '@storybook/react'; import React from 'react'; import { @@ -825,4 +825,63 @@ storiesOf('Bar Chart', module) /> ); + }) + .add('with limited domain', () => { + const darkmode = boolean('darkmode', false); + const className = darkmode ? 'story-chart-dark' : 'story-chart'; + const defaultTheme = darkmode ? DARK_THEME : LIGHT_THEME; + + const xDomainOptions = { + range: false, + max: 3, + min: 0, + step: 0.5, + }; + + const yDomainOptions = { + range: false, + max: 7, + min: 0, + step: 0.5, + }; + + const xDomain = { + min: number('xDomain min', 0, xDomainOptions), + max: number('xDomain max', 3, xDomainOptions), + }; + + const yDomain = { + min: number('yDomain min', 0, yDomainOptions), + max: number('yDomain max', 7, yDomainOptions), + }; + + return ( + + + + Number(d).toFixed(2)} + /> + + + + ); }); From c00fc0ed37c662a2670e3c3b59daf7b58193641e Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 12 Mar 2019 09:00:50 -0700 Subject: [PATCH 02/24] test(y_domain): add test for coerceYScaleTypes when no specs --- src/lib/series/domains/y_domain.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/series/domains/y_domain.test.ts b/src/lib/series/domains/y_domain.test.ts index 5e8b334e0e..1634397a5b 100644 --- a/src/lib/series/domains/y_domain.test.ts +++ b/src/lib/series/domains/y_domain.test.ts @@ -3,7 +3,7 @@ import { ScaleType } from '../../utils/scales/scales'; import { RawDataSeries } from '../series'; import { BasicSeriesSpec } from '../specs'; import { BARCHART_1Y0G } from '../utils/test_dataset'; -import { mergeYDomain, splitSpecsByGroupId } from './y_domain'; +import { coerceYScaleTypes, mergeYDomain, splitSpecsByGroupId } from './y_domain'; describe('Y Domain', () => { test('Should merge Y domain', () => { @@ -405,4 +405,9 @@ describe('Y Domain', () => { expect(groupValues[1].stacked).toEqual([spec3]); expect(groupValues[0].nonStacked).toEqual([]); }); + + test('Should return null for YScaleType when there are no specs', () => { + const specs: Array> = []; + expect(coerceYScaleTypes(specs)).toBe(null); + }); }); From cba5cf17c0b275ee22b5e42e7c0bbea9ef8ed0f8 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 12 Mar 2019 11:06:28 -0700 Subject: [PATCH 03/24] test(y_domain): add test for mismatch spec case --- src/lib/series/domains/y_domain.test.ts | 87 ++++++++++++++++++++++++- src/lib/series/domains/y_domain.ts | 20 +++--- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/lib/series/domains/y_domain.test.ts b/src/lib/series/domains/y_domain.test.ts index 1634397a5b..b0849a7f4d 100644 --- a/src/lib/series/domains/y_domain.test.ts +++ b/src/lib/series/domains/y_domain.test.ts @@ -3,7 +3,13 @@ import { ScaleType } from '../../utils/scales/scales'; import { RawDataSeries } from '../series'; import { BasicSeriesSpec } from '../specs'; import { BARCHART_1Y0G } from '../utils/test_dataset'; -import { coerceYScaleTypes, mergeYDomain, splitSpecsByGroupId } from './y_domain'; +import { + coerceYScaleTypes, + getDataSeriesOnGroup, + mergeYDomain, + splitSpecsByGroupId, + YBasicSeriesSpec, +} from './y_domain'; describe('Y Domain', () => { test('Should merge Y domain', () => { @@ -410,4 +416,83 @@ describe('Y Domain', () => { const specs: Array> = []; expect(coerceYScaleTypes(specs)).toBe(null); }); + + test.only('Should getDataSeriesOnGroup for matching specs', () => { + const dataSeries: RawDataSeries[] = [ + { + specId: getSpecId('a'), + key: [''], + seriesColorKey: '', + data: [{ x: 1, y: 2 }, { x: 2, y: 2 }, { x: 3, y: 2 }, { x: 4, y: 5 }], + }, + { + specId: getSpecId('a'), + key: [''], + seriesColorKey: '', + data: [{ x: 1, y: 2 }, { x: 4, y: 7 }], + }, + ]; + const specDataSeries = new Map(); + specDataSeries.set(getSpecId('b'), dataSeries); + + const specs: YBasicSeriesSpec[] = [{ + seriesType: 'area', + yScaleType: ScaleType.Linear, + groupId: getGroupId('a'), + id: getSpecId('a'), + stackAccessors: ['a'], + yScaleToDataExtent: true, + yDomain: { + max: 12, + min: 2, + }, + }]; + + const rawDataSeries = getDataSeriesOnGroup(specDataSeries, specs); + expect(rawDataSeries).toEqual([]); + + }); + + test('Should merge Y domain with limited range', () => { + const dataSeries: RawDataSeries[] = [ + { + specId: getSpecId('a'), + key: [''], + seriesColorKey: '', + data: [{ x: 1, y: 2 }, { x: 2, y: 2 }, { x: 3, y: 2 }, { x: 4, y: 5 }], + }, + { + specId: getSpecId('a'), + key: [''], + seriesColorKey: '', + data: [{ x: 1, y: 2 }, { x: 4, y: 7 }], + }, + ]; + const specDataSeries = new Map(); + specDataSeries.set(getSpecId('a'), dataSeries); + + const mergedDomain = mergeYDomain(specDataSeries, [ + { + seriesType: 'area', + yScaleType: ScaleType.Linear, + groupId: getGroupId('a'), + id: getSpecId('a'), + stackAccessors: ['a'], + yScaleToDataExtent: true, + yDomain: { + max: 12, + min: 2, + }, + }, + ]); + expect(mergedDomain).toEqual([ + { + type: 'yDomain', + groupId: 'a', + domain: [2, 12], + scaleType: ScaleType.Linear, + isBandScale: false, + }, + ]); + }); }); diff --git a/src/lib/series/domains/y_domain.ts b/src/lib/series/domains/y_domain.ts index 89c9e1f2e6..dbdae31202 100644 --- a/src/lib/series/domains/y_domain.ts +++ b/src/lib/series/domains/y_domain.ts @@ -37,7 +37,7 @@ export function mergeYDomain( const yDomains = specsByGroupIdsEntries.map( ([groupId, groupSpecs]): YDomain => { - const groupYScaleType = coerchYScaleTypes([...groupSpecs.stacked, ...groupSpecs.nonStacked]); + const groupYScaleType = coerceYScaleTypes([...groupSpecs.stacked, ...groupSpecs.nonStacked]); if (groupYScaleType === null) { throw new Error(`Cannot merge ${groupId} domain. Missing Y scale types`); } @@ -79,7 +79,7 @@ export function mergeYDomain( return yDomains; } -function getDataSeriesOnGroup( +export function getDataSeriesOnGroup( dataSeries: Map, specs: YBasicSeriesSpec[], ): RawDataSeries[] { @@ -147,16 +147,16 @@ export function splitSpecsByGroupId(specs: YBasicSeriesSpec[]) { } /** - * Coerch the scale types of a set of specification to a generic one. + * Coerce the scale types of a set of specification to a generic one. * If there is at least one bar series type, than the response will specity - * that the coerched scale is a `scaleBand` (each point needs to have a surrounding empty + * that the coerced scale is a `scaleBand` (each point needs to have a surrounding empty * space to draw the bar width). - * If there are multiple continuous scale types, is coerched to linear. - * If there are at least one Ordinal scale type, is coerched to ordinal. - * If none of the above, than coerch to the specified scale. + * If there are multiple continuous scale types, is coerced to linear. + * If there are at least one Ordinal scale type, is coerced to ordinal. + * If none of the above, than coerce to the specified scale. * @returns {ChartScaleType} */ -export function coerchYScaleTypes( +export function coerceYScaleTypes( specs: Array>, ): ScaleContinuousType | null { const scaleTypes = new Set(); @@ -166,10 +166,10 @@ export function coerchYScaleTypes( if (specs.length === 0 || scaleTypes.size === 0) { return null; } - return coerchYScale(scaleTypes); + return coerceYScale(scaleTypes); } -function coerchYScale(scaleTypes: Set): ScaleContinuousType { +function coerceYScale(scaleTypes: Set): ScaleContinuousType { if (scaleTypes.size === 1) { const scales = scaleTypes.values(); const value = scales.next().value; From f48e00a0949a17936699d3af6e89e0dab9516405 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 12 Mar 2019 15:11:24 -0700 Subject: [PATCH 04/24] refactor(domain): add domain range prop to axis spec --- src/lib/series/domains/x_domain.test.ts | 2 +- src/lib/series/domains/x_domain.ts | 2 +- src/lib/series/domains/y_domain.test.ts | 48 ------------------- src/lib/series/domains/y_domain.ts | 19 +++++++- src/lib/series/specs.ts | 8 ++-- stories/axis.tsx | 50 ++++++++++++++++++++ stories/bar_chart.tsx | 61 +------------------------ stories/line_chart.tsx | 1 + 8 files changed, 76 insertions(+), 115 deletions(-) diff --git a/src/lib/series/domains/x_domain.test.ts b/src/lib/series/domains/x_domain.test.ts index d072ea7322..30a2636784 100644 --- a/src/lib/series/domains/x_domain.test.ts +++ b/src/lib/series/domains/x_domain.test.ts @@ -590,12 +590,12 @@ describe('X Domain', () => { specDataSeries.set(ds1.id, ds1); specDataSeries.set(ds2.id, ds2); const { xValues } = getSplittedSeries(specDataSeries); + const mergedDomain = mergeXDomain( [ { seriesType: 'area', xScaleType: ScaleType.Linear, - xDomain: [0, 10], }, { seriesType: 'line', diff --git a/src/lib/series/domains/x_domain.ts b/src/lib/series/domains/x_domain.ts index 012258cd3c..15799c21c4 100644 --- a/src/lib/series/domains/x_domain.ts +++ b/src/lib/series/domains/x_domain.ts @@ -16,7 +16,7 @@ export type XDomain = BaseDomain & { * Merge X domain value between a set of chart specification. */ export function mergeXDomain( - specs: Array>, + specs: Array>, xValues: Set, ): XDomain { const mainXScaleType = convertXScaleTypes(specs); diff --git a/src/lib/series/domains/y_domain.test.ts b/src/lib/series/domains/y_domain.test.ts index b0849a7f4d..629dae20ac 100644 --- a/src/lib/series/domains/y_domain.test.ts +++ b/src/lib/series/domains/y_domain.test.ts @@ -442,57 +442,9 @@ describe('Y Domain', () => { id: getSpecId('a'), stackAccessors: ['a'], yScaleToDataExtent: true, - yDomain: { - max: 12, - min: 2, - }, }]; const rawDataSeries = getDataSeriesOnGroup(specDataSeries, specs); expect(rawDataSeries).toEqual([]); - - }); - - test('Should merge Y domain with limited range', () => { - const dataSeries: RawDataSeries[] = [ - { - specId: getSpecId('a'), - key: [''], - seriesColorKey: '', - data: [{ x: 1, y: 2 }, { x: 2, y: 2 }, { x: 3, y: 2 }, { x: 4, y: 5 }], - }, - { - specId: getSpecId('a'), - key: [''], - seriesColorKey: '', - data: [{ x: 1, y: 2 }, { x: 4, y: 7 }], - }, - ]; - const specDataSeries = new Map(); - specDataSeries.set(getSpecId('a'), dataSeries); - - const mergedDomain = mergeYDomain(specDataSeries, [ - { - seriesType: 'area', - yScaleType: ScaleType.Linear, - groupId: getGroupId('a'), - id: getSpecId('a'), - stackAccessors: ['a'], - yScaleToDataExtent: true, - yDomain: { - max: 12, - min: 2, - }, - }, - ]); - expect(mergedDomain).toEqual([ - { - type: 'yDomain', - groupId: 'a', - domain: [2, 12], - scaleType: ScaleType.Linear, - isBandScale: false, - }, - ]); }); }); diff --git a/src/lib/series/domains/y_domain.ts b/src/lib/series/domains/y_domain.ts index dbdae31202..fc3e65b6ff 100644 --- a/src/lib/series/domains/y_domain.ts +++ b/src/lib/series/domains/y_domain.ts @@ -18,7 +18,6 @@ export type YBasicSeriesSpec = Pick< | 'id' | 'seriesType' | 'yScaleType' - | 'yDomain' | 'groupId' | 'stackAccessors' | 'yScaleToDataExtent' @@ -177,3 +176,21 @@ function coerceYScale(scaleTypes: Set): ScaleContinuousType } return ScaleType.Linear; } + +/** + * Coerce the y domain limits of a set of specification to a generic one. + * Given a set of domain limits, coerce the maximum + * @returns {ChartScaleType} + */ +export function coerceYDomain( + specs: Array>, +): ScaleContinuousType | null { + const scaleTypes = new Set(); + specs.forEach((spec) => { + scaleTypes.add(spec.yScaleType); + }); + if (specs.length === 0 || scaleTypes.size === 0) { + return null; + } + return coerceYScale(scaleTypes); +} diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 0096bf4069..b2b40de45e 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -37,10 +37,6 @@ export interface SeriesSpec { groupId: GroupId; /** An array of data */ data: Datum[]; - /** If specified, it constrant the x domain to these values */ - xDomain?: DomainRange; - /** If specified, it constrant the y Domain to these values */ - yDomain?: DomainRange; /** The type of series you are looking to render */ seriesType: 'bar' | 'line' | 'area' | 'basic'; /** Custom colors for series */ @@ -138,6 +134,10 @@ export interface AxisSpec { tickLabelRotation?: number; /** The axis title */ title?: string; + /** If specified, it constrains the x domain to these values */ + xDomain?: DomainRange; + /** If specified, it constrains the y domain to these values */ + yDomain?: DomainRange; } export type TickFormatter = (value: any) => string; diff --git a/stories/axis.tsx b/stories/axis.tsx index a2199e49f2..467f77f5f3 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -297,4 +297,54 @@ storiesOf('Axis', module) /> ); + }) + .add('customizing domain limits', () => { + const xDomainOptions = { + range: false, + max: 3, + min: 0, + step: 0.5, + }; + + const yDomainOptions = { + range: false, + max: 7, + min: 0, + step: 0.5, + }; + + const xDomain = { + min: number('xDomain min', 0, xDomainOptions), + max: number('xDomain max', 3, xDomainOptions), + }; + + const yDomain = { + min: number('yDomain min', 0, yDomainOptions), + max: number('yDomain max', 7, yDomainOptions), + }; + + const dg = new DataGenerator(); + const data = dg.generateSimpleSeries(31); + return ( + + + + + + ); }); diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx index 21b2370653..f08f57471f 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -1,4 +1,4 @@ -import { boolean, number } from '@storybook/addon-knobs'; +import { boolean } from '@storybook/addon-knobs'; import { storiesOf } from '@storybook/react'; import React from 'react'; import { @@ -825,63 +825,4 @@ storiesOf('Bar Chart', module) /> ); - }) - .add('with limited domain', () => { - const darkmode = boolean('darkmode', false); - const className = darkmode ? 'story-chart-dark' : 'story-chart'; - const defaultTheme = darkmode ? DARK_THEME : LIGHT_THEME; - - const xDomainOptions = { - range: false, - max: 3, - min: 0, - step: 0.5, - }; - - const yDomainOptions = { - range: false, - max: 7, - min: 0, - step: 0.5, - }; - - const xDomain = { - min: number('xDomain min', 0, xDomainOptions), - max: number('xDomain max', 3, xDomainOptions), - }; - - const yDomain = { - min: number('yDomain min', 0, yDomainOptions), - max: number('yDomain max', 7, yDomainOptions), - }; - - return ( - - - - Number(d).toFixed(2)} - /> - - - - ); }); diff --git a/stories/line_chart.tsx b/stories/line_chart.tsx index 4192466cab..21fb48b966 100644 --- a/stories/line_chart.tsx +++ b/stories/line_chart.tsx @@ -261,3 +261,4 @@ storiesOf('Line Chart', module) ); }); + From 4474a76c8fbb18ff4cb62a49e847ec5144d80727 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Wed, 13 Mar 2019 14:29:29 -0700 Subject: [PATCH 05/24] feat(y_domain): limit y domain using axis specs --- src/lib/axes/axis_utils.test.ts | 78 ++++++++++++++++++++++++++++-- src/lib/axes/axis_utils.ts | 58 ++++++++++++++++++++-- src/lib/series/domains/y_domain.ts | 30 ++++-------- src/lib/series/specs.ts | 7 +-- src/state/chart_state.ts | 7 ++- src/state/utils.test.ts | 4 +- src/state/utils.ts | 8 ++- stories/axis.tsx | 72 +++++++++++++++++---------- 8 files changed, 199 insertions(+), 65 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 003ab98b7a..da8362ac18 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -1,8 +1,8 @@ import { XDomain } from '../series/domains/x_domain'; import { YDomain } from '../series/domains/y_domain'; -import { Position } from '../series/specs'; +import { AxisSpec, DomainRange, Position } from '../series/specs'; import { LIGHT_THEME } from '../themes/light_theme'; -import { getAxisId, getGroupId } from '../utils/ids'; +import { getAxisId, getGroupId, GroupId } from '../utils/ids'; import { ScaleType } from '../utils/scales/scales'; import { centerRotationOrigin, @@ -23,6 +23,8 @@ import { getVerticalAxisTickLineProps, getVerticalDomain, getVisibleTicks, + isYDomain, + mergeDomainsByGroupId, } from './axis_utils'; import { CanvasTextBBoxCalculator } from './canvas_text_bbox_calculator'; import { SvgTextBBoxCalculator } from './svg_text_bbox_calculator'; @@ -71,7 +73,7 @@ describe('Axis computational utils', () => { maxLabelTextWidth: 10, maxLabelTextHeight: 10, }; - const verticalAxisSpec = { + const verticalAxisSpec: AxisSpec = { id: getAxisId('axis_1'), groupId: getGroupId('group_1'), hide: false, @@ -86,7 +88,7 @@ describe('Axis computational utils', () => { showGridLines: true, }; - const horizontalAxisSpec = { + const horizontalAxisSpec: AxisSpec = { id: getAxisId('axis_2'), groupId: getGroupId('group_1'), hide: false, @@ -923,4 +925,72 @@ describe('Axis computational utils', () => { expect(getHorizontalDomain(xDomain, [yDomain], skewChartRotation)).toEqual([yDomain]); expect(getVerticalDomain(xDomain, [yDomain], skewChartRotation)).toEqual(xDomain); }); + + test('should determine if axis belongs to yDomain', () => { + const verticalY = isYDomain(Position.Left, 0); + expect(verticalY).toBe(true); + + const verticalX = isYDomain(Position.Left, 90); + expect(verticalX).toBe(false); + + const horizontalX = isYDomain(Position.Top, 0); + expect(horizontalX).toBe(false); + + const horizontalY = isYDomain(Position.Top, 90); + expect(horizontalY).toBe(true); + }); + + test('should merge axis domains by group id', () => { + const groupId = getGroupId('group_1'); + const domainRange1 = { + min: 2, + max: 9, + }; + + verticalAxisSpec.domain = domainRange1; + + const axesSpecs = new Map(); + axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + // Base case + const expectedSimpleMap = new Map>(); + const expectedSimpleMapDomains = new Map(); + + expectedSimpleMapDomains.set('y', { min: 2, max: 9 }); + expectedSimpleMap.set(groupId, expectedSimpleMapDomains); + + const simpleDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + expect(simpleDomainsByGroupId).toEqual(expectedSimpleMap); + + // Multiple definitions for the same group + const domainRange2 = { + min: 0, + max: 7, + }; + + const altVerticalAxisSpec = { ...verticalAxisSpec, id: getAxisId('axis2') }; + + altVerticalAxisSpec.domain = domainRange2; + axesSpecs.set(altVerticalAxisSpec.id, altVerticalAxisSpec); + + const expectedMergedMap = new Map>(); + const expectedMergedMapDomains = new Map(); + expectedMergedMapDomains.set('y', { min: 0, max: 9 }); + expectedMergedMap.set(groupId, expectedMergedMapDomains); + + const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); + + // xDomain limit + horizontalAxisSpec.domain = { + min: 5, + max: 15, + }; + axesSpecs.set(horizontalAxisSpec.id, horizontalAxisSpec); + expectedMergedMapDomains.set('x', { min: 5, max: 15 }); + + const mergedMultiDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + + expect(mergedMultiDomainsByGroupId).toEqual(expectedMergedMap); + }); }); diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 5e255a2879..20a129b9e2 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -1,11 +1,11 @@ import { XDomain } from '../series/domains/x_domain'; import { YDomain } from '../series/domains/y_domain'; import { computeXScale, computeYScales } from '../series/scales'; -import { AxisSpec, Position, Rotation, TickFormatter } from '../series/specs'; +import { AxisSpec, DomainRange, Position, Rotation, TickFormatter } from '../series/specs'; import { AxisConfig, Theme } from '../themes/theme'; import { Dimensions, Margins } from '../utils/dimensions'; import { Domain } from '../utils/domain'; -import { AxisId } from '../utils/ids'; +import { AxisId, GroupId } from '../utils/ids'; import { Scale, ScaleType } from '../utils/scales/scales'; import { BBox, BBoxCalculator } from './bbox_calculator'; @@ -80,6 +80,16 @@ export function computeAxisTicksDimensions( ...dimensions, }; } + +export function isYDomain(position: Position, chartRotation: Rotation): boolean { + const isStraightRotation = chartRotation === 0 || chartRotation === 180; + if (isVertical(position)) { + return isStraightRotation; + } + + return !isStraightRotation; +} + export function getScaleForAxisSpec( axisSpec: AxisSpec, xDomain: XDomain, @@ -89,9 +99,9 @@ export function getScaleForAxisSpec( minRange: number, maxRange: number, ): Scale | null { - const axisDomain = getAxisDomain(axisSpec.position, xDomain, yDomain, chartRotation); - // If axisDomain is an array of values, this is an array of YDomains - if (Array.isArray(axisDomain)) { + const axisIsYDomain = isYDomain(axisSpec.position, chartRotation); + + if (axisIsYDomain) { const yScales = computeYScales(yDomain, minRange, maxRange); if (yScales.has(axisSpec.groupId)) { return yScales.get(axisSpec.groupId)!; @@ -642,3 +652,41 @@ export function isVertical(position: Position) { export function isHorizontal(position: Position) { return !isVertical(position); } + +// TODO: make domain type +export function mergeDomainsByGroupId( + axesSpecs: Map, + chartRotation: Rotation, +): Map> { + const domainsByGroupId = new Map>(); + axesSpecs.forEach((spec: AxisSpec, id: AxisId) => { + const { groupId, domain } = spec; + + if (domain) { + const prevGroupDomains = domainsByGroupId.get(groupId); + const isAxisYDomain = isYDomain(spec.position, chartRotation); + const domainKey = isAxisYDomain ? 'y' : 'x'; + + if (prevGroupDomains) { + const prevGroupDomainForAxis = prevGroupDomains.get(domainKey); + + if (prevGroupDomainForAxis) { + const mergedDomain = { + min: Math.min(domain.min, prevGroupDomainForAxis.min), + max: Math.max(domain.max, prevGroupDomainForAxis.max), + }; + + prevGroupDomains.set(domainKey, mergedDomain); + } else { + prevGroupDomains.set(domainKey, domain); + } + } else { + const axisDomain = new Map(); + axisDomain.set(domainKey, domain); + + domainsByGroupId.set(groupId, axisDomain); + } + } + }); + return domainsByGroupId; +} diff --git a/src/lib/series/domains/y_domain.ts b/src/lib/series/domains/y_domain.ts index fc3e65b6ff..ea24c0c676 100644 --- a/src/lib/series/domains/y_domain.ts +++ b/src/lib/series/domains/y_domain.ts @@ -4,7 +4,7 @@ import { computeContinuousDataDomain } from '../../utils/domain'; import { GroupId, SpecId } from '../../utils/ids'; import { ScaleContinuousType, ScaleType } from '../../utils/scales/scales'; import { RawDataSeries } from '../series'; -import { BasicSeriesSpec } from '../specs'; +import { BasicSeriesSpec, DomainRange } from '../specs'; import { BaseDomain } from './domain'; export type YDomain = BaseDomain & { @@ -27,6 +27,7 @@ export type YBasicSeriesSpec = Pick< export function mergeYDomain( dataSeries: Map, specs: YBasicSeriesSpec[], + domainsByGroupId: Map>, ): YDomain[] { // group specs by group ids const specsByGroupIds = splitSpecsByGroupId(specs); @@ -65,12 +66,16 @@ export function mergeYDomain( isStackedScaleToExtent || isNonStackedScaleToExtent, ); + const groupDomains = domainsByGroupId.get(groupId); + const limitedDomain = groupDomains && groupDomains.get('y'); + const domain = limitedDomain ? [limitedDomain.min, limitedDomain.max] : groupDomain; + return { type: 'yDomain', isBandScale: false, scaleType: groupYScaleType as ScaleContinuousType, groupId, - domain: groupDomain, + domain, }; }, ); @@ -162,7 +167,8 @@ export function coerceYScaleTypes( specs.forEach((spec) => { scaleTypes.add(spec.yScaleType); }); - if (specs.length === 0 || scaleTypes.size === 0) { + + if (specs.length === 0) { return null; } return coerceYScale(scaleTypes); @@ -176,21 +182,3 @@ function coerceYScale(scaleTypes: Set): ScaleContinuousType } return ScaleType.Linear; } - -/** - * Coerce the y domain limits of a set of specification to a generic one. - * Given a set of domain limits, coerce the maximum - * @returns {ChartScaleType} - */ -export function coerceYDomain( - specs: Array>, -): ScaleContinuousType | null { - const scaleTypes = new Set(); - specs.forEach((spec) => { - scaleTypes.add(spec.yScaleType); - }); - if (specs.length === 0 || scaleTypes.size === 0) { - return null; - } - return coerceYScale(scaleTypes); -} diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index b2b40de45e..9e86a68f71 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -1,6 +1,5 @@ import { GridLineConfig } from '../themes/theme'; import { Accessor } from '../utils/accessor'; -// import { Domain } from '../utils/domain'; import { AxisId, GroupId, SpecId } from '../utils/ids'; import { ScaleContinuousType, ScaleType } from '../utils/scales/scales'; import { CurveType } from './curves'; @@ -134,10 +133,8 @@ export interface AxisSpec { tickLabelRotation?: number; /** The axis title */ title?: string; - /** If specified, it constrains the x domain to these values */ - xDomain?: DomainRange; - /** If specified, it constrains the y domain to these values */ - yDomain?: DomainRange; + /** If specified, it constrains the domain for these values */ + domain?: DomainRange; } export type TickFormatter = (value: any) => string; diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 5ba087f6f7..14da3e3a6b 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -5,6 +5,7 @@ import { AxisTicksDimensions, computeAxisTicksDimensions, getAxisTicksPositions, + mergeDomainsByGroupId, } from '../lib/axes/axis_utils'; import { CanvasTextBBoxCalculator } from '../lib/axes/canvas_text_bbox_calculator'; import { XDomain } from '../lib/series/domains/x_domain'; @@ -440,9 +441,11 @@ export class ChartStore { this.selectedDataSeries = null; } - // The second argument is optional; if not supplied, then all series will be factored into computations + const domainsByGroupId = mergeDomainsByGroupId(this.axesSpecs, this.chartRotation); + + // The last argument is optional; if not supplied, then all series will be factored into computations // Otherwise, selectedDataSeries is used to restrict the computation for just the selected series - const seriesDomains = computeSeriesDomains(this.seriesSpecs, this.selectedDataSeries); + const seriesDomains = computeSeriesDomains(this.seriesSpecs, domainsByGroupId, this.selectedDataSeries); this.seriesDomainsAndData = seriesDomains; // If this.selectedDataSeries is null, initialize with all series diff --git a/src/state/utils.test.ts b/src/state/utils.test.ts index b253316319..4ecf1d78a8 100644 --- a/src/state/utils.test.ts +++ b/src/state/utils.test.ts @@ -42,7 +42,7 @@ describe('Chart State utils', () => { const specs = new Map(); specs.set(spec1.id, spec1); specs.set(spec2.id, spec2); - const domains = computeSeriesDomains(specs); + const domains = computeSeriesDomains(specs, new Map()); expect(domains.xDomain).toEqual({ domain: [0, 3], isBandScale: false, @@ -98,7 +98,7 @@ describe('Chart State utils', () => { const specs = new Map(); specs.set(spec1.id, spec1); specs.set(spec2.id, spec2); - const domains = computeSeriesDomains(specs); + const domains = computeSeriesDomains(specs, new Map()); expect(domains.xDomain).toEqual({ domain: [0, 3], isBandScale: false, diff --git a/src/state/utils.ts b/src/state/utils.ts index 761e9bb1d7..fdb829095a 100644 --- a/src/state/utils.ts +++ b/src/state/utils.ts @@ -28,6 +28,7 @@ import { AreaSeriesSpec, AxisSpec, BasicSeriesSpec, + DomainRange, LineSeriesSpec, Rotation, } from '../lib/series/specs'; @@ -106,6 +107,7 @@ export function getUpdatedCustomSeriesColors(seriesSpecs: Map, + domainsByGroupId: Map>, selectedDataSeries?: DataSeriesColorsValues[] | null, ): { xDomain: XDomain; @@ -122,8 +124,12 @@ export function computeSeriesDomains( // console.log({ splittedSeries, xValues, seriesColors }); const splittedDataSeries = [...splittedSeries.values()]; const specsArray = [...seriesSpecs.values()]; + const xDomain = mergeXDomain(specsArray, xValues); - const yDomain = mergeYDomain(splittedSeries, specsArray); + const yDomain = mergeYDomain(splittedSeries, specsArray, domainsByGroupId); + + // console.log('xDomain', xDomain); + // console.log('yDomain', yDomain); const formattedDataSeries = getFormattedDataseries(specsArray, splittedSeries); // tslint:disable-next-line:no-console // console.log({ formattedDataSeries, xDomain, yDomain }); diff --git a/stories/axis.tsx b/stories/axis.tsx index 467f77f5f3..36bd279d74 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -299,50 +299,72 @@ storiesOf('Axis', module) ); }) .add('customizing domain limits', () => { - const xDomainOptions = { - range: false, - max: 3, - min: 0, - step: 0.5, + const bottomDomain = { + min: number('bottom min', 0), + max: number('botttom max', 3), }; - const yDomainOptions = { - range: false, - max: 7, - min: 0, - step: 0.5, + const leftDomain = { + min: number('left min', 0), + max: number('left max', 7), }; - const xDomain = { - min: number('xDomain min', 0, xDomainOptions), - max: number('xDomain max', 3, xDomainOptions), + const right1Domain = { + min: number('right1 min', 0), + max: number('right1 max', 10), }; - const yDomain = { - min: number('yDomain min', 0, yDomainOptions), - max: number('yDomain max', 7, yDomainOptions), - }; - - const dg = new DataGenerator(); - const data = dg.generateSimpleSeries(31); return ( - + - Number(d).toFixed(2)} + domain={leftDomain} + /> + Number(d).toFixed(2)} + domain={right1Domain} + /> + Number(d).toFixed(2)} + /> + + From c708e0c63f03430b3c4c3e43e763b0110d742434 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Wed, 13 Mar 2019 15:03:09 -0700 Subject: [PATCH 06/24] refactor(axis_utils): remove unused functions and add test coverage --- src/lib/axes/axis_utils.test.ts | 20 ++++++++++-------- src/lib/axes/axis_utils.ts | 37 --------------------------------- 2 files changed, 11 insertions(+), 46 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index da8362ac18..bfcd3787ad 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -14,17 +14,17 @@ import { getAxisTicksPositions, getHorizontalAxisGridLineProps, getHorizontalAxisTickLineProps, - getHorizontalDomain, getMaxBboxDimensions, getMinMaxRange, getScaleForAxisSpec, getTickLabelProps, getVerticalAxisGridLineProps, getVerticalAxisTickLineProps, - getVerticalDomain, getVisibleTicks, isYDomain, mergeDomainsByGroupId, + isVertical, + isHorizontal, } from './axis_utils'; import { CanvasTextBBoxCalculator } from './canvas_text_bbox_calculator'; import { SvgTextBBoxCalculator } from './svg_text_bbox_calculator'; @@ -916,14 +916,16 @@ describe('Axis computational utils', () => { expect(horizontalAxisGridLines).toEqual([25, 0, 25, 100]); }); - test('should return correct domain based on rotation', () => { - const chartRotation = 180; - expect(getHorizontalDomain(xDomain, [yDomain], chartRotation)).toEqual(xDomain); - expect(getVerticalDomain(xDomain, [yDomain], chartRotation)).toEqual([yDomain]); + test('should determine orientation of axis position', () => { + expect(isVertical(Position.Left)).toBe(true); + expect(isVertical(Position.Right)).toBe(true); + expect(isVertical(Position.Top)).toBe(false); + expect(isVertical(Position.Bottom)).toBe(false); - const skewChartRotation = 45; - expect(getHorizontalDomain(xDomain, [yDomain], skewChartRotation)).toEqual([yDomain]); - expect(getVerticalDomain(xDomain, [yDomain], skewChartRotation)).toEqual(xDomain); + expect(isHorizontal(Position.Left)).toBe(false); + expect(isHorizontal(Position.Right)).toBe(false); + expect(isHorizontal(Position.Top)).toBe(true); + expect(isHorizontal(Position.Bottom)).toBe(true); }); test('should determine if axis belongs to yDomain', () => { diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 20a129b9e2..cf8cc222b9 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -608,43 +608,6 @@ export function computeAxisGridLinePositions( return positions; } -export function getVerticalDomain( - xDomain: XDomain, - yDomain: YDomain[], - chartRotation: number, -): XDomain | YDomain[] { - if (chartRotation === 0 || chartRotation === 180) { - return yDomain; - } else { - return xDomain; - } -} - -export function getHorizontalDomain( - xDomain: XDomain, - yDomain: YDomain[], - chartRotation: number, -): XDomain | YDomain[] { - if (chartRotation === 0 || chartRotation === 180) { - return xDomain; - } else { - return yDomain; - } -} - -export function getAxisDomain( - position: Position, - xDomain: XDomain, - yDomain: YDomain[], - chartRotation: number, -): XDomain | YDomain[] { - if (!isHorizontal(position)) { - return getVerticalDomain(xDomain, yDomain, chartRotation); - } else { - return getHorizontalDomain(xDomain, yDomain, chartRotation); - } -} - export function isVertical(position: Position) { return position === Position.Left || position === Position.Right; } From 208b3b3f8a199af33e03e5a16f3bea3ba2731156 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Wed, 13 Mar 2019 15:08:14 -0700 Subject: [PATCH 07/24] test(y_domain): add test for domain limiting --- src/lib/series/domains/y_domain.test.ts | 56 ++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/src/lib/series/domains/y_domain.test.ts b/src/lib/series/domains/y_domain.test.ts index 629dae20ac..6c4fcc0ea1 100644 --- a/src/lib/series/domains/y_domain.test.ts +++ b/src/lib/series/domains/y_domain.test.ts @@ -38,7 +38,7 @@ describe('Y Domain', () => { stackAccessors: ['a'], yScaleToDataExtent: true, }, - ]); + ], new Map()); expect(mergedDomain).toEqual([ { type: 'yDomain', @@ -92,7 +92,7 @@ describe('Y Domain', () => { stackAccessors: ['a'], yScaleToDataExtent: true, }, - ]); + ], new Map()); expect(mergedDomain).toEqual([ { groupId: 'a', @@ -153,7 +153,7 @@ describe('Y Domain', () => { stackAccessors: ['a'], yScaleToDataExtent: true, }, - ]); + ], new Map()); expect(mergedDomain).toEqual([ { groupId: 'a', @@ -206,7 +206,7 @@ describe('Y Domain', () => { id: getSpecId('b'), yScaleToDataExtent: true, }, - ]); + ], new Map()); expect(mergedDomain).toEqual([ { groupId: 'a', @@ -260,7 +260,7 @@ describe('Y Domain', () => { id: getSpecId('b'), yScaleToDataExtent: true, }, - ]); + ], new Map()); expect(mergedDomain.length).toEqual(1); }); test('Should split specs by groupId, two groups, non stacked', () => { @@ -417,7 +417,7 @@ describe('Y Domain', () => { expect(coerceYScaleTypes(specs)).toBe(null); }); - test.only('Should getDataSeriesOnGroup for matching specs', () => { + test('Should getDataSeriesOnGroup for matching specs', () => { const dataSeries: RawDataSeries[] = [ { specId: getSpecId('a'), @@ -447,4 +447,48 @@ describe('Y Domain', () => { const rawDataSeries = getDataSeriesOnGroup(specDataSeries, specs); expect(rawDataSeries).toEqual([]); }); + test('Should merge Y domain accounting for custom domain limits', () => { + const groupId = getGroupId('a'); + + const dataSeries: RawDataSeries[] = [ + { + specId: getSpecId('a'), + key: [''], + seriesColorKey: '', + data: [{ x: 1, y: 2 }, { x: 2, y: 2 }, { x: 3, y: 2 }, { x: 4, y: 5 }], + }, + { + specId: getSpecId('a'), + key: [''], + seriesColorKey: '', + data: [{ x: 1, y: 2 }, { x: 4, y: 7 }], + }, + ]; + const specDataSeries = new Map(); + specDataSeries.set(getSpecId('a'), dataSeries); + const domainsByGroupId = new Map(); + const yDomainConfig = new Map(); + yDomainConfig.set('y', { min: 0, max: 20 }); + domainsByGroupId.set(groupId, yDomainConfig); + + const mergedDomain = mergeYDomain(specDataSeries, [ + { + seriesType: 'area', + yScaleType: ScaleType.Linear, + groupId, + id: getSpecId('a'), + stackAccessors: ['a'], + yScaleToDataExtent: true, + }, + ], domainsByGroupId); + expect(mergedDomain).toEqual([ + { + type: 'yDomain', + groupId, + domain: [0, 20], + scaleType: ScaleType.Linear, + isBandScale: false, + }, + ]); + }); }); From eb91b5781a45c751a777ad18d4233d34dbcc17f7 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Thu, 14 Mar 2019 13:35:45 -0700 Subject: [PATCH 08/24] feat(x_domain): add custom x_domain via settings spec --- src/lib/axes/axis_utils.ts | 2 + src/lib/series/domains/x_domain.test.ts | 12 +++- src/lib/series/domains/x_domain.ts | 16 ++++- src/lib/series/domains/y_domain.ts | 4 +- src/specs/settings.tsx | 6 +- src/state/chart_state.ts | 11 +++- src/state/utils.ts | 6 +- stories/axis.tsx | 87 ++++++++++++++++++++++++- 8 files changed, 131 insertions(+), 13 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index cf8cc222b9..0fd393bf74 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -617,6 +617,8 @@ export function isHorizontal(position: Position) { } // TODO: make domain type +// TODO: only hold yDomain values (xDomain handle elsewhere), so return value can be Map +// TODO: if domain defined on an xDomain axis, do not add and console.warn export function mergeDomainsByGroupId( axesSpecs: Map, chartRotation: Rotation, diff --git a/src/lib/series/domains/x_domain.test.ts b/src/lib/series/domains/x_domain.test.ts index 30a2636784..aa4df33442 100644 --- a/src/lib/series/domains/x_domain.test.ts +++ b/src/lib/series/domains/x_domain.test.ts @@ -13,7 +13,7 @@ describe('X Domain', () => { test('should throw if we miss calling merge X domain without specs configured', () => { expect(() => { - mergeXDomain([], new Set()); + mergeXDomain([], new Set(), undefined); }).toThrow(); }); @@ -226,6 +226,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -264,6 +265,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -306,6 +308,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -348,6 +351,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -390,6 +394,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -432,6 +437,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -474,6 +480,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -516,6 +523,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -558,6 +566,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -603,6 +612,7 @@ describe('X Domain', () => { }, ], xValues, + undefined, ); expect(mergedDomain.domain.length).toEqual(maxValues); }); diff --git a/src/lib/series/domains/x_domain.ts b/src/lib/series/domains/x_domain.ts index 15799c21c4..6cfbf48265 100644 --- a/src/lib/series/domains/x_domain.ts +++ b/src/lib/series/domains/x_domain.ts @@ -1,7 +1,7 @@ import { compareByValueAsc, identity } from '../../utils/commons'; -import { computeContinuousDataDomain, computeOrdinalDataDomain } from '../../utils/domain'; +import { computeContinuousDataDomain, computeOrdinalDataDomain, Domain } from '../../utils/domain'; import { ScaleType } from '../../utils/scales/scales'; -import { BasicSeriesSpec } from '../specs'; +import { BasicSeriesSpec, DomainRange } from '../specs'; import { BaseDomain } from './domain'; export type XDomain = BaseDomain & { @@ -18,6 +18,7 @@ export type XDomain = BaseDomain & { export function mergeXDomain( specs: Array>, xValues: Set, + xDomain: DomainRange | Domain | undefined, ): XDomain { const mainXScaleType = convertXScaleTypes(specs); if (!mainXScaleType) { @@ -31,10 +32,21 @@ export function mergeXDomain( let minInterval = null; if (mainXScaleType.scaleType === ScaleType.Ordinal) { seriesXComputedDomains = computeOrdinalDataDomain(values, identity, false, true); + if (xDomain) { + if (Array.isArray(xDomain)) { + seriesXComputedDomains = xDomain; + } + } } else { seriesXComputedDomains = computeContinuousDataDomain(values, identity, true); + if (xDomain) { + if (!Array.isArray(xDomain)) { + seriesXComputedDomains = [xDomain.min, xDomain.max]; + } + } minInterval = findMinInterval(values); } + return { type: 'xDomain', scaleType: mainXScaleType.scaleType, diff --git a/src/lib/series/domains/y_domain.ts b/src/lib/series/domains/y_domain.ts index ea24c0c676..ecf526a61b 100644 --- a/src/lib/series/domains/y_domain.ts +++ b/src/lib/series/domains/y_domain.ts @@ -67,8 +67,8 @@ export function mergeYDomain( ); const groupDomains = domainsByGroupId.get(groupId); - const limitedDomain = groupDomains && groupDomains.get('y'); - const domain = limitedDomain ? [limitedDomain.min, limitedDomain.max] : groupDomain; + const customDomain = groupDomains && groupDomains.get('y'); + const domain = customDomain ? [customDomain.min, customDomain.max] : groupDomain; return { type: 'yDomain', diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index fd0543c528..c982b44c88 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -1,8 +1,9 @@ import { inject } from 'mobx-react'; import { PureComponent } from 'react'; -import { Position, Rendering, Rotation } from '../lib/series/specs'; +import { DomainRange, Position, Rendering, Rotation } from '../lib/series/specs'; import { LIGHT_THEME } from '../lib/themes/light_theme'; import { Theme } from '../lib/themes/theme'; +import { Domain } from '../lib/utils/domain'; import { BrushEndListener, ChartStore, @@ -29,6 +30,7 @@ interface SettingSpecProps { onLegendItemClick?: LegendItemListener; onLegendItemPlusClick?: LegendItemListener; onLegendItemMinusClick?: LegendItemListener; + xDomain?: Domain | DomainRange; } function updateChartStore(props: SettingSpecProps) { @@ -50,6 +52,7 @@ function updateChartStore(props: SettingSpecProps) { onLegendItemMinusClick, onLegendItemPlusClick, debug, + xDomain, } = props; if (!chartStore) { return; @@ -62,6 +65,7 @@ function updateChartStore(props: SettingSpecProps) { chartStore.setShowLegend(showLegend); chartStore.legendPosition = legendPosition; + chartStore.xDomain = xDomain; if (onElementOver) { chartStore.setOnElementOverListener(onElementOver); diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 14da3e3a6b..d38f1afe89 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -30,6 +30,7 @@ import { AxisSpec, BarSeriesSpec, BasicSeriesSpec, + DomainRange, LineSeriesSpec, Position, Rendering, @@ -39,6 +40,7 @@ import { formatTooltip } from '../lib/series/tooltip'; import { LIGHT_THEME } from '../lib/themes/light_theme'; import { Theme } from '../lib/themes/theme'; import { computeChartDimensions, Dimensions } from '../lib/utils/dimensions'; +import { Domain } from '../lib/utils/domain'; import { AxisId, GroupId, SpecId } from '../lib/utils/ids'; import { Scale, ScaleType } from '../lib/utils/scales/scales'; import { @@ -55,6 +57,7 @@ import { Transform, updateSelectedDataSeries, } from './utils'; + export interface TooltipPosition { top?: number; left?: number; @@ -129,6 +132,7 @@ export class ChartStore { seriesDomainsAndData?: SeriesDomainsAndData; // computed xScale?: Scale; yScales?: Map; + xDomain?: Domain | DomainRange; legendItems: LegendItem[] = []; highlightedLegendItemIndex: IObservableValue = observable.box(null); @@ -445,7 +449,12 @@ export class ChartStore { // The last argument is optional; if not supplied, then all series will be factored into computations // Otherwise, selectedDataSeries is used to restrict the computation for just the selected series - const seriesDomains = computeSeriesDomains(this.seriesSpecs, domainsByGroupId, this.selectedDataSeries); + const seriesDomains = computeSeriesDomains( + this.seriesSpecs, + domainsByGroupId, + this.xDomain, + this.selectedDataSeries, + ); this.seriesDomainsAndData = seriesDomains; // If this.selectedDataSeries is null, initialize with all series diff --git a/src/state/utils.ts b/src/state/utils.ts index fdb829095a..644bf35736 100644 --- a/src/state/utils.ts +++ b/src/state/utils.ts @@ -34,6 +34,7 @@ import { } from '../lib/series/specs'; import { ColorConfig } from '../lib/themes/theme'; import { Dimensions } from '../lib/utils/dimensions'; +import { Domain } from '../lib/utils/domain'; import { AxisId, GroupId, SpecId } from '../lib/utils/ids'; import { Scale } from '../lib/utils/scales/scales'; @@ -108,6 +109,7 @@ export function getUpdatedCustomSeriesColors(seriesSpecs: Map, domainsByGroupId: Map>, + customXDomain: DomainRange | Domain | undefined, selectedDataSeries?: DataSeriesColorsValues[] | null, ): { xDomain: XDomain; @@ -125,11 +127,9 @@ export function computeSeriesDomains( const splittedDataSeries = [...splittedSeries.values()]; const specsArray = [...seriesSpecs.values()]; - const xDomain = mergeXDomain(specsArray, xValues); + const xDomain = mergeXDomain(specsArray, xValues, customXDomain); const yDomain = mergeYDomain(splittedSeries, specsArray, domainsByGroupId); - // console.log('xDomain', xDomain); - // console.log('yDomain', yDomain); const formattedDataSeries = getFormattedDataseries(specsArray, splittedSeries); // tslint:disable-next-line:no-console // console.log({ formattedDataSeries, xDomain, yDomain }); diff --git a/stories/axis.tsx b/stories/axis.tsx index 36bd279d74..e2f43ebe83 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -298,7 +298,77 @@ storiesOf('Axis', module) ); }) - .add('customizing domain limits', () => { + .add('customizing domain limits [mixed chart]', () => { + const bottomDomain = { + min: number('bottom min', 0), + max: number('botttom max', 3), + }; + + const leftDomain = { + min: number('left min', 0), + max: number('left max', 7), + }; + + const rightDomain = { + min: number('right1 min', 0), + max: number('right1 max', 10), + }; + + const xDomain = { + min: number('xDomain min', 0), + max: number('xDomain max', 3), + }; + + return ( + + + + Number(d).toFixed(2)} + domain={leftDomain} + /> + Number(d).toFixed(2)} + domain={rightDomain} + /> + + + + ); + }) + .add('customizing domain limits [mixed ordinal & linear x domain]', () => { const bottomDomain = { min: number('bottom min', 0), max: number('botttom max', 3), @@ -314,9 +384,11 @@ storiesOf('Axis', module) max: number('right1 max', 10), }; + const xDomain = ['a', 'b', 'c', 'd', 3, 0]; + return ( - + Number(d).toFixed(2)} /> + {/* + /> */} Date: Thu, 14 Mar 2019 13:50:40 -0700 Subject: [PATCH 09/24] refactor(y_domain): define only y axis domain on axis spec --- src/lib/axes/axis_utils.test.ts | 14 ++++--------- src/lib/axes/axis_utils.ts | 28 ++++++++++--------------- src/lib/series/domains/y_domain.test.ts | 10 ++++----- src/lib/series/domains/y_domain.ts | 5 ++--- src/state/utils.test.ts | 4 ++-- src/state/utils.ts | 2 +- 6 files changed, 24 insertions(+), 39 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index bfcd3787ad..0079e0d7c2 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -955,11 +955,8 @@ describe('Axis computational utils', () => { axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); // Base case - const expectedSimpleMap = new Map>(); - const expectedSimpleMapDomains = new Map(); - - expectedSimpleMapDomains.set('y', { min: 2, max: 9 }); - expectedSimpleMap.set(groupId, expectedSimpleMapDomains); + const expectedSimpleMap = new Map(); + expectedSimpleMap.set(groupId, { min: 2, max: 9 }); const simpleDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); expect(simpleDomainsByGroupId).toEqual(expectedSimpleMap); @@ -975,10 +972,8 @@ describe('Axis computational utils', () => { altVerticalAxisSpec.domain = domainRange2; axesSpecs.set(altVerticalAxisSpec.id, altVerticalAxisSpec); - const expectedMergedMap = new Map>(); - const expectedMergedMapDomains = new Map(); - expectedMergedMapDomains.set('y', { min: 0, max: 9 }); - expectedMergedMap.set(groupId, expectedMergedMapDomains); + const expectedMergedMap = new Map(); + expectedMergedMap.set(groupId, { min: 0, max: 9 }); const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); @@ -989,7 +984,6 @@ describe('Axis computational utils', () => { max: 15, }; axesSpecs.set(horizontalAxisSpec.id, horizontalAxisSpec); - expectedMergedMapDomains.set('x', { min: 5, max: 15 }); const mergedMultiDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 0fd393bf74..f78cd70494 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -616,40 +616,34 @@ export function isHorizontal(position: Position) { return !isVertical(position); } -// TODO: make domain type // TODO: only hold yDomain values (xDomain handle elsewhere), so return value can be Map // TODO: if domain defined on an xDomain axis, do not add and console.warn export function mergeDomainsByGroupId( axesSpecs: Map, chartRotation: Rotation, -): Map> { - const domainsByGroupId = new Map>(); +): Map { + const domainsByGroupId = new Map(); axesSpecs.forEach((spec: AxisSpec, id: AxisId) => { const { groupId, domain } = spec; if (domain) { - const prevGroupDomains = domainsByGroupId.get(groupId); + const prevGroupDomain = domainsByGroupId.get(groupId); const isAxisYDomain = isYDomain(spec.position, chartRotation); - const domainKey = isAxisYDomain ? 'y' : 'x'; - if (prevGroupDomains) { - const prevGroupDomainForAxis = prevGroupDomains.get(domainKey); - - if (prevGroupDomainForAxis) { + if (prevGroupDomain) { + if (isAxisYDomain) { const mergedDomain = { - min: Math.min(domain.min, prevGroupDomainForAxis.min), - max: Math.max(domain.max, prevGroupDomainForAxis.max), + min: Math.min(domain.min, prevGroupDomain.min), + max: Math.max(domain.max, prevGroupDomain.max), }; - prevGroupDomains.set(domainKey, mergedDomain); + domainsByGroupId.set(groupId, mergedDomain); } else { - prevGroupDomains.set(domainKey, domain); + // tslint:disable-next-line:no-console + console.warn(`A custom domain was specified for an xDomain axis ${id}; instead define this in Settings`); } } else { - const axisDomain = new Map(); - axisDomain.set(domainKey, domain); - - domainsByGroupId.set(groupId, axisDomain); + domainsByGroupId.set(groupId, domain); } } }); diff --git a/src/lib/series/domains/y_domain.test.ts b/src/lib/series/domains/y_domain.test.ts index 6c4fcc0ea1..25e8c84448 100644 --- a/src/lib/series/domains/y_domain.test.ts +++ b/src/lib/series/domains/y_domain.test.ts @@ -1,7 +1,7 @@ -import { getGroupId, getSpecId } from '../../utils/ids'; +import { getGroupId, getSpecId, GroupId } from '../../utils/ids'; import { ScaleType } from '../../utils/scales/scales'; import { RawDataSeries } from '../series'; -import { BasicSeriesSpec } from '../specs'; +import { BasicSeriesSpec, DomainRange } from '../specs'; import { BARCHART_1Y0G } from '../utils/test_dataset'; import { coerceYScaleTypes, @@ -466,10 +466,8 @@ describe('Y Domain', () => { ]; const specDataSeries = new Map(); specDataSeries.set(getSpecId('a'), dataSeries); - const domainsByGroupId = new Map(); - const yDomainConfig = new Map(); - yDomainConfig.set('y', { min: 0, max: 20 }); - domainsByGroupId.set(groupId, yDomainConfig); + const domainsByGroupId = new Map(); + domainsByGroupId.set(groupId, { min: 0, max: 20 }); const mergedDomain = mergeYDomain(specDataSeries, [ { diff --git a/src/lib/series/domains/y_domain.ts b/src/lib/series/domains/y_domain.ts index ecf526a61b..cf98fd0711 100644 --- a/src/lib/series/domains/y_domain.ts +++ b/src/lib/series/domains/y_domain.ts @@ -27,7 +27,7 @@ export type YBasicSeriesSpec = Pick< export function mergeYDomain( dataSeries: Map, specs: YBasicSeriesSpec[], - domainsByGroupId: Map>, + domainsByGroupId: Map, ): YDomain[] { // group specs by group ids const specsByGroupIds = splitSpecsByGroupId(specs); @@ -66,8 +66,7 @@ export function mergeYDomain( isStackedScaleToExtent || isNonStackedScaleToExtent, ); - const groupDomains = domainsByGroupId.get(groupId); - const customDomain = groupDomains && groupDomains.get('y'); + const customDomain = domainsByGroupId.get(groupId); const domain = customDomain ? [customDomain.min, customDomain.max] : groupDomain; return { diff --git a/src/state/utils.test.ts b/src/state/utils.test.ts index 4ecf1d78a8..08ebaaada9 100644 --- a/src/state/utils.test.ts +++ b/src/state/utils.test.ts @@ -42,7 +42,7 @@ describe('Chart State utils', () => { const specs = new Map(); specs.set(spec1.id, spec1); specs.set(spec2.id, spec2); - const domains = computeSeriesDomains(specs, new Map()); + const domains = computeSeriesDomains(specs, new Map(), undefined); expect(domains.xDomain).toEqual({ domain: [0, 3], isBandScale: false, @@ -98,7 +98,7 @@ describe('Chart State utils', () => { const specs = new Map(); specs.set(spec1.id, spec1); specs.set(spec2.id, spec2); - const domains = computeSeriesDomains(specs, new Map()); + const domains = computeSeriesDomains(specs, new Map(), undefined); expect(domains.xDomain).toEqual({ domain: [0, 3], isBandScale: false, diff --git a/src/state/utils.ts b/src/state/utils.ts index 644bf35736..348aa5b2c4 100644 --- a/src/state/utils.ts +++ b/src/state/utils.ts @@ -108,7 +108,7 @@ export function getUpdatedCustomSeriesColors(seriesSpecs: Map, - domainsByGroupId: Map>, + domainsByGroupId: Map, customXDomain: DomainRange | Domain | undefined, selectedDataSeries?: DataSeriesColorsValues[] | null, ): { From 72b1b1c22283258a66f40de54a3d1993eacdae89 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Thu, 14 Mar 2019 15:30:29 -0700 Subject: [PATCH 10/24] test(axis_utils): add test for domain validation --- src/lib/axes/axis_utils.test.ts | 17 +++++++++++++++++ src/lib/axes/axis_utils.ts | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 0079e0d7c2..c850174e49 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -989,4 +989,21 @@ describe('Axis computational utils', () => { expect(mergedMultiDomainsByGroupId).toEqual(expectedMergedMap); }); + + test('should throw on invalid domain', () => { + const domainRange1 = { + min: 9, + max: 2, + }; + + verticalAxisSpec.domain = domainRange1; + + const axesSpecs = new Map(); + axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + const attemptToMerge = () => { mergeDomainsByGroupId(axesSpecs, 0); }; + const expectedError = '[Axis axis_1]: custom domain is invalid, min is greater than max'; + + expect(attemptToMerge).toThrowError(expectedError); + }); }); diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index f78cd70494..52c0d77467 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -627,6 +627,11 @@ export function mergeDomainsByGroupId( const { groupId, domain } = spec; if (domain) { + if (domain.min > domain.max) { + const errorMessage = `[Axis ${id}]: custom domain is invalid, min is greater than max`; + throw new Error(errorMessage); + } + const prevGroupDomain = domainsByGroupId.get(groupId); const isAxisYDomain = isYDomain(spec.position, chartRotation); From cff4d89ecc82c0d1aab0383f9a161304650efafd Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Thu, 14 Mar 2019 15:44:48 -0700 Subject: [PATCH 11/24] refactor(axis_utils): move xDomain check --- src/lib/axes/axis_utils.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 52c0d77467..50183b3376 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -627,30 +627,33 @@ export function mergeDomainsByGroupId( const { groupId, domain } = spec; if (domain) { + const isAxisYDomain = isYDomain(spec.position, chartRotation); + + if (!isAxisYDomain) { + // tslint:disable-next-line:no-console + console.warn(`[Axis ${id}]: custom domain for xDomain should be defined in Settings`); + return; + } + if (domain.min > domain.max) { const errorMessage = `[Axis ${id}]: custom domain is invalid, min is greater than max`; throw new Error(errorMessage); } const prevGroupDomain = domainsByGroupId.get(groupId); - const isAxisYDomain = isYDomain(spec.position, chartRotation); if (prevGroupDomain) { - if (isAxisYDomain) { - const mergedDomain = { - min: Math.min(domain.min, prevGroupDomain.min), - max: Math.max(domain.max, prevGroupDomain.max), - }; - - domainsByGroupId.set(groupId, mergedDomain); - } else { - // tslint:disable-next-line:no-console - console.warn(`A custom domain was specified for an xDomain axis ${id}; instead define this in Settings`); - } + const mergedDomain = { + min: Math.min(domain.min, prevGroupDomain.min), + max: Math.max(domain.max, prevGroupDomain.max), + }; + + domainsByGroupId.set(groupId, mergedDomain); } else { domainsByGroupId.set(groupId, domain); } } }); + return domainsByGroupId; } From 5b4357082cc244446ce1805ec81b5d30065582b1 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Thu, 14 Mar 2019 17:57:47 -0700 Subject: [PATCH 12/24] test(x_domain): add tests for custom domain --- src/lib/series/domains/x_domain.test.ts | 30 +++++++++++++++++++++++++ src/lib/series/domains/x_domain.ts | 9 ++++++++ 2 files changed, 39 insertions(+) diff --git a/src/lib/series/domains/x_domain.test.ts b/src/lib/series/domains/x_domain.test.ts index aa4df33442..77fd6be279 100644 --- a/src/lib/series/domains/x_domain.test.ts +++ b/src/lib/series/domains/x_domain.test.ts @@ -640,4 +640,34 @@ describe('X Domain', () => { const minInterval = findMinInterval([100]); expect(minInterval).toBe(1); }); + test('should account for custom domain when merging a linear domain', () => { + const xValues = new Set([1, 2, 3, 4, 5]); + const xDomain = { min: 0, max: 3 }; + const specs: Array> = + [{ seriesType: 'line', xScaleType: ScaleType.Linear }]; + + const basicMergedDomain = mergeXDomain(specs, xValues, xDomain); + expect(basicMergedDomain.domain).toEqual([0, 3]); + + const arrayXDomain = [1, 2]; + const unappliedArrayMergedDomain = mergeXDomain(specs, xValues, arrayXDomain); + expect(unappliedArrayMergedDomain.domain).toEqual([1, 5]); + + const invalidXDomain = { min: 10, max: 0 }; + const attemptToMerge = () => { mergeXDomain(specs, xValues, invalidXDomain); }; + expect(attemptToMerge).toThrowError('custom xDomain is invalid, min is greater than max'); + }); + + test('should account for custom domain when merging an ordinal domain', () => { + const xValues = new Set(['a', 'b', 'c', 'd']); + const xDomain = ['a', 'b', 'c']; + const specs: Array> = + [{ seriesType: 'bar', xScaleType: ScaleType.Ordinal }]; + const basicMergedDomain = mergeXDomain(specs, xValues, xDomain); + expect(basicMergedDomain.domain).toEqual(['a', 'b', 'c']); + + const objectXDomain = { max: 10, min: 0 }; + const unappliedArrayMergedDomain = mergeXDomain(specs, xValues, objectXDomain); + expect(unappliedArrayMergedDomain.domain).toEqual(['a', 'b', 'c', 'd']); + }); }); diff --git a/src/lib/series/domains/x_domain.ts b/src/lib/series/domains/x_domain.ts index 6cfbf48265..af4d73d35b 100644 --- a/src/lib/series/domains/x_domain.ts +++ b/src/lib/series/domains/x_domain.ts @@ -35,13 +35,22 @@ export function mergeXDomain( if (xDomain) { if (Array.isArray(xDomain)) { seriesXComputedDomains = xDomain; + } else { + // tslint:disable-next-line:no-console + console.warn('xDomain for ordinal scale should be an array of values, not a DomainRange object'); } } } else { seriesXComputedDomains = computeContinuousDataDomain(values, identity, true); if (xDomain) { if (!Array.isArray(xDomain)) { + if (xDomain.min > xDomain.max) { + throw new Error('custom xDomain is invalid, min is greater than max'); + } seriesXComputedDomains = [xDomain.min, xDomain.max]; + } else { + // tslint:disable-next-line:no-console + console.warn('xDomain for continuous scale should be a DomainRange object, not an array'); } } minInterval = findMinInterval(values); From 8702f0f4755fe8f821cee38fe1b3cdebf37c00e8 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Thu, 14 Mar 2019 18:13:34 -0700 Subject: [PATCH 13/24] docs(axis): add knobs for domain configurations --- stories/axis.tsx | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/stories/axis.tsx b/stories/axis.tsx index e2f43ebe83..f7e4b466a0 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -1,4 +1,4 @@ -import { boolean, number } from '@storybook/addon-knobs'; +import { boolean, number, array } from '@storybook/addon-knobs'; import { storiesOf } from '@storybook/react'; import React from 'react'; import { @@ -309,11 +309,16 @@ storiesOf('Axis', module) max: number('left max', 7), }; - const rightDomain = { + const rightDomain1 = { min: number('right1 min', 0), max: number('right1 max', 10), }; + const rightDomain2 = { + min: number('right2 min', 0), + max: number('right2 max', 10), + }; + const xDomain = { min: number('xDomain min', 0), max: number('xDomain max', 3), @@ -338,11 +343,19 @@ storiesOf('Axis', module) /> Number(d).toFixed(2)} + domain={rightDomain1} + /> + Number(d).toFixed(2)} - domain={rightDomain} + domain={rightDomain2} /> From 9d73e1fee7263ccf619578a403fe285789354ddb Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 15 Mar 2019 15:56:24 -0700 Subject: [PATCH 14/24] feat(axis): implement axis hide feature --- src/lib/axes/axis_utils.test.ts | 19 ++++++++++++-- src/lib/axes/axis_utils.ts | 4 +++ src/lib/utils/dimensions.test.ts | 43 ++++++++++++++++++++++++++++++++ src/lib/utils/dimensions.ts | 2 +- stories/axis.tsx | 6 ++++- 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index c850174e49..c87e21cc45 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -21,10 +21,10 @@ import { getVerticalAxisGridLineProps, getVerticalAxisTickLineProps, getVisibleTicks, + isHorizontal, + isVertical, isYDomain, mergeDomainsByGroupId, - isVertical, - isHorizontal, } from './axis_utils'; import { CanvasTextBBoxCalculator } from './canvas_text_bbox_calculator'; import { SvgTextBBoxCalculator } from './svg_text_bbox_calculator'; @@ -151,6 +151,21 @@ describe('Axis computational utils', () => { bboxCalculator.destroy(); }); + test('should not compute axis dimensions when spec is configured to hide', () => { + const bboxCalculator = new SvgTextBBoxCalculator(); + verticalAxisSpec.hide = true; + const axisDimensions = computeAxisTicksDimensions( + verticalAxisSpec, + xDomain, + [yDomain], + 1, + bboxCalculator, + 0, + axes, + ); + expect(axisDimensions).toBe(null); + }); + test('should compute dimensions for the bounding box containing a rotated label', () => { expect(computeRotatedLabelDimensions({ width: 1, height: 2 }, 0)).toEqual({ width: 1, diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 50183b3376..22d79e94e9 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -54,6 +54,10 @@ export function computeAxisTicksDimensions( chartRotation: Rotation, axisConfig: AxisConfig, ): AxisTicksDimensions | null { + if (axisSpec.hide) { + return null; + } + const scale = getScaleForAxisSpec( axisSpec, xDomain, diff --git a/src/lib/utils/dimensions.test.ts b/src/lib/utils/dimensions.test.ts index 137f80ca42..a235ef1fce 100644 --- a/src/lib/utils/dimensions.test.ts +++ b/src/lib/utils/dimensions.test.ts @@ -148,4 +148,47 @@ describe('Computed chart dimensions', () => { ); expect(chartDimensions).toMatchSnapshot(); }); + test('should not add space for axis when no spec for axis dimensions or axis is hidden', () => { + const axisDims = new Map(); + const axisSpecs = new Map(); + axisDims.set(getAxisId('foo'), axis1Dims); + axisSpecs.set(getAxisId('axis_1'), { + ...axisLeftSpec, + position: Position.Bottom, + }); + const chartDimensions = computeChartDimensions( + parentDim, + chartTheme, + axisDims, + axisSpecs, + showLegend, + ); + + const expectedDims = { + height: 60, + width: 60, + left: 20, + top: 20, + }; + + expect(chartDimensions).toEqual(expectedDims); + + const hiddenAxisDims = new Map(); + const hiddenAxisSpecs = new Map(); + hiddenAxisDims.set(getAxisId('axis_1'), axis1Dims); + hiddenAxisSpecs.set(getAxisId('axis_1'), { + ...axisLeftSpec, + hide: true, + position: Position.Bottom, + }); + const hiddenAxisChartDimensions = computeChartDimensions( + parentDim, + chartTheme, + axisDims, + axisSpecs, + showLegend, + ); + + expect(hiddenAxisChartDimensions).toEqual(expectedDims); + }); }); diff --git a/src/lib/utils/dimensions.ts b/src/lib/utils/dimensions.ts index e73bc2383e..c9fda2e3d5 100644 --- a/src/lib/utils/dimensions.ts +++ b/src/lib/utils/dimensions.ts @@ -44,7 +44,7 @@ export function computeChartDimensions( axisDimensions.forEach(({ maxLabelBboxWidth = 0, maxLabelBboxHeight = 0 }, id) => { const axisSpec = axisSpecs.get(id); - if (!axisSpec) { + if (!axisSpec || axisSpec.hide) { return; } const { position, tickSize, tickPadding } = axisSpec; diff --git a/stories/axis.tsx b/stories/axis.tsx index f7e4b466a0..841074628e 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -154,24 +154,28 @@ storiesOf('Axis', module) position={Position.Bottom} title={'bottom'} showOverlappingTicks={true} + hide={boolean('hide botttom axis', false)} /> Number(d).toFixed(2)} + hide={boolean('hide left axis', false)} /> Number(d).toFixed(2)} + hide={boolean('hide right axis', false)} /> ); }) - .add('customizing domain limits [mixed chart]', () => { + .add('customizing domain limits [mixed linear chart]', () => { const bottomDomain = { min: number('bottom min', 0), max: number('botttom max', 3), From b9e828b032dc90c01e60f9b1ddfa6e7d8be1ac2e Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 15 Mar 2019 16:53:38 -0700 Subject: [PATCH 15/24] docs(axis): add hide knob to custom domain story --- stories/axis.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stories/axis.tsx b/stories/axis.tsx index 841074628e..da6be73460 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -1,4 +1,4 @@ -import { boolean, number, array } from '@storybook/addon-knobs'; +import { array, boolean, number } from '@storybook/addon-knobs'; import { storiesOf } from '@storybook/react'; import React from 'react'; import { @@ -344,6 +344,7 @@ storiesOf('Axis', module) position={Position.Left} tickFormat={(d) => Number(d).toFixed(2)} domain={leftDomain} + hide={boolean('hide left axis', false)} /> Date: Sun, 17 Mar 2019 19:43:17 -0700 Subject: [PATCH 16/24] fix(rendering): exclude bar data which is not within ordinal x domain --- src/lib/series/rendering.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index 0b231f712a..7ec411abc3 100644 --- a/src/lib/series/rendering.ts +++ b/src/lib/series/rendering.ts @@ -98,8 +98,17 @@ export function renderBars( specId: SpecId, seriesKey: any[], ): BarGeometry[] { - return dataset.map((datum, i) => { + const barGeometries: BarGeometry[] = []; + const xDomain = xScale.domain; + const xScaleType = xScale.type; + + dataset.forEach((datum, i) => { const { x, y0, y1 } = datum; + + if (xScaleType === ScaleType.Ordinal && !xDomain.includes(x)) { + return; + } + let height = 0; let y = 0; if (yScale.type === ScaleType.Log) { @@ -116,7 +125,7 @@ export function renderBars( height = yScale.scale(y0) - y; } - return { + const barGeometry = { x: xScale.scale(x) + xScale.bandwidth * orderIndex, y, // top most value width: xScale.bandwidth, @@ -132,7 +141,11 @@ export function renderBars( seriesKey, }, }; + + barGeometries.push(barGeometry); }); + + return barGeometries; } export function renderLine( From ca861934e1d68b1b45df77c03267dd5f503fa782 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Mon, 18 Mar 2019 07:44:06 -0700 Subject: [PATCH 17/24] style: remove addressed TODOs --- src/lib/axes/axis_utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 22d79e94e9..7c6c76d587 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -620,8 +620,6 @@ export function isHorizontal(position: Position) { return !isVertical(position); } -// TODO: only hold yDomain values (xDomain handle elsewhere), so return value can be Map -// TODO: if domain defined on an xDomain axis, do not add and console.warn export function mergeDomainsByGroupId( axesSpecs: Map, chartRotation: Rotation, From e7bf8c4e77e6ce13d5883f917074e6642350f465 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Mon, 18 Mar 2019 08:07:41 -0700 Subject: [PATCH 18/24] style: remove superfluous extra line --- stories/line_chart.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/stories/line_chart.tsx b/stories/line_chart.tsx index 21fb48b966..4192466cab 100644 --- a/stories/line_chart.tsx +++ b/stories/line_chart.tsx @@ -261,4 +261,3 @@ storiesOf('Line Chart', module) ); }); - From faf9f25ee155a1c0d05edbf8de40a166ed61da47 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 19 Mar 2019 06:38:32 -0700 Subject: [PATCH 19/24] feat(axis_utils): throw when adding xDomain via axis spec --- src/lib/axes/axis_utils.test.ts | 6 +++--- src/lib/axes/axis_utils.ts | 5 ++--- stories/axis.tsx | 12 ------------ 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index c87e21cc45..68cfa780e8 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -993,16 +993,16 @@ describe('Axis computational utils', () => { const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); - // xDomain limit + // xDomain limit (bad config) horizontalAxisSpec.domain = { min: 5, max: 15, }; axesSpecs.set(horizontalAxisSpec.id, horizontalAxisSpec); - const mergedMultiDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + const attemptToMerge = () => { mergeDomainsByGroupId(axesSpecs, 0); }; - expect(mergedMultiDomainsByGroupId).toEqual(expectedMergedMap); + expect(attemptToMerge).toThrowError('[Axis axis_2]: custom domain for xDomain should be defined in Settings'); }); test('should throw on invalid domain', () => { diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 7c6c76d587..6b881c150f 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -632,9 +632,8 @@ export function mergeDomainsByGroupId( const isAxisYDomain = isYDomain(spec.position, chartRotation); if (!isAxisYDomain) { - // tslint:disable-next-line:no-console - console.warn(`[Axis ${id}]: custom domain for xDomain should be defined in Settings`); - return; + const errorMessage = `[Axis ${id}]: custom domain for xDomain should be defined in Settings`; + throw new Error(errorMessage); } if (domain.min > domain.max) { diff --git a/stories/axis.tsx b/stories/axis.tsx index da6be73460..b664770f66 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -303,11 +303,6 @@ storiesOf('Axis', module) ); }) .add('customizing domain limits [mixed linear chart]', () => { - const bottomDomain = { - min: number('bottom min', 0), - max: number('botttom max', 3), - }; - const leftDomain = { min: number('left min', 0), max: number('left max', 7), @@ -336,7 +331,6 @@ storiesOf('Axis', module) position={Position.Bottom} title={'Bottom axis'} showOverlappingTicks={true} - domain={bottomDomain} /> { - const bottomDomain = { - min: number('bottom min', 0), - max: number('botttom max', 3), - }; - const leftDomain = { min: number('left min', 0), max: number('left max', 7), @@ -412,7 +401,6 @@ storiesOf('Axis', module) position={Position.Bottom} title={'Bottom axis'} showOverlappingTicks={true} - domain={bottomDomain} /> Date: Tue, 19 Mar 2019 06:39:25 -0700 Subject: [PATCH 20/24] feat(x_domain): throw when custom domain is invalid --- src/lib/series/domains/x_domain.test.ts | 10 ++++++---- src/lib/series/domains/x_domain.ts | 6 ++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lib/series/domains/x_domain.test.ts b/src/lib/series/domains/x_domain.test.ts index 77fd6be279..cd7031a2d6 100644 --- a/src/lib/series/domains/x_domain.test.ts +++ b/src/lib/series/domains/x_domain.test.ts @@ -650,8 +650,9 @@ describe('X Domain', () => { expect(basicMergedDomain.domain).toEqual([0, 3]); const arrayXDomain = [1, 2]; - const unappliedArrayMergedDomain = mergeXDomain(specs, xValues, arrayXDomain); - expect(unappliedArrayMergedDomain.domain).toEqual([1, 5]); + const attemptToMergeArrayDomain = () => { mergeXDomain(specs, xValues, arrayXDomain); }; + const errorMessage = 'xDomain for continuous scale should be a DomainRange object, not an array'; + expect(attemptToMergeArrayDomain).toThrowError(errorMessage); const invalidXDomain = { min: 10, max: 0 }; const attemptToMerge = () => { mergeXDomain(specs, xValues, invalidXDomain); }; @@ -667,7 +668,8 @@ describe('X Domain', () => { expect(basicMergedDomain.domain).toEqual(['a', 'b', 'c']); const objectXDomain = { max: 10, min: 0 }; - const unappliedArrayMergedDomain = mergeXDomain(specs, xValues, objectXDomain); - expect(unappliedArrayMergedDomain.domain).toEqual(['a', 'b', 'c', 'd']); + const attemptToMerge = () => { mergeXDomain(specs, xValues, objectXDomain); }; + const errorMessage = 'xDomain for ordinal scale should be an array of values, not a DomainRange object'; + expect(attemptToMerge).toThrowError(errorMessage); }); }); diff --git a/src/lib/series/domains/x_domain.ts b/src/lib/series/domains/x_domain.ts index af4d73d35b..af687f0305 100644 --- a/src/lib/series/domains/x_domain.ts +++ b/src/lib/series/domains/x_domain.ts @@ -36,8 +36,7 @@ export function mergeXDomain( if (Array.isArray(xDomain)) { seriesXComputedDomains = xDomain; } else { - // tslint:disable-next-line:no-console - console.warn('xDomain for ordinal scale should be an array of values, not a DomainRange object'); + throw new Error('xDomain for ordinal scale should be an array of values, not a DomainRange object'); } } } else { @@ -49,8 +48,7 @@ export function mergeXDomain( } seriesXComputedDomains = [xDomain.min, xDomain.max]; } else { - // tslint:disable-next-line:no-console - console.warn('xDomain for continuous scale should be a DomainRange object, not an array'); + throw new Error('xDomain for continuous scale should be a DomainRange object, not an array'); } } minInterval = findMinInterval(values); From b49d76872f237896a174273146abe59e66bdc3a6 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 19 Mar 2019 08:41:10 -0700 Subject: [PATCH 21/24] test(axis_utils): use CanvasTextBboxCalculator instead of SVG --- src/lib/axes/axis_utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 68cfa780e8..e954a19a57 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -152,7 +152,7 @@ describe('Axis computational utils', () => { }); test('should not compute axis dimensions when spec is configured to hide', () => { - const bboxCalculator = new SvgTextBBoxCalculator(); + const bboxCalculator = new CanvasTextBBoxCalculator(); verticalAxisSpec.hide = true; const axisDimensions = computeAxisTicksDimensions( verticalAxisSpec, From 9c55397d940a64d27a1bd6880970b5bcb6b93a97 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 19 Mar 2019 09:41:30 -0700 Subject: [PATCH 22/24] refactor(axis_utils): return early if no domain --- src/lib/axes/axis_utils.ts | 43 ++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 6b881c150f..ac81e3ce88 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -625,34 +625,37 @@ export function mergeDomainsByGroupId( chartRotation: Rotation, ): Map { const domainsByGroupId = new Map(); + axesSpecs.forEach((spec: AxisSpec, id: AxisId) => { const { groupId, domain } = spec; - if (domain) { - const isAxisYDomain = isYDomain(spec.position, chartRotation); + if (!domain) { + return; + } - if (!isAxisYDomain) { - const errorMessage = `[Axis ${id}]: custom domain for xDomain should be defined in Settings`; - throw new Error(errorMessage); - } + const isAxisYDomain = isYDomain(spec.position, chartRotation); - if (domain.min > domain.max) { - const errorMessage = `[Axis ${id}]: custom domain is invalid, min is greater than max`; - throw new Error(errorMessage); - } + if (!isAxisYDomain) { + const errorMessage = `[Axis ${id}]: custom domain for xDomain should be defined in Settings`; + throw new Error(errorMessage); + } - const prevGroupDomain = domainsByGroupId.get(groupId); + if (domain.min > domain.max) { + const errorMessage = `[Axis ${id}]: custom domain is invalid, min is greater than max`; + throw new Error(errorMessage); + } - if (prevGroupDomain) { - const mergedDomain = { - min: Math.min(domain.min, prevGroupDomain.min), - max: Math.max(domain.max, prevGroupDomain.max), - }; + const prevGroupDomain = domainsByGroupId.get(groupId); - domainsByGroupId.set(groupId, mergedDomain); - } else { - domainsByGroupId.set(groupId, domain); - } + if (prevGroupDomain) { + const mergedDomain = { + min: Math.min(domain.min, prevGroupDomain.min), + max: Math.max(domain.max, prevGroupDomain.max), + }; + + domainsByGroupId.set(groupId, mergedDomain); + } else { + domainsByGroupId.set(groupId, domain); } }); From b3c5078e9092d2ad30188eaac25ed145386ee0c6 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 19 Mar 2019 09:57:36 -0700 Subject: [PATCH 23/24] refactor(x_domain): remove explicit undefineds from tests --- src/lib/series/domains/x_domain.test.ts | 12 +----------- src/lib/series/domains/x_domain.ts | 2 +- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/lib/series/domains/x_domain.test.ts b/src/lib/series/domains/x_domain.test.ts index cd7031a2d6..1a5bb20845 100644 --- a/src/lib/series/domains/x_domain.test.ts +++ b/src/lib/series/domains/x_domain.test.ts @@ -13,7 +13,7 @@ describe('X Domain', () => { test('should throw if we miss calling merge X domain without specs configured', () => { expect(() => { - mergeXDomain([], new Set(), undefined); + mergeXDomain([], new Set()); }).toThrow(); }); @@ -226,7 +226,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -265,7 +264,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -308,7 +306,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -351,7 +348,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -394,7 +390,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -437,7 +432,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -480,7 +474,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -523,7 +516,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 1, 2, 5, 7]); }); @@ -566,7 +558,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain).toEqual([0, 7]); }); @@ -612,7 +603,6 @@ describe('X Domain', () => { }, ], xValues, - undefined, ); expect(mergedDomain.domain.length).toEqual(maxValues); }); diff --git a/src/lib/series/domains/x_domain.ts b/src/lib/series/domains/x_domain.ts index af687f0305..6206833e2a 100644 --- a/src/lib/series/domains/x_domain.ts +++ b/src/lib/series/domains/x_domain.ts @@ -18,7 +18,7 @@ export type XDomain = BaseDomain & { export function mergeXDomain( specs: Array>, xValues: Set, - xDomain: DomainRange | Domain | undefined, + xDomain?: DomainRange | Domain, ): XDomain { const mainXScaleType = convertXScaleTypes(specs); if (!mainXScaleType) { From 5516d55790e688700de097f1ad354724c9049178 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 19 Mar 2019 10:06:11 -0700 Subject: [PATCH 24/24] refactor(utils): make domain parameter optional in computeSeriesDomain --- src/state/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/utils.ts b/src/state/utils.ts index 348aa5b2c4..aad3871c08 100644 --- a/src/state/utils.ts +++ b/src/state/utils.ts @@ -109,7 +109,7 @@ export function getUpdatedCustomSeriesColors(seriesSpecs: Map, domainsByGroupId: Map, - customXDomain: DomainRange | Domain | undefined, + customXDomain?: DomainRange | Domain, selectedDataSeries?: DataSeriesColorsValues[] | null, ): { xDomain: XDomain;