Skip to content

Commit

Permalink
feat(tooltip): series tick formatters (opensearch-project#802)
Browse files Browse the repository at this point in the history
* feat(tooltip): series tick formatters

* test: add tests for new custom formatting

* test: remove only
  • Loading branch information
nickofthyme authored Sep 4, 2020
1 parent de3cb63 commit 813b915
Show file tree
Hide file tree
Showing 21 changed files with 471 additions and 61 deletions.
3 changes: 2 additions & 1 deletion packages/osd-charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export interface AxisSpec extends Spec {
// (undocumented)
specType: typeof SpecTypes.Axis;
style?: RecursivePartial<Omit<AxisStyle, 'gridLine'>>;
tickFormat: TickFormatter;
tickFormat?: TickFormatter;
ticks?: number;
title?: string;
}
Expand Down Expand Up @@ -1262,6 +1262,7 @@ export interface SeriesSpec extends Spec {
sortIndex?: number;
// (undocumented)
specType: typeof SpecTypes.Series;
tickFormat?: TickFormatter;
useDefaultGroupDomain?: boolean;
// Warning: (ae-forgotten-export) The symbol "AccessorFormat" needs to be exported by the entry point index.d.ts
y0AccessorFormat?: AccessorFormat;
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
30 changes: 30 additions & 0 deletions packages/osd-charts/integration/tests/interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,34 @@ describe('Interactions', () => {
);
});
});

describe('Tooltip formatting', () => {
it('should use all custom tick formatters', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=&knob-Cat%20line%20unit=cats',
{ left: 280, top: 80 },
);
});

it('should use series tick formatter with no axis tick formatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=&knob-Cat%20line%20unit=cats',
{ left: 280, top: 80 },
);
});

it('should use series tick formatter with no axis tick formatter, missing series tick formatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=true&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=true&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=&knob-Cat%20line%20unit=cats',
{ left: 280, top: 80 },
);
});

it('should use default tick formatter with no axis tick formatter, nor series tick formatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=true&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=true&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=true&knob-Cat%20line%20unit=cats',
{ left: 280, top: 80 },
);
});
});
});
6 changes: 4 additions & 2 deletions packages/osd-charts/src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@

