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

feat: remove duplicate tick labels from axis #577

Merged
merged 32 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
f784b2b
fix: add helper function to remove duplicate tickLabels from axis
rshen91 Mar 3, 2020
e71ac18
test: set utc time for test
rshen91 Mar 3, 2020
f4eafa5
refactor: clean up removeDupeTickLabels
rshen91 Mar 4, 2020
f7a1aa0
refactor: add filter after formatting
rshen91 Mar 4, 2020
a0c8839
feat: adding duplicate ticks story
rshen91 Mar 5, 2020
d6fb645
test: add missing snapshot
rshen91 Mar 6, 2020
f263fbf
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 9, 2020
d5abfd7
fix: add license heading to story file
rshen91 Mar 9, 2020
591c603
docs: correct x axis format for duplicates
rshen91 Mar 9, 2020
dc9a38f
test: update snapshot for x axis
rshen91 Mar 9, 2020
efce9bf
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 9, 2020
60664b9
feat: implement duplicate tick functionality
rshen91 Mar 10, 2020
af7c506
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 10, 2020
935e64a
style: clean up comments
rshen91 Mar 10, 2020
4976be7
test: add unit tests
rshen91 Mar 11, 2020
1b76c69
test: set timezone to clean up tests
rshen91 Mar 11, 2020
5037110
refactor: update naming and remove from scales spec
rshen91 Mar 11, 2020
1d888f7
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 11, 2020
ead2e9e
refactor: add timeZone prop to story
rshen91 Mar 11, 2020
c5ac664
refactor: improve filter() with a set and return allTicks earlier
rshen91 Mar 12, 2020
4e88447
refactor: refactor to reduce() implementation
rshen91 Mar 13, 2020
054c088
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 17, 2020
55bf6ee
refactor: use moment library for time calculations
rshen91 Mar 17, 2020
6ba6751
refactor: change config prop name to show vs enable
rshen91 Mar 17, 2020
9aa1329
fix: fix moment in duplicate ticks
rshen91 Mar 17, 2020
4952e6b
refactor: revert moment changes in brush stories
rshen91 Mar 17, 2020
8a9739c
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 17, 2020
594c6f6
feat: refactor getUniqueValues to be reusable
rshen91 Mar 17, 2020
a63b9eb
Merge remote-tracking branch 'upstream/master' into duplicate-ticks
rshen91 Mar 18, 2020
9ed79f0
refactor: refactored generics in getUniqueValues
rshen91 Mar 18, 2020
506bf0b
style: remove export from computeTickDimensions
rshen91 Mar 18, 2020
d0ff3c8
test: move story to axes stories and update vrt
rshen91 Mar 18, 2020
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 7 additions & 2 deletions src/chart_types/xy_chart/state/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,14 @@ export function computeSeriesGeometries(

// compute how many series are clustered
const { stackedBarsInCluster, totalBarsInCluster } = countBarsInCluster(stacked, nonStacked);

// compute scales
const xScale = computeXScale({ xDomain, totalBarsInCluster, range: [0, width], barsPadding, enableHistogramMode });
const xScale = computeXScale({
xDomain,
totalBarsInCluster,
range: [0, width],
barsPadding,
enableHistogramMode,
});
const yScales = computeYScales({ yDomains: yDomain, range: [height, 0] });

// compute colors
Expand Down
92 changes: 90 additions & 2 deletions src/chart_types/xy_chart/utils/axis_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { AxisSpec, DomainRange, AxisStyle } from './specs';
import { Position } from '../../../utils/commons';
import { LIGHT_THEME } from '../../../utils/themes/light_theme';
import { AxisId, GroupId } from '../../../utils/ids';
import { ScaleType } from '../../../scales';
import { ScaleType, Scale } from '../../../scales';
import {
AxisTick,
AxisTicksDimensions,
Expand All @@ -48,13 +48,17 @@ import {
getAxisTickLabelPadding,
isVerticalGrid,
isHorizontalGrid,
enableDuplicatedTicks,
} 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';
import { mergeYCustomDomainsByGroupId } from '../state/selectors/merge_y_custom_domains';
import { ChartTypes } from '../..';
import { SpecTypes } from '../../../specs/settings';
import { DateTime } from 'luxon';
import { computeXScale } from './scales';
import moment from 'moment-timezone';

describe('Axis computational utils', () => {
const mockedRect = {
Expand Down Expand Up @@ -202,7 +206,7 @@ describe('Axis computational utils', () => {
expect(axisDimensions).toEqual(axis1Dims);

const computeScalelessSpec = () => {
computeAxisTicksDimensions(ungroupedAxisSpec, xDomain, [yDomain], 1, bboxCalculator, 0, axes);
computeAxisTicksDimensions(ungroupedAxisSpec, xDomain, [yDomain], 1, bboxCalculator, 0, axes, undefined, false);
};

const ungroupedAxisSpec = { ...verticalAxisSpec, groupId: 'foo' };
Expand Down Expand Up @@ -1436,4 +1440,88 @@ describe('Axis computational utils', () => {

expect(getAxisTickLabelPadding(axisConfigTickLabelPadding, axisSpecStyle)).toEqual(2);
});
test('should show unique tick labels if duplicateTicks is set to false', () => {
const now = DateTime.fromISO('2019-01-11T00:00:00.000')
.setZone('utc+1')
.toMillis();
const oneDay = moment.duration(1, 'day');
const formatter = niceTimeFormatter([now, oneDay.add(now).asMilliseconds() * 31]);
const axisSpec: AxisSpec = {
id: 'bottom',
position: 'bottom',
showDuplicatedTicks: false,
chartType: 'xy_axis',
specType: 'axis',
groupId: '__global__',
hide: false,
showOverlappingLabels: false,
showOverlappingTicks: false,
tickSize: 10,
tickPadding: 10,
tickLabelRotation: 0,
tickFormat: formatter,
};
const xDomainTime: XDomain = {
type: 'xDomain',
isBandScale: false,
domain: [1547190000000, 1547622000000],
minInterval: 86400000,
scaleType: ScaleType.Time,
};
const scale: Scale = computeXScale({ xDomain: xDomainTime, totalBarsInCluster: 0, range: [0, 603.5] });
const offset = 0;
const tickFormatOption = { timeZone: 'utc+1' };
expect(enableDuplicatedTicks(axisSpec, scale, offset, tickFormatOption)).toEqual([
{ value: 1547208000000, label: '2019-01-11', position: 25.145833333333332 },
{ value: 1547251200000, label: '2019-01-12', position: 85.49583333333334 },
{ value: 1547337600000, label: '2019-01-13', position: 206.19583333333333 },
{ value: 1547424000000, label: '2019-01-14', position: 326.8958333333333 },
{ value: 1547510400000, label: '2019-01-15', position: 447.59583333333336 },
{ value: 1547596800000, label: '2019-01-16', position: 568.2958333333333 },
]);
});
test('should show duplicate tick labels if duplicateTicks is set to true', () => {
const now = DateTime.fromISO('2019-01-11T00:00:00.000')
.setZone('utc+1')
.toMillis();
const oneDay = moment.duration(1, 'day');
const formatter = niceTimeFormatter([now, oneDay.add(now).asMilliseconds() * 31]);
const axisSpec: AxisSpec = {
id: 'bottom',
position: 'bottom',
showDuplicatedTicks: true,
chartType: 'xy_axis',
specType: 'axis',
groupId: '__global__',
hide: false,
showOverlappingLabels: false,
showOverlappingTicks: false,
tickSize: 10,
tickPadding: 10,
tickLabelRotation: 0,
tickFormat: formatter,
};
const xDomainTime: XDomain = {
type: 'xDomain',
isBandScale: false,
domain: [1547190000000, 1547622000000],
minInterval: 86400000,
scaleType: ScaleType.Time,
};
const scale: Scale = computeXScale({ xDomain: xDomainTime, totalBarsInCluster: 0, range: [0, 603.5] });
const offset = 0;
const tickFormatOption = { timeZone: 'utc+1' };
expect(enableDuplicatedTicks(axisSpec, scale, offset, tickFormatOption)).toEqual([
{ value: 1547208000000, label: '2019-01-11', position: 25.145833333333332 },
{ value: 1547251200000, label: '2019-01-12', position: 85.49583333333334 },
{ value: 1547294400000, label: '2019-01-12', position: 145.84583333333333 },
{ value: 1547337600000, label: '2019-01-13', position: 206.19583333333333 },
{ value: 1547380800000, label: '2019-01-13', position: 266.54583333333335 },
{ value: 1547424000000, label: '2019-01-14', position: 326.8958333333333 },
{ value: 1547467200000, label: '2019-01-14', position: 387.24583333333334 },
{ value: 1547510400000, label: '2019-01-15', position: 447.59583333333336 },
{ value: 1547553600000, label: '2019-01-15', position: 507.9458333333333 },
{ value: 1547596800000, label: '2019-01-16', position: 568.2958333333333 },
]);
});
});
25 changes: 19 additions & 6 deletions src/chart_types/xy_chart/utils/axis_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
AxisStyle,
TickFormatterOptions,
} from './specs';
import { Position, Rotation } from '../../../utils/commons';
import { Position, Rotation, getUniqueValues } from '../../../utils/commons';
import { AxisConfig, Theme } from '../../../utils/themes/theme';
import { Dimensions, Margins } from '../../../utils/dimensions';
import { AxisId } from '../../../utils/ids';
Expand Down Expand Up @@ -87,7 +87,6 @@ export function computeAxisTicksDimensions(
if (axisSpec.hide) {
return null;
}

const scale = getScaleForAxisSpec(
axisSpec,
xDomain,
Expand Down Expand Up @@ -238,11 +237,9 @@ function computeTickDimensions(
const tickLabels = tickValues.map((d) => {
return tickFormat(d, tickFormatOptions);
});

const {
tickLabelStyle: { fontFamily, fontSize },
} = axisConfig;

const {
maxLabelBboxWidth,
maxLabelBboxHeight,
Expand All @@ -252,7 +249,6 @@ function computeTickDimensions(
getMaxBboxDimensions(bboxCalculator, fontSize, fontFamily, tickLabelRotation, tickLabelPadding),
{ maxLabelBboxWidth: 0, maxLabelBboxHeight: 0, maxLabelTextWidth: 0, maxLabelTextHeight: 0 },
);

return {
tickValues,
tickLabels,
Expand Down Expand Up @@ -444,14 +440,31 @@ export function getAvailableTicks(

return [firstTick, lastTick];
}
return ticks.map((tick) => {
return enableDuplicatedTicks(axisSpec, scale, offset, tickFormatOptions);
}

/** @internal */
export function enableDuplicatedTicks(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: enableX to me sounds like you are toggling a config option or something of that nature. Also, your config property and this method have the same name, these should usually be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out - changed the config property naming in commit
6ba6751

axisSpec: AxisSpec,
scale: Scale,
offset: number,
tickFormatOptions?: TickFormatterOptions,
) {
const ticks = scale.ticks();
const allTicks: AxisTick[] = ticks.map((tick) => {
return {
value: tick,
label: axisSpec.tickFormat(tick, tickFormatOptions),
position: scale.scale(tick) + offset,
};
});

if (axisSpec.showDuplicatedTicks === true) {
return allTicks;
}
return getUniqueValues(allTicks, 'label');
}

export function getVisibleTicks(allTicks: AxisTick[], axisSpec: AxisSpec, axisDim: AxisTicksDimensions): AxisTick[] {
// We sort the ticks by position so that we can incrementally compute previousOccupiedSpace
allTicks.sort((a: AxisTick, b: AxisTick) => a.position - b.position);
Expand Down
3 changes: 3 additions & 0 deletions src/chart_types/xy_chart/utils/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,11 +525,14 @@ export interface AxisSpec extends Spec {
style?: AxisStyle;
/** Show only integar values **/
integersOnly?: boolean;
/** Remove duplicate ticks, default is false*/
showDuplicatedTicks?: boolean;
}

export type TickFormatterOptions = {
timeZone?: string;
};

export type TickFormatter = (value: any, options?: TickFormatterOptions) => string;

export interface AxisStyle {
Expand Down
22 changes: 22 additions & 0 deletions src/utils/commons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,28 @@ export function isNumberArray(value: unknown): value is number[] {
return Array.isArray(value) && value.every((element) => typeof element === 'number');
}

/** @internal */
export function getUniqueValues<T>(fullArray: T[], uniqueProperty: keyof T): T[] {
return fullArray.reduce<{
filtered: T[];
uniqueValues: Set<T[keyof T]>;
}>(
(acc, currentValue) => {
const uniqueValue = currentValue[uniqueProperty];
if (acc.uniqueValues.has(uniqueValue)) {
return acc;
}
acc.uniqueValues.add(uniqueValue);
acc.filtered.push(currentValue);
return acc;
},
{
filtered: [],
uniqueValues: new Set(),
},
).filtered;
}

export type ValueFormatter = (value: number) => string;
export type ValueAccessor = (d: Datum) => number;
export type LabelAccessor = (value: PrimitiveValue) => string;
Expand Down
65 changes: 65 additions & 0 deletions stories/axes/12_duplicate_ticks.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License. */

import React from 'react';
import { Axis, Chart, LineSeries, Position, ScaleType, niceTimeFormatter } from '../../src';
import { KIBANA_METRICS } from '../../src/utils/data_samples/test_dataset_kibana';
import { boolean } from '@storybook/addon-knobs';
import { DateTime } from 'luxon';
import moment from 'moment-timezone';

export const example = () => {
const now = DateTime.fromISO('2019-01-11T00:00:00.000')
.setZone('utc+1')
.toMillis();
const oneDay = moment.duration(1, 'd');
const twoDays = moment.duration(2, 'd');
const oneMonth = moment.duration(31, 'd');
const threeDays = moment.duration(3, 'd');
const fourDays = moment.duration(4, 'd');
const fiveDays = moment.duration(5, 'd');
const formatter = niceTimeFormatter([now, oneMonth.add(now).asMilliseconds()]);
const duplicateTicksInAxis = boolean('Show duplicate ticks in x axis', false);
return (
<Chart className="story-chart">
<Axis id="bottom" position={Position.Bottom} tickFormat={formatter} showDuplicatedTicks={duplicateTicksInAxis} />
<Axis
id="left"
title={KIBANA_METRICS.metrics.kibana_os_load[0].metric.title}
position={Position.Left}
tickFormat={(d) => `${Number(d).toFixed(1)}`}
/>
<LineSeries
id="lines"
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
data={[
{ x: now, y: 2 },
{ x: oneDay.add(now).asMilliseconds(), y: 3 },
{ x: twoDays.add(now).asMilliseconds(), y: 3 },
{ x: threeDays.add(now).asMilliseconds(), y: 4 },
{ x: fourDays.add(now).asMilliseconds(), y: 8 },
{ x: fiveDays.add(now).asMilliseconds(), y: 6 },
]}
timeZone="local"
/>
</Chart>
);
};
1 change: 1 addition & 0 deletions stories/axes/axes.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ export { example as customDomain } from './8_custom_domain';
export { example as customMixed } from './9_custom_mixed_domain';
export { example as oneDomainBound } from './10_one_domain_bound';
export { example as fitDomain } from './11_fit_domain_extent';
export { example as duplicateTicks } from './12_duplicate_ticks';
10 changes: 7 additions & 3 deletions stories/interactions/11_brush_time.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ import { Axis, BarSeries, Chart, LineSeries, niceTimeFormatter, Position, ScaleT
import { boolean } from '@storybook/addon-knobs';
import { DateTime } from 'luxon';
import { getChartRotationKnob } from '../utils/knobs';
import moment from 'moment-timezone';

export const example = () => {
const now = DateTime.fromISO('2019-01-11T00:00:00.000')
.setZone('utc+1')
.toMillis();
const oneDay = 1000 * 60 * 60 * 24;
const formatter = niceTimeFormatter([now, now + oneDay * 5]);
const oneDays = moment.duration(1, 'd');
const twoDays = moment.duration(2, 'd');
const fiveDays = moment.duration(5, 'd');
const formatter = niceTimeFormatter([now, fiveDays.add(now).asMilliseconds()]);
return (
<Chart className="story-chart">
<Settings
Expand All @@ -52,8 +56,8 @@ export const example = () => {
timeZone="Europe/Rome"
data={[
{ x: now, y: 2 },
{ x: now + oneDay, y: 7 },
{ x: now + oneDay * 2, y: 3 },
{ x: oneDays.add(now).asMilliseconds(), y: 7 },
{ x: twoDays.add(now).asMilliseconds(), y: 3 },
{ x: now + oneDay * 5, y: 6 },
]}
/>
Expand Down