Skip to content

Commit

Permalink
fix(timescale): consider timezone on axis ticks (opensearch-project#151)
Browse files Browse the repository at this point in the history
The d3-scale function, used to compute axis ticks, compute a discrete and nice rounded number of ticks on a time scale. By the way, this rounding is applied using UTC or local timezone, meaning
that we cannot display a nicely rounded tick if we want to display data in a timezone different from
the local or utc one. This commit includes a new optional prop to each series `timeZone` that can be used to configure this behaviour (default to utc).

fix opensearch-project#130
  • Loading branch information
markov00 authored Apr 8, 2019
1 parent d5081c2 commit e4ee93e
Show file tree
Hide file tree
Showing 13 changed files with 802 additions and 69 deletions.
1 change: 1 addition & 0 deletions packages/osd-charts/.storybook/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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 packages/osd-charts/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 packages/osd-charts/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 packages/osd-charts/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 packages/osd-charts/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 packages/osd-charts/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 packages/osd-charts/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
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

0 comments on commit e4ee93e

Please sign in to comment.