import { LegendItem } from '../../../commons/legend';
import { SeriesKey, SeriesIdentifier } from '../../../commons/series_id';
import { identity, Color } from '../../../utils/commons';
import { Color } from '../../../utils/commons';
import { BandedAccessorType } from '../../../utils/geometry';
import { getAxesSpecForSpecId, getSpecsById } from '../state/utils/spec';
import { Y0_ACCESSOR_POSTFIX, Y1_ACCESSOR_POSTFIX } from '../tooltip/tooltip';
import { defaultTickFormatter } from '../utils/axis_utils';
import {
SeriesCollectionValue,
getSeriesIndex,
Expand Down Expand Up @@ -66,6 +67,7 @@ export function computeLegend(
): LegendItem[] {
const legendItems: LegendItem[] = [];
const sortedCollection = getSortedDataSeriesColorsValuesMap(seriesCollection);
const fallbackTickFormatter = specs.find(({ tickFormat }) => tickFormat)?.tickFormat ?? defaultTickFormatter;

sortedCollection.forEach((series, key) => {
const { banded, lastValue, seriesIdentifier } = series;
Expand All @@ -83,7 +85,7 @@ export function computeLegend(

// Use this to get axis spec w/ tick formatter
const { yAxis } = getAxesSpecForSpecId(axesSpecs, spec.groupId);
const formatter = yAxis ? yAxis.tickFormat : identity;
const formatter = yAxis?.tickFormat ?? fallbackTickFormatter;
const { hideInLegend } = spec;

legendItems.push({
Expand Down
8 changes: 3 additions & 5 deletions packages/osd-charts/src/chart_types/xy_chart/specs/axis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,13 @@ const defaultProps = {
showOverlappingTicks: false,
showOverlappingLabels: false,
position: Position.Left,
tickFormat: (tick: any) => `${tick}`,
};

type SpecRequired = Pick<AxisSpec, 'id'>;
type SpecOptionals = Partial<Omit<AxisSpec, 'chartType' | 'specType' | 'seriesType' | 'id'>>;

export const Axis: React.FunctionComponent<SpecRequired & SpecOptionals> = getConnect()(
specComponentFactory<
AxisSpec,
'groupId' | 'hide' | 'showOverlappingTicks' | 'showOverlappingLabels' | 'position' | 'tickFormat'
>(defaultProps),
specComponentFactory<AxisSpec, 'groupId' | 'hide' | 'showOverlappingTicks' | 'showOverlappingLabels' | 'position'>(
defaultProps,
),
);
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,18 @@ describe('Render chart', () => {
store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip
let tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(day1);
expect(tooltip.values[0].value).toBe(10);
expect(tooltip.header?.value).toBe(`${day1}`);
expect(tooltip.values[0].value).toBe(`${10}`);
store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(day2);
expect(tooltip.values[0].value).toBe(22);
expect(tooltip.header?.value).toBe(`${day2}`);
expect(tooltip.values[0].value).toBe(`${22}`);
store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(day3);
expect(tooltip.values[0].value).toBe(6);
expect(tooltip.header?.value).toBe(`${day3}`);
expect(tooltip.values[0].value).toBe(`${6}`);
});
});
describe('line, utc-time, 5m interval', () => {
Expand Down Expand Up @@ -159,18 +159,18 @@ describe('Render chart', () => {
store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip
let tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(date1);
expect(tooltip.values[0].value).toBe(10);
expect(tooltip.header?.value).toBe(`${date1}`);
expect(tooltip.values[0].value).toBe(`${10}`);
store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(date2);
expect(tooltip.values[0].value).toBe(22);
expect(tooltip.header?.value).toBe(`${date2}`);
expect(tooltip.values[0].value).toBe(`${22}`);
store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(date3);
expect(tooltip.values[0].value).toBe(6);
expect(tooltip.header?.value).toBe(`${date3}`);
expect(tooltip.values[0].value).toBe(`${6}`);
});
});
describe('line, non utc-time, 5m + 1s interval', () => {
Expand Down Expand Up @@ -245,18 +245,18 @@ describe('Render chart', () => {
store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip
let tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(date1);
expect(tooltip.values[0].value).toBe(10);
expect(tooltip.header?.value).toBe(`${date1}`);
expect(tooltip.values[0].value).toBe(`${10}`);
store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(date2);
expect(tooltip.values[0].value).toBe(22);
expect(tooltip.header?.value).toBe(`${date2}`);
expect(tooltip.values[0].value).toBe(`${22}`);
store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(date3);
expect(tooltip.values[0].value).toBe(6);
expect(tooltip.header?.value).toBe(`${date3}`);
expect(tooltip.values[0].value).toBe(`${6}`);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@ import { getChartThemeSelector } from '../../../../state/selectors/get_chart_the
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { CanvasTextBBoxCalculator } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { AxisId } from '../../../../utils/ids';
import { computeAxisTicksDimensions, AxisTicksDimensions, isDuplicateAxis } from '../../utils/axis_utils';
import {
computeAxisTicksDimensions,
AxisTicksDimensions,
isDuplicateAxis,
defaultTickFormatter,
} from '../../utils/axis_utils';
import { computeSeriesDomainsSelector } from './compute_series_domains';
import { countBarsInClusterSelector } from './count_bars_in_cluster';
import { getBarPaddingsSelector } from './get_bar_paddings';
import { getAxisSpecsSelector } from './get_specs';
import { getAxisSpecsSelector, getSeriesSpecsSelector } from './get_specs';
import { isHistogramModeEnabledSelector } from './is_histogram_mode_enabled';

/** @internal */
Expand All @@ -41,6 +46,7 @@ export const computeAxisTicksDimensionsSelector = createCachedSelector(
getSettingsSpecSelector,
computeSeriesDomainsSelector,
countBarsInClusterSelector,
getSeriesSpecsSelector,
],
(
barsPadding,
Expand All @@ -50,9 +56,10 @@ export const computeAxisTicksDimensionsSelector = createCachedSelector(
settingsSpec,
seriesDomainsAndData,
totalBarsInCluster,
seriesSpecs,
): Map<AxisId, AxisTicksDimensions> => {
const { xDomain, yDomain } = seriesDomainsAndData;

const fallBackTickFormatter = seriesSpecs.find(({ tickFormat }) => tickFormat)?.tickFormat ?? defaultTickFormatter;
const bboxCalculator = new CanvasTextBBoxCalculator();
const axesTicksDimensions: Map<AxisId, AxisTicksDimensions> = new Map();
axesSpecs.forEach((axisSpec) => {
Expand All @@ -65,6 +72,7 @@ export const computeAxisTicksDimensionsSelector = createCachedSelector(
bboxCalculator,
settingsSpec.rotation,
chartTheme.axes,
fallBackTickFormatter,
barsPadding,
isHistogramMode,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ import { getChartThemeSelector } from '../../../../state/selectors/get_chart_the
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { Dimensions } from '../../../../utils/dimensions';
import { AxisId } from '../../../../utils/ids';
import { getAxisTicksPositions, AxisTick, AxisLinePosition } from '../../utils/axis_utils';
import { getAxisTicksPositions, AxisTick, AxisLinePosition, defaultTickFormatter } from '../../utils/axis_utils';
import { computeAxisTicksDimensionsSelector } from './compute_axis_ticks_dimensions';
import { computeChartDimensionsSelector } from './compute_chart_dimensions';
import { computeSeriesDomainsSelector } from './compute_series_domains';
import { countBarsInClusterSelector } from './count_bars_in_cluster';
import { getAxesStylesSelector } from './get_axis_styles';
import { getBarPaddingsSelector } from './get_bar_paddings';
import { getAxisSpecsSelector } from './get_specs';
import { getAxisSpecsSelector, getSeriesSpecsSelector } from './get_specs';
import { isHistogramModeEnabledSelector } from './is_histogram_mode_enabled';

/** @internal */
Expand All @@ -54,6 +54,7 @@ export const computeAxisVisibleTicksSelector = createCachedSelector(
countBarsInClusterSelector,
isHistogramModeEnabledSelector,
getBarPaddingsSelector,
getSeriesSpecsSelector,
],
(
chartDimensions,
Expand All @@ -66,7 +67,9 @@ export const computeAxisVisibleTicksSelector = createCachedSelector(
totalBarsInCluster,
isHistogramMode,
barsPadding,
seriesSpecs,
): AxisVisibleTicks => {
const fallBackTickFormatter = seriesSpecs.find(({ tickFormat }) => tickFormat)?.tickFormat ?? defaultTickFormatter;
const { xDomain, yDomain } = seriesDomainsAndData;
return getAxisTicksPositions(
chartDimensions,
Expand All @@ -79,6 +82,7 @@ export const computeAxisVisibleTicksSelector = createCachedSelector(
yDomain,
totalBarsInCluster,
isHistogramMode,
fallBackTickFormatter,
barsPadding,
);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { SeriesKey, SeriesIdentifier } from '../../../../commons/series_id';
import { Scale } from '../../../../scales';
import { ScaleType } from '../../../../scales/constants';
import { identity, mergePartial, Rotation, Color, isUniqueArray } from '../../../../utils/commons';
import { mergePartial, Rotation, Color, isUniqueArray } from '../../../../utils/commons';
import { CurveType } from '../../../../utils/curves';
import { Dimensions } from '../../../../utils/dimensions';
import { Domain } from '../../../../utils/domain';
Expand All @@ -31,6 +31,7 @@ import { XDomain, YDomain } from '../../domains/types';
import { mergeXDomain } from '../../domains/x_domain';
import { mergeYDomain } from '../../domains/y_domain';
import { renderArea, renderBars, renderLine, renderBubble, isDatumFilled } from '../../rendering/rendering';
import { defaultTickFormatter } from '../../utils/axis_utils';
import { fillSeries } from '../../utils/fill_series';
import { IndexedGeometryMap } from '../../utils/indexed_geometry_map';
import { computeXScale, computeYScales, countBarsInCluster } from '../../utils/scales';
Expand Down Expand Up @@ -483,6 +484,7 @@ function renderGeometries(
const bubbles: BubbleGeometry[] = [];
const indexedGeometryMap = new IndexedGeometryMap();
const isMixedChart = isUniqueArray(seriesSpecs, ({ seriesType }) => seriesType) && seriesSpecs.length > 1;
const fallBackTickFormatter = seriesSpecs.find(({ tickFormat }) => tickFormat)?.tickFormat ?? defaultTickFormatter;
const geometriesCounts: GeometriesCounts = {
points: 0,
bars: 0,
Expand Down Expand Up @@ -510,7 +512,7 @@ function renderGeometries(
});

const { yAxis } = getAxesSpecForSpecId(axesSpecs, spec.groupId);
const valueFormatter = yAxis && yAxis.tickFormat ? yAxis.tickFormat : identity;
const valueFormatter = yAxis?.tickFormat ?? fallBackTickFormatter;

const displayValueSettings = spec.displayValueSettings
? { valueFormatter, ...spec.displayValueSettings }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { SpecTypes } from '../../../specs/constants';
import { Position, RecursivePartial } from '../../../utils/commons';
import { BarGeometry } from '../../../utils/geometry';
import { AxisStyle } from '../../../utils/themes/theme';
import { AxisSpec, BarSeriesSpec, SeriesTypes } from '../utils/specs';
import { AxisSpec, BarSeriesSpec, SeriesTypes, TickFormatter } from '../utils/specs';
import { formatTooltip } from './tooltip';

const style: RecursivePartial<AxisStyle> = {
Expand Down Expand Up @@ -274,4 +274,59 @@ describe('Tooltip formatting', () => {
tooltipValue = formatTooltip(geometry, SPEC_1, true, true, false, YAXIS_SPEC);
expect(tooltipValue.isHighlighted).toBe(false);
});

it('should format ticks with custom formatter from spec', () => {
const axisTickFormatter: TickFormatter = (v) => `${v} axis`;
const tickFormatter: TickFormatter = (v) => `${v} spec`;
const axisSpec: AxisSpec = {
...YAXIS_SPEC,
tickFormat: axisTickFormatter,
};
const spec: BarSeriesSpec = {
...SPEC_1,
tickFormat: tickFormatter,
};
const tooltipValue = formatTooltip(indexedGeometry, spec, false, false, false, axisSpec);
expect(tooltipValue.value).toBe('10 spec');
});

it('should format ticks with custom formatter from axis', () => {
const axisTickFormatter: TickFormatter = (v) => `${v} axis`;
const axisSpec: AxisSpec = {
...YAXIS_SPEC,
tickFormat: axisTickFormatter,
};
const tooltipValue = formatTooltip(indexedGeometry, SPEC_1, false, false, false, axisSpec);
expect(tooltipValue.value).toBe('10 axis');
});

it('should format ticks with default formatter', () => {
const tooltipValue = formatTooltip(indexedGeometry, SPEC_1, false, false, false, YAXIS_SPEC);
expect(tooltipValue.value).toBe('10');
});

it('should format header with custom formatter from axis', () => {
const axisTickFormatter: TickFormatter = (v) => `${v} axis`;
const tickFormatter: TickFormatter = (v) => `${v} spec`;
const axisSpec: AxisSpec = {
...YAXIS_SPEC,
tickFormat: axisTickFormatter,
};
const spec: BarSeriesSpec = {
...SPEC_1,
tickFormat: tickFormatter,
};
const tooltipValue = formatTooltip(indexedGeometry, spec, true, false, false, axisSpec);
expect(tooltipValue.value).toBe('1 axis');
});

it('should format header with default formatter from axis', () => {
const tickFormatter: TickFormatter = (v) => `${v} spec`;
const spec: BarSeriesSpec = {
...SPEC_1,
tickFormat: tickFormatter,
};
const tooltipValue = formatTooltip(indexedGeometry, spec, true, false, false, YAXIS_SPEC);
expect(tooltipValue.value).toBe('1');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import { LegendItemExtraValues } from '../../../commons/legend';
import { SeriesKey } from '../../../commons/series_id';
import { TooltipValue } from '../../../specs';
import { getAccessorFormatLabel } from '../../../utils/accessor';
import { identity } from '../../../utils/commons';
import { IndexedGeometry, BandedAccessorType } from '../../../utils/geometry';
import { defaultTickFormatter } from '../utils/axis_utils';
import { getSeriesName } from '../utils/series';
import {
AxisSpec,
Expand Down Expand Up @@ -84,12 +84,14 @@ export function formatTooltip(

const value = isHeader ? x : y;
const tickFormatOptions: TickFormatterOptions | undefined = spec.timeZone ? { timeZone: spec.timeZone } : undefined;
const tickFormatter =
(isHeader ? axisSpec?.tickFormat : spec.tickFormat ?? axisSpec?.tickFormat) ?? defaultTickFormatter;

return {
seriesIdentifier,
valueAccessor: accessor,
label,
value: axisSpec ? axisSpec.tickFormat(value, tickFormatOptions) : identity(value),
value: tickFormatter(value, tickFormatOptions),
markValue: isHeader || mark === null ? null : mark,
color,
isHighlighted: isHeader ? false : isHighlighted,
Expand Down
Loading

0 comments on commit 813b915

Please sign in to comment.