Skip to content

Commit

Permalink
[XY] Add addTimeMarker arg (#131495)
Browse files Browse the repository at this point in the history
* Add `addTimeMarker` arg

* Some fixes

* Update validation

* Fix snapshots

* Some fixes after merge

* Add unit tests

* Fix CI

* Update src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx

Co-authored-by: Yaroslav Kuznietsov <[email protected]>

* Fixed tests

* Fix checks

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Yaroslav Kuznietsov <[email protected]>
  • Loading branch information
3 people authored May 20, 2022
1 parent de90ea5 commit f75b6fa
Show file tree
Hide file tree
Showing 18 changed files with 580 additions and 456 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const createSampleDatatableWithRows = (rows: DatatableRow[]): Datatable =
id: 'c',
name: 'c',
meta: {
type: 'string',
type: 'date',
field: 'order_date',
sourceParams: { type: 'date-histogram', params: { interval: 'auto' } },
params: { id: 'string' },
Expand Down Expand Up @@ -128,8 +128,8 @@ export const createArgsWithLayers = (

export function sampleArgs() {
const data = createSampleDatatableWithRows([
{ a: 1, b: 2, c: 'I', d: 'Foo' },
{ a: 1, b: 5, c: 'J', d: 'Bar' },
{ a: 1, b: 2, c: 1652034840000, d: 'Foo' },
{ a: 1, b: 5, c: 1652122440000, d: 'Bar' },
]);

return {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const commonDataLayerArgs: Omit<
xScaleType: {
options: [...Object.values(XScaleTypes)],
help: strings.getXScaleTypeHelp(),
default: XScaleTypes.ORDINAL,
strict: true,
},
isHistogram: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ export const commonXYArgs: CommonXYFn['args'] = {
types: ['string'],
help: strings.getAriaLabelHelp(),
},
addTimeMarker: {
types: ['boolean'],
default: false,
help: strings.getAddTimeMakerHelp(),
},
markSizeRatio: {
types: ['number'],
help: strings.getMarkSizeRatioHelp(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@
*/

import { XY_VIS_RENDERER } from '../constants';
import { appendLayerIds, getDataLayers } from '../helpers';
import { LayeredXyVisFn } from '../types';
import { logDatatables } from '../utils';
import {
validateMarkSizeRatioLimits,
validateAddTimeMarker,
validateMinTimeBarInterval,
hasBarLayer,
errors,
} from './validate';
import { appendLayerIds, getDataLayers } from '../helpers';

export const layeredXyVisFn: LayeredXyVisFn['fn'] = async (data, args, handlers) => {
const layers = appendLayerIds(args.layers ?? [], 'layers');
Expand All @@ -24,6 +25,7 @@ export const layeredXyVisFn: LayeredXyVisFn['fn'] = async (data, args, handlers)

const dataLayers = getDataLayers(layers);
const hasBar = hasBarLayer(dataLayers);
validateAddTimeMarker(dataLayers, args.addTimeMarker);
validateMarkSizeRatioLimits(args.markSizeRatio);
validateMinTimeBarInterval(dataLayers, hasBar, args.minTimeBarInterval);
const hasMarkSizeAccessors =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
CommonXYDataLayerConfigResult,
ValueLabelMode,
CommonXYDataLayerConfig,
ExtendedDataLayerConfigResult,
} from '../types';
import { isTimeChart } from '../helpers';

Expand Down Expand Up @@ -58,6 +59,10 @@ export const errors = {
i18n.translate('expressionXY.reusable.function.xyVis.errors.dataBoundsForNotLineChartError', {
defaultMessage: 'Only line charts can be fit to the data bounds',
}),
timeMarkerForNotTimeChartsError: () =>
i18n.translate('expressionXY.reusable.function.xyVis.errors.timeMarkerForNotTimeChartsError', {
defaultMessage: 'Only time charts can have current time marker',
}),
isInvalidIntervalError: () =>
i18n.translate('expressionXY.reusable.function.xyVis.errors.isInvalidIntervalError', {
defaultMessage:
Expand Down Expand Up @@ -135,6 +140,15 @@ export const validateValueLabels = (
}
};

export const validateAddTimeMarker = (
dataLayers: Array<DataLayerConfigResult | ExtendedDataLayerConfigResult>,
addTimeMarker?: boolean
) => {
if (addTimeMarker && !isTimeChart(dataLayers)) {
throw new Error(errors.timeMarkerForNotTimeChartsError());
}
};

export const validateMarkSizeForChartType = (
markSizeAccessor: ExpressionValueVisDimension | string | undefined,
seriesType: SeriesType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,17 @@
*/

import { xyVisFunction } from '.';
import { Datatable } from '@kbn/expressions-plugin/common';
import { createMockExecutionContext } from '@kbn/expressions-plugin/common/mocks';
import { sampleArgs, sampleLayer } from '../__mocks__';
import { XY_VIS } from '../constants';

describe('xyVis', () => {
test('it renders with the specified data and args', async () => {
const { data, args } = sampleArgs();
const newData = {
...data,
type: 'datatable',

columns: data.columns.map((c) =>
c.id !== 'c'
? c
: {
...c,
meta: {
type: 'string',
},
}
),
} as Datatable;

const { layers, ...rest } = args;
const { layerId, layerType, table, type, ...restLayerArgs } = sampleLayer;
const result = await xyVisFunction.fn(
newData,
data,
{ ...rest, ...restLayerArgs, referenceLines: [], annotationLayers: [] },
createMockExecutionContext()
);
Expand All @@ -45,7 +28,7 @@ describe('xyVis', () => {
value: {
args: {
...rest,
layers: [{ layerType, table: newData, layerId: 'dataLayers-0', type, ...restLayerArgs }],
layers: [{ layerType, table: data, layerId: 'dataLayers-0', type, ...restLayerArgs }],
},
},
});
Expand Down Expand Up @@ -120,6 +103,25 @@ describe('xyVis', () => {
).rejects.toThrowErrorMatchingSnapshot();
});

test('it should throw error if addTimeMarker applied for not time chart', async () => {
const { data, args } = sampleArgs();
const { layers, ...rest } = args;
const { layerId, layerType, table, type, ...restLayerArgs } = sampleLayer;
expect(
xyVisFunction.fn(
data,
{
...rest,
...restLayerArgs,
addTimeMarker: true,
referenceLines: [],
annotationLayers: [],
},
createMockExecutionContext()
)
).rejects.toThrowErrorMatchingSnapshot();
});

test('it should throw error if splitRowAccessor is pointing to the absent column', async () => {
const { data, args } = sampleArgs();
const { layers, ...rest } = args;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
validateFillOpacity,
validateMarkSizeRatioLimits,
validateValueLabels,
validateAddTimeMarker,
validateMinTimeBarInterval,
validateMarkSizeForChartType,
validateMarkSizeRatioWithAccessor,
Expand Down Expand Up @@ -107,6 +108,7 @@ export const xyVisFn: XyVisFn['fn'] = async (data, args, handlers) => {
validateExtent(args.yLeftExtent, hasBar || hasArea, dataLayers);
validateExtent(args.yRightExtent, hasBar || hasArea, dataLayers);
validateFillOpacity(args.fillOpacity, hasArea);
validateAddTimeMarker(dataLayers, args.addTimeMarker);
validateMinTimeBarInterval(dataLayers, hasBar, args.minTimeBarInterval);

const hasNotHistogramBars = !hasHistogramBarLayer(dataLayers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@
* Side Public License, v 1.
*/

import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils';
import { XScaleTypes } from '../constants';
import { CommonXYDataLayerConfigResult } from '../types';

export function isTimeChart(layers: CommonXYDataLayerConfigResult[]) {
return layers.every<CommonXYDataLayerConfigResult>(
(l): l is CommonXYDataLayerConfigResult =>
l.table.columns.find((col) => col.id === l.xAccessor)?.meta.type === 'date' &&
l.xScaleType === XScaleTypes.TIME
(l.xAccessor
? getColumnByAccessor(l.xAccessor, l.table.columns)?.meta.type === 'date'
: false) &&
(!l.xScaleType || l.xScaleType === XScaleTypes.TIME)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ export const strings = {
i18n.translate('expressionXY.xyVis.ariaLabel.help', {
defaultMessage: 'Specifies the aria label of the xy chart',
}),
getAddTimeMakerHelp: () =>
i18n.translate('expressionXY.xyVis.addTimeMaker.help', {
defaultMessage: 'Show time marker',
}),
getMarkSizeRatioHelp: () =>
i18n.translate('expressionXY.xyVis.markSizeRatio.help', {
defaultMessage: 'Specifies the ratio of the dots at the line and area charts',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ export interface XYArgs extends DataLayerArgs {
hideEndzones?: boolean;
valuesInLegend?: boolean;
ariaLabel?: string;
addTimeMarker?: boolean;
markSizeRatio?: number;
minTimeBarInterval?: string;
splitRowAccessor?: ExpressionValueVisDimension | string;
Expand Down Expand Up @@ -236,6 +237,7 @@ export interface LayeredXYArgs {
hideEndzones?: boolean;
valuesInLegend?: boolean;
ariaLabel?: string;
addTimeMarker?: boolean;
markSizeRatio?: number;
minTimeBarInterval?: string;
}
Expand Down Expand Up @@ -263,6 +265,7 @@ export interface XYProps {
hideEndzones?: boolean;
valuesInLegend?: boolean;
ariaLabel?: string;
addTimeMarker?: boolean;
markSizeRatio?: number;
minTimeBarInterval?: string;
splitRowAccessor?: ExpressionValueVisDimension | string;
Expand Down
Loading

0 comments on commit f75b6fa

Please sign in to comment.