From f75b6fa1561fb8592a493c41c08302fddd136760 Mon Sep 17 00:00:00 2001 From: Uladzislau Lasitsa Date: Fri, 20 May 2022 12:50:46 +0300 Subject: [PATCH] [XY] Add `addTimeMarker` arg (#131495) * 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 * Fixed tests * Fix checks Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yaroslav Kuznietsov --- .../expression_xy/common/__mocks__/index.ts | 6 +- .../__snapshots__/xy_vis.test.ts.snap | 2 + .../common_data_layer_args.ts | 1 - .../expression_functions/common_xy_args.ts | 5 + .../expression_functions/layered_xy_vis_fn.ts | 4 +- .../common/expression_functions/validate.ts | 14 + .../expression_functions/xy_vis.test.ts | 40 +- .../common/expression_functions/xy_vis_fn.ts | 2 + .../common/helpers/visualization.ts | 7 +- .../expression_xy/common/i18n/index.tsx | 4 + .../common/types/expression_functions.ts | 3 + .../__snapshots__/xy_chart.test.tsx.snap | 880 +++++++++--------- .../public/components/data_layers.tsx | 4 + .../public/components/xy_chart.test.tsx | 13 +- .../public/components/xy_chart.tsx | 16 +- .../public/components/xy_current_time.tsx | 26 + .../public/helpers/data_layers.tsx | 4 +- .../expression_xy/public/helpers/interval.ts | 5 +- 18 files changed, 580 insertions(+), 456 deletions(-) create mode 100644 src/plugins/chart_expressions/expression_xy/public/components/xy_current_time.tsx diff --git a/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts b/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts index 76e524960b159..1f19428e420bf 100644 --- a/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts +++ b/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts @@ -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' }, @@ -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 { diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap b/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap index 05109cc65446b..e396aace05191 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap @@ -1,5 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`xyVis it should throw error if addTimeMarker applied for not time chart 1`] = `"Only time charts can have current time marker"`; + exports[`xyVis it should throw error if markSizeRatio is lower then 1 or greater then 100 1`] = `"Mark size ratio must be greater or equal to 1 and less or equal to 100"`; exports[`xyVis it should throw error if markSizeRatio is lower then 1 or greater then 100 2`] = `"Mark size ratio must be greater or equal to 1 and less or equal to 100"`; diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_data_layer_args.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_data_layer_args.ts index 0c9085cce7664..f4543c5236ce2 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_data_layer_args.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_data_layer_args.ts @@ -36,7 +36,6 @@ export const commonDataLayerArgs: Omit< xScaleType: { options: [...Object.values(XScaleTypes)], help: strings.getXScaleTypeHelp(), - default: XScaleTypes.ORDINAL, strict: true, }, isHistogram: { diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_xy_args.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_xy_args.ts index 0921760f9f676..2e2e6765734cf 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_xy_args.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/common_xy_args.ts @@ -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(), diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/layered_xy_vis_fn.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/layered_xy_vis_fn.ts index 29624d8037393..fb7c91c682847 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/layered_xy_vis_fn.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/layered_xy_vis_fn.ts @@ -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'); @@ -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 = diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts index 60e590b0f8cca..df7f9ee08632e 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts @@ -17,6 +17,7 @@ import { CommonXYDataLayerConfigResult, ValueLabelMode, CommonXYDataLayerConfig, + ExtendedDataLayerConfigResult, } from '../types'; import { isTimeChart } from '../helpers'; @@ -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: @@ -135,6 +140,15 @@ export const validateValueLabels = ( } }; +export const validateAddTimeMarker = ( + dataLayers: Array, + addTimeMarker?: boolean +) => { + if (addTimeMarker && !isTimeChart(dataLayers)) { + throw new Error(errors.timeMarkerForNotTimeChartsError()); + } +}; + export const validateMarkSizeForChartType = ( markSizeAccessor: ExpressionValueVisDimension | string | undefined, seriesType: SeriesType diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts index 73d4444217d90..8a327ccca9e20 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts @@ -7,7 +7,6 @@ */ 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'; @@ -15,26 +14,10 @@ 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() ); @@ -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 }], }, }, }); @@ -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; diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts index 3de2dd35831e4..4c25e3378d523 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts @@ -25,6 +25,7 @@ import { validateFillOpacity, validateMarkSizeRatioLimits, validateValueLabels, + validateAddTimeMarker, validateMinTimeBarInterval, validateMarkSizeForChartType, validateMarkSizeRatioWithAccessor, @@ -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); diff --git a/src/plugins/chart_expressions/expression_xy/common/helpers/visualization.ts b/src/plugins/chart_expressions/expression_xy/common/helpers/visualization.ts index 8ddbc4bc97f10..66d4c11a9f7ae 100644 --- a/src/plugins/chart_expressions/expression_xy/common/helpers/visualization.ts +++ b/src/plugins/chart_expressions/expression_xy/common/helpers/visualization.ts @@ -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( (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) ); } diff --git a/src/plugins/chart_expressions/expression_xy/common/i18n/index.tsx b/src/plugins/chart_expressions/expression_xy/common/i18n/index.tsx index ba26bb973f64f..ed2ef4a7a57ce 100644 --- a/src/plugins/chart_expressions/expression_xy/common/i18n/index.tsx +++ b/src/plugins/chart_expressions/expression_xy/common/i18n/index.tsx @@ -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', diff --git a/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts b/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts index 0a7b93c495c29..c0336fc67536f 100644 --- a/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts +++ b/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts @@ -207,6 +207,7 @@ export interface XYArgs extends DataLayerArgs { hideEndzones?: boolean; valuesInLegend?: boolean; ariaLabel?: string; + addTimeMarker?: boolean; markSizeRatio?: number; minTimeBarInterval?: string; splitRowAccessor?: ExpressionValueVisDimension | string; @@ -236,6 +237,7 @@ export interface LayeredXYArgs { hideEndzones?: boolean; valuesInLegend?: boolean; ariaLabel?: string; + addTimeMarker?: boolean; markSizeRatio?: number; minTimeBarInterval?: string; } @@ -263,6 +265,7 @@ export interface XYProps { hideEndzones?: boolean; valuesInLegend?: boolean; ariaLabel?: string; + addTimeMarker?: boolean; markSizeRatio?: number; minTimeBarInterval?: string; splitRowAccessor?: ExpressionValueVisDimension | string; diff --git a/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap b/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap index e7a26ec20bbfc..c3d1fc980ad01 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap +++ b/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap @@ -334,6 +334,10 @@ exports[`XYChart component it renders area 1`] = ` } } /> + + + + + + + + + + = ({ @@ -67,6 +69,7 @@ export const DataLayers: FC = ({ shouldShowValueLabels, formattedDatatables, chartHasMoreThanOneBarSeries, + defaultXScaleType, }) => { const colorAssignments = getColorAssignments(layers, formatFactory); return ( @@ -104,6 +107,7 @@ export const DataLayers: FC = ({ timeZone, emphasizeFitting, fillOpacity, + defaultXScaleType, }); const index = `${layer.layerId}-${accessorIndex}`; diff --git a/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.test.tsx b/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.test.tsx index d03a5e648f366..91e5ae8ad1484 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.test.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.test.tsx @@ -1967,17 +1967,10 @@ describe('XYChart component', () => { test('it should pass the formatter function to the axis', () => { const { args } = sampleArgs(); - const instance = shallow(); - - const tickFormatter = instance.find(Axis).first().prop('tickFormat'); - - if (!tickFormatter) { - throw new Error('tickFormatter prop not found'); - } - - tickFormatter('I'); + shallow(); - expect(convertSpy).toHaveBeenCalledWith('I'); + expect(convertSpy).toHaveBeenCalledWith(1652034840000); + expect(convertSpy).toHaveBeenCalledWith(1652122440000); }); test('it should set the tickLabel visibility on the x axis if the tick labels is hidden', () => { diff --git a/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.tsx b/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.tsx index 80048bcb84038..7eceb72ecf75d 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/components/xy_chart.tsx @@ -42,6 +42,7 @@ import { LegendSizeToPixels, } from '@kbn/visualizations-plugin/common/constants'; import type { FilterEvent, BrushEvent, FormatFactory } from '../types'; +import { isTimeChart } from '../../common/helpers'; import type { CommonXYDataLayerConfig, ExtendedYConfig, @@ -81,8 +82,10 @@ import { OUTSIDE_RECT_ANNOTATION_WIDTH, OUTSIDE_RECT_ANNOTATION_WIDTH_SUGGESTION, } from './annotations'; -import { AxisExtentModes, SeriesTypes, ValueLabelModes } from '../../common/constants'; +import { AxisExtentModes, SeriesTypes, ValueLabelModes, XScaleTypes } from '../../common/constants'; import { DataLayers } from './data_layers'; +import { XYCurrentTime } from './xy_current_time'; + import './xy_chart.scss'; declare global { @@ -249,7 +252,10 @@ export function XYChart({ filteredBarLayers.some((layer) => layer.accessors.length > 1) || filteredBarLayers.some((layer) => isDataLayer(layer) && layer.splitAccessor); - const isTimeViz = Boolean(dataLayers.every((l) => l.xScaleType === 'time')); + const isTimeViz = isTimeChart(dataLayers); + + const defaultXScaleType = isTimeViz ? XScaleTypes.TIME : XScaleTypes.ORDINAL; + const isHistogramViz = dataLayers.every((l) => l.isHistogram); const { baseDomain: rawXDomain, extendedDomain: xDomain } = getXDomain( @@ -604,6 +610,11 @@ export function XYChart({ ariaLabel={args.ariaLabel} ariaUseDefaultSummary={!args.ariaLabel} /> + )} {referenceLineLayers.length ? ( diff --git a/src/plugins/chart_expressions/expression_xy/public/components/xy_current_time.tsx b/src/plugins/chart_expressions/expression_xy/public/components/xy_current_time.tsx new file mode 100644 index 0000000000000..68f1dd0d60b13 --- /dev/null +++ b/src/plugins/chart_expressions/expression_xy/public/components/xy_current_time.tsx @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { FC } from 'react'; +import { DomainRange } from '@elastic/charts'; +import { CurrentTime } from '@kbn/charts-plugin/public'; + +interface XYCurrentTime { + enabled: boolean; + isDarkMode: boolean; + domain?: DomainRange; +} + +export const XYCurrentTime: FC = ({ enabled, isDarkMode, domain }) => { + if (!enabled) { + return null; + } + + const domainEnd = domain && 'max' in domain ? domain.max : undefined; + return ; +}; diff --git a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx index 7ac661ed9709d..08761f633f851 100644 --- a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx @@ -53,6 +53,7 @@ type GetSeriesPropsFn = (config: { emphasizeFitting?: boolean; fillOpacity?: number; formattedDatatableInfo: DatatableWithFormatInfo; + defaultXScaleType: XScaleType; }) => SeriesSpec; type GetSeriesNameFn = ( @@ -280,6 +281,7 @@ export const getSeriesProps: GetSeriesPropsFn = ({ emphasizeFitting, fillOpacity, formattedDatatableInfo, + defaultXScaleType, }): SeriesSpec => { const { table, markSizeAccessor } = layer; const isStacked = layer.seriesType.includes('stacked'); @@ -342,7 +344,7 @@ export const getSeriesProps: GetSeriesPropsFn = ({ markSizeAccessor: markSizeColumnId, markFormat: (value) => markFormatter.convert(value), data: rows, - xScaleType: xColumnId ? layer.xScaleType : 'ordinal', + xScaleType: xColumnId ? layer.xScaleType ?? defaultXScaleType : 'ordinal', yScaleType: formatter?.id === 'bytes' && yAxis?.scale === ScaleType.Linear ? ScaleType.LinearBinary diff --git a/src/plugins/chart_expressions/expression_xy/public/helpers/interval.ts b/src/plugins/chart_expressions/expression_xy/public/helpers/interval.ts index a9f68ffc0a29b..5c202bb6200a9 100644 --- a/src/plugins/chart_expressions/expression_xy/public/helpers/interval.ts +++ b/src/plugins/chart_expressions/expression_xy/public/helpers/interval.ts @@ -9,13 +9,14 @@ import { search } from '@kbn/data-plugin/public'; import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils'; import { XYChartProps } from '../../common'; +import { isTimeChart } from '../../common/helpers'; import { getFilteredLayers } from './layers'; -import { isDataLayer } from './visualization'; +import { isDataLayer, getDataLayers } from './visualization'; export function calculateMinInterval({ args: { layers, minTimeBarInterval } }: XYChartProps) { const filteredLayers = getFilteredLayers(layers); if (filteredLayers.length === 0) return; - const isTimeViz = filteredLayers.every((l) => isDataLayer(l) && l.xScaleType === 'time'); + const isTimeViz = isTimeChart(getDataLayers(filteredLayers)); const xColumn = isDataLayer(filteredLayers[0]) && filteredLayers[0].xAccessor &&