From d0b6b197cbc100a9e60199bfca6f437e45ed369c Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Mon, 25 Mar 2019 07:08:14 -0700 Subject: [PATCH] feat: allow partial custom domain (#116) --- src/lib/axes/axis_utils.test.ts | 142 +++++++++++++++++++ src/lib/axes/axis_utils.ts | 54 ++++++- src/lib/series/domains/x_domain.test.ts | 36 ++++- src/lib/series/domains/x_domain.ts | 24 +++- src/lib/series/domains/y_domain.test.ts | 178 +++++++++++++++++++++++- src/lib/series/domains/y_domain.ts | 22 ++- src/lib/series/specs.ts | 12 +- stories/axis.tsx | 46 ++++-- 8 files changed, 493 insertions(+), 21 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 94b5715398..38bb9c7220 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -21,6 +21,7 @@ import { getVerticalAxisGridLineProps, getVerticalAxisTickLineProps, getVisibleTicks, + isBounded, isHorizontal, isVertical, isYDomain, @@ -1034,6 +1035,134 @@ describe('Axis computational utils', () => { ); }); + test('should merge axis domains by group id: partial upper bounded prevDomain with complete domain', () => { + const groupId = getGroupId('group_1'); + const domainRange1 = { + max: 9, + }; + + const domainRange2 = { + min: 0, + max: 7, + }; + + verticalAxisSpec.domain = domainRange1; + + const axesSpecs = new Map(); + axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + const axis2 = { ...verticalAxisSpec, id: getAxisId('axis2') }; + + axis2.domain = domainRange2; + axesSpecs.set(axis2.id, axis2); + + const expectedMergedMap = new Map(); + expectedMergedMap.set(groupId, { min: 0, max: 9 }); + + const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); + }); + + test('should merge axis domains by group id: partial lower bounded prevDomain with complete domain', () => { + const groupId = getGroupId('group_1'); + const domainRange1 = { + min: -1, + }; + + const domainRange2 = { + min: 0, + max: 7, + }; + + verticalAxisSpec.domain = domainRange1; + + const axesSpecs = new Map(); + axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + const axis2 = { ...verticalAxisSpec, id: getAxisId('axis2') }; + + axis2.domain = domainRange2; + axesSpecs.set(axis2.id, axis2); + + const expectedMergedMap = new Map(); + expectedMergedMap.set(groupId, { min: -1, max: 7 }); + + const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); + }); + + test('should merge axis domains by group id: partial upper bounded prevDomain with lower bounded domain', () => { + const groupId = getGroupId('group_1'); + const domainRange1 = { + max: 9, + }; + + const domainRange2 = { + min: 0, + }; + + const domainRange3 = { + min: -1, + }; + + verticalAxisSpec.domain = domainRange1; + + const axesSpecs = new Map(); + axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + const axis2 = { ...verticalAxisSpec, id: getAxisId('axis2') }; + + axis2.domain = domainRange2; + axesSpecs.set(axis2.id, axis2); + + const axis3 = { ...verticalAxisSpec, id: getAxisId('axis3') }; + + axis3.domain = domainRange3; + axesSpecs.set(axis3.id, axis3); + + const expectedMergedMap = new Map(); + expectedMergedMap.set(groupId, { min: -1, max: 9 }); + + const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); + }); + + test('should merge axis domains by group id: partial lower bounded prevDomain with upper bounded domain', () => { + const groupId = getGroupId('group_1'); + const domainRange1 = { + min: 2, + }; + + const domainRange2 = { + max: 7, + }; + + const domainRange3 = { + max: 9, + }; + + verticalAxisSpec.domain = domainRange1; + + const axesSpecs = new Map(); + axesSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + const axis2 = { ...verticalAxisSpec, id: getAxisId('axis2') }; + + axis2.domain = domainRange2; + axesSpecs.set(axis2.id, axis2); + + const axis3 = { ...verticalAxisSpec, id: getAxisId('axis3') }; + + axis3.domain = domainRange3; + axesSpecs.set(axis3.id, axis3); + + const expectedMergedMap = new Map(); + expectedMergedMap.set(groupId, { min: 2, max: 9 }); + + const mergedDomainsByGroupId = mergeDomainsByGroupId(axesSpecs, 0); + expect(mergedDomainsByGroupId).toEqual(expectedMergedMap); + }); + test('should throw on invalid domain', () => { const domainRange1 = { min: 9, @@ -1052,4 +1181,17 @@ describe('Axis computational utils', () => { expect(attemptToMerge).toThrowError(expectedError); }); + + test('should determine that a domain has at least one bound', () => { + const lowerBounded = { + min: 0, + }; + + const upperBounded = { + max: 0, + }; + + expect(isBounded(lowerBounded)).toBe(true); + expect(isBounded(upperBounded)).toBe(true); + }); }); diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 50eb38fb72..7f8d95c84d 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -1,7 +1,16 @@ import { XDomain } from '../series/domains/x_domain'; import { YDomain } from '../series/domains/y_domain'; import { computeXScale, computeYScales } from '../series/scales'; -import { AxisSpec, DomainRange, Position, Rotation, TickFormatter } from '../series/specs'; +import { + AxisSpec, + CompleteBoundedDomain, + DomainRange, + LowerBoundedDomain, + Position, + Rotation, + TickFormatter, + UpperBoundedDomain, +} from '../series/specs'; import { AxisConfig, Theme } from '../themes/theme'; import { Dimensions, Margins } from '../utils/dimensions'; import { Domain } from '../utils/domain'; @@ -620,6 +629,22 @@ export function isHorizontal(position: Position) { return !isVertical(position); } +export function isLowerBound(domain: Partial): domain is LowerBoundedDomain { + return domain.min != null; +} + +export function isUpperBound(domain: Partial): domain is UpperBoundedDomain { + return domain.max != null; +} + +export function isCompleteBound(domain: Partial): domain is CompleteBoundedDomain { + return domain.max != null && domain.min != null; +} + +export function isBounded(domain: Partial): domain is DomainRange { + return domain.max != null || domain.min != null; +} + export function mergeDomainsByGroupId( axesSpecs: Map, chartRotation: Rotation, @@ -640,7 +665,7 @@ export function mergeDomainsByGroupId( throw new Error(errorMessage); } - if (domain.min > domain.max) { + if (isCompleteBound(domain) && domain.min > domain.max) { const errorMessage = `[Axis ${id}]: custom domain is invalid, min is greater than max`; throw new Error(errorMessage); } @@ -648,12 +673,31 @@ export function mergeDomainsByGroupId( const prevGroupDomain = domainsByGroupId.get(groupId); if (prevGroupDomain) { + const prevDomain = prevGroupDomain as DomainRange; + + const prevMin = (isLowerBound(prevDomain)) ? prevDomain.min : undefined; + const prevMax = (isUpperBound(prevDomain)) ? prevDomain.max : undefined; + + let max = prevMax; + let min = prevMin; + + if (isCompleteBound(domain)) { + min = prevMin != null ? Math.min(domain.min, prevMin) : domain.min; + max = prevMax != null ? Math.max(domain.max, prevMax) : domain.max; + } else if (isLowerBound(domain)) { + min = prevMin != null ? Math.min(domain.min, prevMin) : domain.min; + } else if (isUpperBound(domain)) { + max = prevMax != null ? Math.max(domain.max, prevMax) : domain.max; + } + const mergedDomain = { - min: Math.min(domain.min, prevGroupDomain.min), - max: Math.max(domain.max, prevGroupDomain.max), + min, + max, }; - domainsByGroupId.set(groupId, mergedDomain); + if (isBounded(mergedDomain)) { + domainsByGroupId.set(groupId, mergedDomain); + } } else { domainsByGroupId.set(groupId, domain); } diff --git a/src/lib/series/domains/x_domain.test.ts b/src/lib/series/domains/x_domain.test.ts index 1fc99fac20..10e2efb758 100644 --- a/src/lib/series/domains/x_domain.test.ts +++ b/src/lib/series/domains/x_domain.test.ts @@ -634,7 +634,7 @@ describe('X Domain', () => { const minInterval = findMinInterval([]); expect(minInterval).toBe(0); }); - test('should account for custom domain when merging a linear domain', () => { + test('should account for custom domain when merging a linear domain: complete bounded domain', () => { const xValues = new Set([1, 2, 3, 4, 5]); const xDomain = { min: 0, max: 3 }; const specs: Array> = [ @@ -659,6 +659,40 @@ describe('X Domain', () => { expect(attemptToMerge).toThrowError('custom xDomain is invalid, min is greater than max'); }); + test('should account for custom domain when merging a linear domain: lower bounded domain', () => { + const xValues = new Set([1, 2, 3, 4, 5]); + const xDomain = { min: 0 }; + const specs: Array> = [ + { seriesType: 'line', xScaleType: ScaleType.Linear }, + ]; + + const mergedDomain = mergeXDomain(specs, xValues, xDomain); + expect(mergedDomain.domain).toEqual([0, 5]); + + const invalidXDomain = { min: 10 }; + const attemptToMerge = () => { + mergeXDomain(specs, xValues, invalidXDomain); + }; + expect(attemptToMerge).toThrowError('custom xDomain is invalid, custom min is greater than computed max'); + }); + + test('should account for custom domain when merging a linear domain: upper bounded domain', () => { + const xValues = new Set([1, 2, 3, 4, 5]); + const xDomain = { max: 3 }; + const specs: Array> = [ + { seriesType: 'line', xScaleType: ScaleType.Linear }, + ]; + + const mergedDomain = mergeXDomain(specs, xValues, xDomain); + expect(mergedDomain.domain).toEqual([1, 3]); + + const invalidXDomain = { max: -1 }; + const attemptToMerge = () => { + mergeXDomain(specs, xValues, invalidXDomain); + }; + expect(attemptToMerge).toThrowError('custom xDomain is invalid, computed min is greater than custom 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']; diff --git a/src/lib/series/domains/x_domain.ts b/src/lib/series/domains/x_domain.ts index 4fffbee487..fba4d039e1 100644 --- a/src/lib/series/domains/x_domain.ts +++ b/src/lib/series/domains/x_domain.ts @@ -1,3 +1,4 @@ +import { isCompleteBound, isLowerBound, isUpperBound } from '../../axes/axis_utils'; import { compareByValueAsc, identity } from '../../utils/commons'; import { computeContinuousDataDomain, computeOrdinalDataDomain, Domain } from '../../utils/domain'; import { ScaleType } from '../../utils/scales/scales'; @@ -42,10 +43,27 @@ export function mergeXDomain( 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'); + const [computedDomainMin, computedDomainMax] = seriesXComputedDomains; + + if (isCompleteBound(xDomain)) { + if (xDomain.min > xDomain.max) { + throw new Error('custom xDomain is invalid, min is greater than max'); + } + + seriesXComputedDomains = [xDomain.min, xDomain.max]; + } else if (isLowerBound(xDomain)) { + if (xDomain.min > computedDomainMax) { + throw new Error('custom xDomain is invalid, custom min is greater than computed max'); + } + + seriesXComputedDomains = [xDomain.min, computedDomainMax]; + } else if (isUpperBound(xDomain)) { + if (computedDomainMin > xDomain.max) { + throw new Error('custom xDomain is invalid, computed min is greater than custom max'); + } + + seriesXComputedDomains = [computedDomainMin, xDomain.max]; } - seriesXComputedDomains = [xDomain.min, xDomain.max]; } else { throw new Error('xDomain for continuous scale should be a DomainRange object, not an array'); } diff --git a/src/lib/series/domains/y_domain.test.ts b/src/lib/series/domains/y_domain.test.ts index 49fde02a00..e0601a56e2 100644 --- a/src/lib/series/domains/y_domain.test.ts +++ b/src/lib/series/domains/y_domain.test.ts @@ -469,7 +469,7 @@ describe('Y Domain', () => { const rawDataSeries = getDataSeriesOnGroup(specDataSeries, specs); expect(rawDataSeries).toEqual([]); }); - test('Should merge Y domain accounting for custom domain limits', () => { + test('Should merge Y domain accounting for custom domain limits: complete bounded domain', () => { const groupId = getGroupId('a'); const dataSeries: RawDataSeries[] = [ @@ -515,4 +515,180 @@ describe('Y Domain', () => { }, ]); }); + test('Should merge Y domain accounting for custom domain limits: partial lower bounded domain', () => { + 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(); + domainsByGroupId.set(groupId, { min: 0 }); + + 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, 12], + scaleType: ScaleType.Linear, + isBandScale: false, + }, + ]); + }); + test('Should not merge Y domain with invalid custom domain limits: partial lower bounded domain', () => { + 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(); + domainsByGroupId.set(groupId, { min: 20 }); + + const attemptToMerge = () => { + mergeYDomain( + specDataSeries, + [ + { + seriesType: 'area', + yScaleType: ScaleType.Linear, + groupId, + id: getSpecId('a'), + stackAccessors: ['a'], + yScaleToDataExtent: true, + }, + ], + domainsByGroupId, + ); + }; + + const errorMessage = 'custom yDomain for a is invalid, custom min is greater than computed max'; + expect(attemptToMerge).toThrowError(errorMessage); + }); + test('Should merge Y domain accounting for custom domain limits: partial upper bounded domain', () => { + 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(); + domainsByGroupId.set(groupId, { max: 20 }); + + 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: [2, 20], + scaleType: ScaleType.Linear, + isBandScale: false, + }, + ]); + }); + test('Should not merge Y domain with invalid custom domain limits: partial upper bounded domain', () => { + 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(); + domainsByGroupId.set(groupId, { max: -1 }); + + const attemptToMerge = () => { + mergeYDomain( + specDataSeries, + [ + { + seriesType: 'area', + yScaleType: ScaleType.Linear, + groupId, + id: getSpecId('a'), + stackAccessors: ['a'], + yScaleToDataExtent: true, + }, + ], + domainsByGroupId, + ); + }; + + const errorMessage = 'custom yDomain for a is invalid, computed min is greater than custom max'; + expect(attemptToMerge).toThrowError(errorMessage); + }); }); diff --git a/src/lib/series/domains/y_domain.ts b/src/lib/series/domains/y_domain.ts index da1e06570a..0c2006fad4 100644 --- a/src/lib/series/domains/y_domain.ts +++ b/src/lib/series/domains/y_domain.ts @@ -1,4 +1,5 @@ import { sum } from 'd3-array'; +import { isCompleteBound, isLowerBound, isUpperBound } from '../../axes/axis_utils'; import { identity } from '../../utils/commons'; import { computeContinuousDataDomain } from '../../utils/domain'; import { GroupId, SpecId } from '../../utils/ids'; @@ -63,8 +64,27 @@ export function mergeYDomain( isStackedScaleToExtent || isNonStackedScaleToExtent, ); + const [computedDomainMin, computedDomainMax] = groupDomain; + let domain = groupDomain; + const customDomain = domainsByGroupId.get(groupId); - const domain = customDomain ? [customDomain.min, customDomain.max] : groupDomain; + + if (customDomain && isCompleteBound(customDomain)) { + // Don't need to check min > max because this has been validated on axis domain merge + domain = [customDomain.min, customDomain.max]; + } else if (customDomain && isLowerBound(customDomain)) { + if (customDomain.min > computedDomainMax) { + throw new Error(`custom yDomain for ${groupId} is invalid, custom min is greater than computed max`); + } + + domain = [customDomain.min, computedDomainMax]; + } else if (customDomain && isUpperBound(customDomain)) { + if (computedDomainMin > customDomain.max) { + throw new Error(`custom yDomain for ${groupId} is invalid, computed min is greater than custom max`); + } + + domain = [computedDomainMin, customDomain.max]; + } return { type: 'yDomain', diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index e975dce8a7..7345464057 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -9,11 +9,21 @@ export type Datum = any; export type Rotation = 0 | 90 | -90 | 180; export type Rendering = 'canvas' | 'svg'; -export interface DomainRange { +export interface LowerBoundedDomain { min: number; +} + +export interface UpperBoundedDomain { max: number; } +export interface CompleteBoundedDomain { + min: number; + max: number; +} + +export type DomainRange = LowerBoundedDomain | UpperBoundedDomain | CompleteBoundedDomain; + export interface SeriesSpec { /** The ID of the spec, generated via getSpecId method */ id: SpecId; diff --git a/stories/axis.tsx b/stories/axis.tsx index b664770f66..04c776799c 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -433,15 +433,6 @@ storiesOf('Axis', module) data={[{ x: 'a', y: 2 }, { x: 'b', y: 7 }, { x: 'c', y: 3 }, { x: 'd', y: 6 }]} yScaleToDataExtent={false} /> - {/* */} ); + }) + .add('customizing domain limits [only one bound defined]', () => { + const leftDomain = { + min: number('left min', 0), + }; + + const xDomain = { + max: number('xDomain max', 3), + }; + + return ( + + + + Number(d).toFixed(2)} + domain={leftDomain} + /> + + + ); });