Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(timescale): consider timezone on axis ticks #151

Merged
merged 3 commits into from
Apr 8, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .storybook/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function loadStories() {
require('../stories/styling.tsx');
require('../stories/grid.tsx');
require('../stories/annotations.tsx');
require('../stories/scales.tsx');
}

configure(loadStories, module);
65 changes: 33 additions & 32 deletions src/lib/axes/axis_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,36 +144,39 @@ export const getMaxBboxDimensions = (
fontSize: number,
fontFamily: string,
tickLabelRotation: number,
) => (acc: { [key: string]: number }, tickLabel: string): {
maxLabelBboxWidth: number,
maxLabelBboxHeight: number,
maxLabelTextWidth: number,
maxLabelTextHeight: number,
) => (
acc: { [key: string]: number },
tickLabel: string,
): {
maxLabelBboxWidth: number;
maxLabelBboxHeight: number;
maxLabelTextWidth: number;
maxLabelTextHeight: number;
} => {
const bbox = bboxCalculator.compute(tickLabel, fontSize, fontFamily).getOrElse({
width: 0,
height: 0,
});
const bbox = bboxCalculator.compute(tickLabel, fontSize, fontFamily).getOrElse({
width: 0,
height: 0,
});

const rotatedBbox = computeRotatedLabelDimensions(bbox, tickLabelRotation);
const rotatedBbox = computeRotatedLabelDimensions(bbox, tickLabelRotation);

const width = Math.ceil(rotatedBbox.width);
const height = Math.ceil(rotatedBbox.height);
const labelWidth = Math.ceil(bbox.width);
const labelHeight = Math.ceil(bbox.height);
const width = Math.ceil(rotatedBbox.width);
const height = Math.ceil(rotatedBbox.height);
const labelWidth = Math.ceil(bbox.width);
const labelHeight = Math.ceil(bbox.height);

const prevWidth = acc.maxLabelBboxWidth;
const prevHeight = acc.maxLabelBboxHeight;
const prevLabelWidth = acc.maxLabelTextWidth;
const prevLabelHeight = acc.maxLabelTextHeight;
const prevWidth = acc.maxLabelBboxWidth;
const prevHeight = acc.maxLabelBboxHeight;
const prevLabelWidth = acc.maxLabelTextWidth;
const prevLabelHeight = acc.maxLabelTextHeight;

return {
maxLabelBboxWidth: prevWidth > width ? prevWidth : width,
maxLabelBboxHeight: prevHeight > height ? prevHeight : height,
maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth,
maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight,
};
return {
maxLabelBboxWidth: prevWidth > width ? prevWidth : width,
maxLabelBboxHeight: prevHeight > height ? prevHeight : height,
maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth,
maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight,
};
};

function computeTickDimensions(
scale: Scale,
Expand Down Expand Up @@ -562,11 +565,7 @@ export function getAxisTicksPositions(
}

const allTicks = getAvailableTicks(axisSpec, scale, totalGroupsCount);
const visibleTicks = getVisibleTicks(
allTicks,
axisSpec,
axisDim,
);
const visibleTicks = getVisibleTicks(allTicks, axisSpec, axisDim);

if (axisSpec.showGridLines) {
const isVerticalAxis = isVertical(axisSpec.position);
Expand Down Expand Up @@ -637,7 +636,9 @@ export function isUpperBound(domain: Partial<CompleteBoundedDomain>): domain is
return domain.max != null;
}

export function isCompleteBound(domain: Partial<CompleteBoundedDomain>): domain is CompleteBoundedDomain {
export function isCompleteBound(
domain: Partial<CompleteBoundedDomain>,
): domain is CompleteBoundedDomain {
return domain.max != null && domain.min != null;
}

Expand Down Expand Up @@ -675,8 +676,8 @@ export function mergeDomainsByGroupId(
if (prevGroupDomain) {
const prevDomain = prevGroupDomain as DomainRange;

const prevMin = (isLowerBound(prevDomain)) ? prevDomain.min : undefined;
const prevMax = (isUpperBound(prevDomain)) ? prevDomain.max : undefined;
const prevMin = isLowerBound(prevDomain) ? prevDomain.min : undefined;
const prevMax = isUpperBound(prevDomain) ? prevDomain.max : undefined;

let max = prevMax;
let min = prevMin;
Expand Down
1 change: 1 addition & 0 deletions src/lib/series/domains/domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ import { ScaleType } from '../../utils/scales/scales';
export interface BaseDomain {
scaleType: ScaleType;
domain: Domain;
/* if the scale needs to be a band scale: used when displaying bars */
isBandScale: boolean;
}
58 changes: 51 additions & 7 deletions src/lib/series/domains/x_domain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,61 @@ describe('X Domain', () => {
});
});
test('Should return correct scale type with single line (time)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType' | 'timeZone'>> = [
{
seriesType: 'line',
xScaleType: ScaleType.Time,
timeZone: 'utc-3',
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Time,
isBandScale: false,
timeZone: 'utc-3',
});
});
test('Should return correct scale type with multi line with same scale types (time) same tz', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType' | 'timeZone'>> = [
{
seriesType: 'line',
xScaleType: ScaleType.Time,
timeZone: 'UTC-3',
},
{
seriesType: 'line',
xScaleType: ScaleType.Time,
timeZone: 'utc-3',
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Time,
isBandScale: false,
timeZone: 'utc-3',
});
});
test('Should return correct scale type with multi line with same scale types (time) coerce to UTC', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType' | 'timeZone'>> = [
{
seriesType: 'line',
xScaleType: ScaleType.Time,
timeZone: 'utc-3',
},
{
seriesType: 'line',
xScaleType: ScaleType.Time,
timeZone: 'utc+3',
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
expect(mainXScale).toEqual({
scaleType: ScaleType.Time,
isBandScale: false,
timeZone: 'utc',
});
});

