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 19 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
91 changes: 89 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,16 @@ 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';

describe('Axis computational utils', () => {
const mockedRect = {
Expand Down Expand Up @@ -202,7 +205,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 +1439,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 = 1000 * 60 * 60 * 24;
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should leverage packages to your advantage. Like using duration from moment.

Suggested change
const oneDay = 1000 * 60 * 60 * 24;
const oneDay = moment.duration(1, 'day');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! Pls see commit 55bf6ee for changes 💪

const formatter = niceTimeFormatter([now, now + oneDay * 31]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better yet you could use moment's super easy and declarative API for this using .add and .subtract depending on what type of date niceTimeFormatter is expecting. If you need the value in mills you could use the duration.add.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes in commit 55bf6ee thanks!

const axisSpec: AxisSpec = {
id: 'bottom',
position: 'bottom',
enableDuplicatedTicks: 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 = 1000 * 60 * 60 * 24;
const formatter = niceTimeFormatter([now, now + oneDay * 31]);
const axisSpec: AxisSpec = {
id: 'bottom',
position: 'bottom',
enableDuplicatedTicks: 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 },
]);
});
});
36 changes: 30 additions & 6 deletions src/chart_types/xy_chart/utils/axis_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ export function computeAxisTicksDimensions(
if (axisSpec.hide) {
return null;
}

const scale = getScaleForAxisSpec(
axisSpec,
xDomain,
Expand Down Expand Up @@ -225,7 +224,7 @@ export const getMaxBboxDimensions = (
};
};

function computeTickDimensions(
export function computeTickDimensions(
scale: Scale,
tickFormat: TickFormatter,
bboxCalculator: BBoxCalculator,
Expand All @@ -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,42 @@ export function getAvailableTicks(

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

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 labels = [...new Set(ticks.map((tick) => axisSpec.tickFormat(tick, tickFormatOptions)))];
const allTicks = ticks.map((tick) => {
return {
value: tick,
label: axisSpec.tickFormat(tick, tickFormatOptions),
position: scale.scale(tick) + offset,
};
});

if (axisSpec.enableDuplicatedTicks === false) {
const uniqueTickLabels: { value: any; label: string; position: number }[] = [];
allTicks.filter((value, index) => {
for (let i = 0; i < labels.length; i++) {
if (labels[i] === allTicks[index].label) {
uniqueTickLabels.push(allTicks[index]);
// once uniqueTickLabels has the unique label, remove the label from the labels array so it doesnt create a duplicate
labels.splice(i, 1);
}
}
});
return uniqueTickLabels;
} else {
return allTicks;
}
}

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*/
enableDuplicatedTicks?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These properties are more like the current state of something. Say showGridLines as in a boolean, it wouldn't be enableGirdLines because it is not performing an action like that would imply.

}

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

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

export interface AxisStyle {
Expand Down
64 changes: 64 additions & 0 deletions stories/line/10_duplicate_ticks.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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';

export const example = () => {
const now = DateTime.fromISO('2019-01-11T00:00:00.000')
.setZone('utc+1')
Copy link
Member

Choose a reason for hiding this comment

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

if you are using a timezone here, you should also add it to the LineSeries if not, the ticks will be slightly off
Screenshot 2020-03-10 at 19 13 21

<LineSeries
        ...
        timeZone="utc+1"

Copy link
Contributor Author

@rshen91 rshen91 Mar 11, 2020

Choose a reason for hiding this comment

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

Weirdly, utc+1 wasn't aligning but timeZone="local" seems to do the trick. Not really sure how that is possible. Commit ead2e9e for reference

.toMillis();
const oneDay = 1000 * 60 * 60 * 24;
const formatter = niceTimeFormatter([now, now + oneDay * 31]);
const duplicateTicksInAxis = boolean('Show duplicate ticks in x axis', false);
return (
<Chart className="story-chart">
<Axis
id="bottom"
position={Position.Bottom}
tickFormat={formatter}
enableDuplicatedTicks={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: now + oneDay, y: 3 },
{ x: now + oneDay * 2, y: 3 },
{ x: now + oneDay * 3, y: 4 },
{ x: now + oneDay * 4, y: 8 },
{ x: now + oneDay * 5, y: 6 },
]}
timeZone="local"
/>
</Chart>
);
};
1 change: 1 addition & 0 deletions stories/line/line.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ export { example as curvedWithAxisAndLegend } from './6_curved';
export { example as multipleWithAxisAndLegend } from './7_multiple';
export { example as stackedWithAxisAndLegend } from './8_stacked';
export { example as multiSeriesWithLogValues } from './9_multi_series';
export { example as duplicateTicksDates } from './10_duplicate_ticks';