From 8015f5eef821c9daed4a60aa8212dea5d5130170 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Mon, 21 Oct 2019 17:27:54 +0200 Subject: [PATCH 1/2] fix(tickformatter): add timeZone to tickFormatter to correctly handle a the tick formatting on time based series, we need to share the timeZone parameter to the tickFormatter. When the tickFormatter is an external function, the user can manually configure that parameter, but for the niceTimeFormatter we don't have that option already. Instead of adding a breaking change I've added an optional parameter to the tickFormat props to share the configured timeZone parameter fix #427 --- .playground/playgroud.tsx | 40 ++++-------- src/chart_types/xy_chart/tooltip/tooltip.ts | 13 +++- .../xy_chart/utils/axis_utils.test.ts | 65 +++++++++++++++++++ src/chart_types/xy_chart/utils/axis_utils.ts | 21 ++++-- src/chart_types/xy_chart/utils/specs.ts | 5 +- src/utils/data/formatters.ts | 12 ++-- 6 files changed, 115 insertions(+), 41 deletions(-) diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index b819393c17..846a259c40 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -1,5 +1,6 @@ import React, { Fragment } from 'react'; -import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, BarSeries, Settings } from '../src'; +import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, BarSeries, Settings, niceTimeFormatter } from '../src'; +import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana'; export class Playground extends React.Component<{}, { dataLimit: boolean }> { state = { @@ -13,21 +14,7 @@ export class Playground extends React.Component<{}, { dataLimit: boolean }> { }); }; render() { - const data = [ - { - g: null, - i: 'aa', - x: 1571212800000, - y: 16, - y1: 2, - }, - // { - // x: 1571290200000, - // y: 1, - // y1: 5, - // // g: 'authentication_success', - // }, - ]; + const { data } = KIBANA_METRICS.metrics.kibana_os_load[0]; return (
@@ -36,23 +23,22 @@ export class Playground extends React.Component<{}, { dataLimit: boolean }> {
- Number(d).toFixed(2)} + id={getAxisId('top')} + position={Position.Bottom} + title={'Top axis'} + tickFormat={niceTimeFormatter([data[0][0], data[data.length - 1][0]])} /> +
diff --git a/src/chart_types/xy_chart/tooltip/tooltip.ts b/src/chart_types/xy_chart/tooltip/tooltip.ts index 63f5eee4bd..607a51ecd2 100644 --- a/src/chart_types/xy_chart/tooltip/tooltip.ts +++ b/src/chart_types/xy_chart/tooltip/tooltip.ts @@ -1,7 +1,15 @@ import { TooltipValue, isFollowTooltipType, TooltipType, TooltipValueFormatter } from '../utils/interactions'; import { IndexedGeometry, isPointOnGeometry, AccessorType } from '../rendering/rendering'; import { getColorValuesAsString } from '../utils/series'; -import { AxisSpec, BasicSeriesSpec, Rotation, isAreaSeriesSpec, isBandedSpec, isBarSeriesSpec } from '../utils/specs'; +import { + AxisSpec, + BasicSeriesSpec, + Rotation, + isAreaSeriesSpec, + isBandedSpec, + isBarSeriesSpec, + TickFormatterOptions, +} from '../utils/specs'; import { SpecId, AxisId, GroupId } from '../../../utils/ids'; import { getAxesSpecForSpecId } from '../store/utils'; import { Scale } from '../../../utils/scales/scales'; @@ -65,10 +73,11 @@ export function formatTooltip( } const value = isXValue ? x : y; + const tickFormatOptions: TickFormatterOptions | undefined = spec.timeZone ? { timeZone: spec.timeZone } : undefined; return { seriesKey: seriesKeyAsString, name: displayName, - value: axisSpec ? axisSpec.tickFormat(value) : emptyFormatter(value), + value: axisSpec ? axisSpec.tickFormat(value, tickFormatOptions) : emptyFormatter(value), color, isHighlighted: isXValue ? false : isHighlighted, isXValue, diff --git a/src/chart_types/xy_chart/utils/axis_utils.test.ts b/src/chart_types/xy_chart/utils/axis_utils.test.ts index b83708b4f0..b2b57f2474 100644 --- a/src/chart_types/xy_chart/utils/axis_utils.test.ts +++ b/src/chart_types/xy_chart/utils/axis_utils.test.ts @@ -34,6 +34,7 @@ import { } from './axis_utils'; import { CanvasTextBBoxCalculator } from '../../../utils/bbox/canvas_text_bbox_calculator'; import { SvgTextBBoxCalculator } from '../../../utils/bbox/svg_text_bbox_calculator'; +import { niceTimeFormatter } from '../../../utils/data/formatters'; describe('Axis computational utils', () => { const mockedRect = { @@ -119,6 +120,20 @@ describe('Axis computational utils', () => { showGridLines: true, integersOnly: false, }; + const xAxisWithTime: AxisSpec = { + id: getAxisId('axis_1'), + groupId: getGroupId('group_1'), + title: 'v axis', + hide: false, + showOverlappingTicks: false, + showOverlappingLabels: false, + position: Position.Bottom, + tickSize: 10, + tickPadding: 10, + tickFormat: niceTimeFormatter([1551438000000, 1551441300000]), + showGridLines: true, + integersOnly: false, + }; // const horizontalAxisSpecWTitle: AxisSpec = { // id: getAxisId('axis_2'), @@ -175,6 +190,56 @@ describe('Axis computational utils', () => { expect(axisDimensions).toBe(null); }); + test('should compute axis dimensions with timeZone', () => { + const bboxCalculator = new SvgTextBBoxCalculator(); + const xDomain: XDomain = { + type: 'xDomain', + scaleType: ScaleType.Time, + domain: [1551438000000, 1551441300000], + isBandScale: false, + minInterval: 0, + timeZone: 'utc', + }; + let axisDimensions = computeAxisTicksDimensions(xAxisWithTime, xDomain, [yDomain], 1, bboxCalculator, 0, axes); + expect(axisDimensions).not.toBeNull(); + expect(axisDimensions!.tickLabels[0]).toBe('11:00:00'); + expect(axisDimensions!.tickLabels[11]).toBe('11:55:00'); + + axisDimensions = computeAxisTicksDimensions( + xAxisWithTime, + { + ...xDomain, + timeZone: 'utc+3', + }, + [yDomain], + 1, + bboxCalculator, + 0, + axes, + ); + expect(axisDimensions).not.toBeNull(); + expect(axisDimensions!.tickLabels[0]).toBe('14:00:00'); + expect(axisDimensions!.tickLabels[11]).toBe('14:55:00'); + + axisDimensions = computeAxisTicksDimensions( + xAxisWithTime, + { + ...xDomain, + timeZone: 'utc-3', + }, + [yDomain], + 1, + bboxCalculator, + 0, + axes, + ); + expect(axisDimensions).not.toBeNull(); + expect(axisDimensions!.tickLabels[0]).toBe('08:00:00'); + expect(axisDimensions!.tickLabels[11]).toBe('08:55:00'); + + bboxCalculator.destroy(); + }); + 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/chart_types/xy_chart/utils/axis_utils.ts b/src/chart_types/xy_chart/utils/axis_utils.ts index 804583c7e2..eabff98bf2 100644 --- a/src/chart_types/xy_chart/utils/axis_utils.ts +++ b/src/chart_types/xy_chart/utils/axis_utils.ts @@ -11,6 +11,7 @@ import { TickFormatter, UpperBoundedDomain, AxisStyle, + TickFormatterOptions, } from './specs'; import { AxisConfig, Theme } from '../../../utils/themes/theme'; import { Dimensions, Margins } from '../../../utils/dimensions'; @@ -91,6 +92,9 @@ export function computeAxisTicksDimensions( axisConfig, tickLabelPadding, axisSpec.tickLabelRotation, + { + timeZone: xDomain.timeZone, + }, ); return { @@ -211,9 +215,12 @@ function computeTickDimensions( axisConfig: AxisConfig, tickLabelPadding: number, tickLabelRotation = 0, + tickFormatOptions?: TickFormatterOptions, ) { const tickValues = scale.ticks(); - const tickLabels = tickValues.map(tickFormat); + const tickLabels = tickValues.map((d) => { + return tickFormat(d, tickFormatOptions); + }); const { tickLabelStyle: { fontFamily, fontSize }, @@ -404,6 +411,7 @@ export function getAvailableTicks( scale: Scale, totalBarsInCluster: number, enableHistogramMode: boolean, + tickFormatOptions?: TickFormatterOptions, ): AxisTick[] { const ticks = scale.ticks(); const isSingleValueScale = scale.domain[0] - scale.domain[1] === 0; @@ -433,14 +441,14 @@ export function getAvailableTicks( const firstTickValue = ticks[0]; const firstTick = { value: firstTickValue, - label: axisSpec.tickFormat(firstTickValue), + label: axisSpec.tickFormat(firstTickValue, tickFormatOptions), position: scale.scale(firstTickValue) + offset, }; const lastTickValue = firstTickValue + scale.minInterval; const lastTick = { value: lastTickValue, - label: axisSpec.tickFormat(lastTickValue), + label: axisSpec.tickFormat(lastTickValue, tickFormatOptions), position: scale.bandwidth + halfPadding * 2, }; @@ -449,7 +457,7 @@ export function getAvailableTicks( return ticks.map((tick) => { return { value: tick, - label: axisSpec.tickFormat(tick), + label: axisSpec.tickFormat(tick, tickFormatOptions), position: scale.scale(tick) + offset, }; }); @@ -605,8 +613,11 @@ export function getAxisTicksPositions( if (!scale) { throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } + const tickFormatOptions = { + timeZone: xDomain.timeZone, + }; - const allTicks = getAvailableTicks(axisSpec, scale, totalGroupsCount, enableHistogramMode); + const allTicks = getAvailableTicks(axisSpec, scale, totalGroupsCount, enableHistogramMode, tickFormatOptions); const visibleTicks = getVisibleTicks(allTicks, axisSpec, axisDim); if (axisSpec.showGridLines) { diff --git a/src/chart_types/xy_chart/utils/specs.ts b/src/chart_types/xy_chart/utils/specs.ts index 28951a51b2..fbd4269dca 100644 --- a/src/chart_types/xy_chart/utils/specs.ts +++ b/src/chart_types/xy_chart/utils/specs.ts @@ -300,7 +300,10 @@ export interface AxisSpec { integersOnly?: boolean; } -export type TickFormatter = (value: any) => string; +export type TickFormatterOptions = Partial<{ + timeZone: string; +}>; +export type TickFormatter = (value: any, options?: TickFormatterOptions) => string; export interface AxisStyle { /** Specifies the amount of padding on the tick label bounding box */ diff --git a/src/utils/data/formatters.ts b/src/utils/data/formatters.ts index 3a9578ec7b..633175e326 100644 --- a/src/utils/data/formatters.ts +++ b/src/utils/data/formatters.ts @@ -1,14 +1,14 @@ import { DateTime, Interval } from 'luxon'; +import { TickFormatter, TickFormatterOptions } from '../../chart_types/xy_chart/utils/specs'; -type TimeFormatter = (value: number) => string; - -export function timeFormatter(format: string): TimeFormatter { - return (value: number): string => { - return DateTime.fromMillis(value).toFormat(format); +export function timeFormatter(format: string): TickFormatter { + return (value: number, options?: Partial): string => { + const dateTimeOptions = options && options.timeZone ? { zone: options.timeZone } : undefined; + return DateTime.fromMillis(value, dateTimeOptions).toFormat(format); }; } -export function niceTimeFormatter(domain: [number, number]): TimeFormatter { +export function niceTimeFormatter(domain: [number, number]): TickFormatter { const minDate = DateTime.fromMillis(domain[0]); const maxDate = DateTime.fromMillis(domain[1]); const diff = Interval.fromDateTimes(minDate, maxDate); From 7cca9e814d3ebbe8362342b51936e177cc45a342 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 22 Oct 2019 17:36:42 +0200 Subject: [PATCH 2/2] fix: unnecessary partial type --- src/chart_types/xy_chart/utils/specs.ts | 6 +++--- src/utils/data/formatters.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/chart_types/xy_chart/utils/specs.ts b/src/chart_types/xy_chart/utils/specs.ts index fbd4269dca..a34837a5a3 100644 --- a/src/chart_types/xy_chart/utils/specs.ts +++ b/src/chart_types/xy_chart/utils/specs.ts @@ -300,9 +300,9 @@ export interface AxisSpec { integersOnly?: boolean; } -export type TickFormatterOptions = Partial<{ - timeZone: string; -}>; +export type TickFormatterOptions = { + timeZone?: string; +}; export type TickFormatter = (value: any, options?: TickFormatterOptions) => string; export interface AxisStyle { diff --git a/src/utils/data/formatters.ts b/src/utils/data/formatters.ts index 633175e326..296b2b5345 100644 --- a/src/utils/data/formatters.ts +++ b/src/utils/data/formatters.ts @@ -2,7 +2,7 @@ import { DateTime, Interval } from 'luxon'; import { TickFormatter, TickFormatterOptions } from '../../chart_types/xy_chart/utils/specs'; export function timeFormatter(format: string): TickFormatter { - return (value: number, options?: Partial): string => { + return (value: number, options?: TickFormatterOptions): string => { const dateTimeOptions = options && options.timeZone ? { zone: options.timeZone } : undefined; return DateTime.fromMillis(value, dateTimeOptions).toFormat(format); };