test('Should return correct scale type with multi line with different scale types (linear, ordinal)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
{
Expand Down Expand Up @@ -106,14 +149,15 @@ describe('X Domain', () => {
});
});
test('Should return correct scale type with multi bar, area with same scale types (linear, linear)', () => {
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>> = [
const seriesSpecs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType' | 'timeZone'>> = [
{
seriesType: 'bar',
xScaleType: ScaleType.Linear,
},
{
seriesType: 'area',
xScaleType: ScaleType.Time,
timeZone: 'utc+3',
},
];
const mainXScale = convertXScaleTypes(seriesSpecs);
Expand Down Expand Up @@ -272,11 +316,11 @@ describe('X Domain', () => {
[
{
seriesType: 'bar',
xScaleType: ScaleType.Time,
xScaleType: ScaleType.Linear,
},
{
seriesType: 'bar',
xScaleType: ScaleType.Time,
xScaleType: ScaleType.Linear,
},
],
xValues,
Expand Down Expand Up @@ -374,7 +418,7 @@ describe('X Domain', () => {
seriesType: 'bar',
xAccessor: 'x',
yAccessors: ['y'],
xScaleType: ScaleType.Linear,
xScaleType: ScaleType.Ordinal,
yScaleType: ScaleType.Linear,
yScaleToDataExtent: false,
data: [{ x: 0, y: 0 }, { x: 1, y: 0 }, { x: 2, y: 0 }, { x: 5, y: 0 }],
Expand Down Expand Up @@ -457,7 +501,7 @@ describe('X Domain', () => {
const ds1: BasicSeriesSpec = {
id: getSpecId('ds1'),
groupId: getGroupId('g1'),
seriesType: 'line',
seriesType: 'area',
xAccessor: 'x',
yAccessors: ['y'],
xScaleType: ScaleType.Linear,
Expand All @@ -471,7 +515,7 @@ describe('X Domain', () => {
seriesType: 'line',
xAccessor: 'x',
yAccessors: ['y'],
xScaleType: ScaleType.Linear,
xScaleType: ScaleType.Ordinal,
yScaleType: ScaleType.Linear,
yScaleToDataExtent: false,
data: new Array(maxValues).fill(0).map((d, i) => ({ x: i, y: i })),
Expand Down
29 changes: 23 additions & 6 deletions src/lib/series/domains/x_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { BaseDomain } from './domain';

export type XDomain = BaseDomain & {
type: 'xDomain';
/* if the scale needs to be a band scale: used when displaying bars */
isBandScale: boolean;
/* the minimum interval of the scale if not-ordinal band-scale*/
minInterval: number;
/** if x domain is time, we should also specify the timezone */
timeZone?: string;
};

/**
Expand All @@ -25,7 +25,6 @@ export function mergeXDomain(
if (!mainXScaleType) {
throw new Error('Cannot merge the domain. Missing X scale types');
}
// TODO: compute this domain merging also/overwritted by any configured static domains

const values = [...xValues.values()];
let seriesXComputedDomains;
Expand Down Expand Up @@ -81,6 +80,7 @@ export function mergeXDomain(
isBandScale: mainXScaleType.isBandScale,
domain: seriesXComputedDomains,
minInterval,
timeZone: mainXScaleType.timeZone,
};
}

Expand Down Expand Up @@ -119,20 +119,37 @@ export function findMinInterval(xValues: number[]): number {
* @returns {ChartScaleType}
*/
export function convertXScaleTypes(
specs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType'>>,
): Pick<XDomain, 'scaleType' | 'isBandScale'> | null {
specs: Array<Pick<BasicSeriesSpec, 'seriesType' | 'xScaleType' | 'timeZone'>>,
): {
scaleType: ScaleType;
isBandScale: boolean;
timeZone?: string;
} | null {
const seriesTypes = new Set<string>();
const scaleTypes = new Set<ScaleType>();
const timeZones = new Set<string>();
specs.forEach((spec) => {
seriesTypes.add(spec.seriesType);
scaleTypes.add(spec.xScaleType);
if (spec.timeZone) {
timeZones.add(spec.timeZone.toLowerCase());
}
});
if (specs.length === 0 || seriesTypes.size === 0 || scaleTypes.size === 0) {
return null;
}
const isBandScale = seriesTypes.has('bar');
if (scaleTypes.size === 1) {
return { scaleType: [...scaleTypes.values()][0], isBandScale };
const scaleType = scaleTypes.values().next().value;
let timeZone: string | undefined;
if (scaleType === ScaleType.Time) {
if (timeZones.size > 1) {
timeZone = 'utc';
} else {
timeZone = timeZones.values().next().value;
}
}
return { scaleType, isBandScale, timeZone };
}

if (scaleTypes.size > 1 && scaleTypes.has(ScaleType.Ordinal)) {
Expand Down
4 changes: 3 additions & 1 deletion src/lib/series/scales.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function computeXScale(
minRange: number,
maxRange: number,
): Scale {
const { scaleType, minInterval, domain, isBandScale } = xDomain;
const { scaleType, minInterval, domain, isBandScale, timeZone } = xDomain;
const rangeDiff = Math.abs(maxRange - minRange);
const isInverse = maxRange < minRange;
if (scaleType === ScaleType.Ordinal) {
Expand All @@ -74,6 +74,7 @@ export function computeXScale(
bandwidth / totalBarsInCluster,
false,
minInterval,
timeZone,
);
} else {
return createContinuousScale(
Expand All @@ -84,6 +85,7 @@ export function computeXScale(
0,
undefined,
minInterval,
timeZone,
);
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/lib/series/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ export interface SeriesScales {
* @default ScaleType.Ordinal
*/
xScaleType: ScaleType.Ordinal | ScaleType.Linear | ScaleType.Time;
/**
* If using a ScaleType.Time this timezone identifier is required to
* compute a nice set of xScale ticks. Can be any IANA zone supported by
* the host environment, or a fixed-offset name of the form 'utc+3',
* or the strings 'local' or 'utc'.
*/
timeZone?: string;
/**
* The y axis scale type
* @default ScaleType.Linear
Expand Down
13 changes: 1 addition & 12 deletions src/lib/utils/scales/scale_continuous.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,7 @@ import { ScaleBand } from './scale_band';
import { isLogarithmicScale, ScaleContinuous } from './scale_continuous';
import { ScaleType } from './scales';

describe.only('Scale Continuous', () => {
/**
* These tests cover the following cases:
* line/area with simple linear scale
* line/area chart with time scale (ac: w axis)
* barscale with linear scale (bc: with linear x axis)
* barscale with time scale (bc: with time x axis)
* bar + line with linear scale (mc: bar and lines)
* bar + line with time scale (missing story)
* bar clustered with time scale (bc: time clustered using various specs)
* bar clustered with linear scale (bc: clustered multiple series specs)
*/
describe('Scale Continuous', () => {
test('shall invert on continuous scale linear', () => {
const domain = [0, 2];
const minRange = 0;
Expand Down
Loading