From a0756268c1e1f55ab4fb5ab6c75fe5cf91fec96d Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 9 Jun 2022 12:43:37 +0200 Subject: [PATCH 1/5] :sparkles: Add axis extent for histogram --- .../expression_xy/common/__mocks__/index.ts | 4 + .../expression_functions/common_xy_args.ts | 5 + .../expression_xy/common/i18n/index.tsx | 4 + .../common/types/expression_functions.ts | 3 + .../public/components/x_domain.tsx | 15 +- .../public/components/xy_chart.test.tsx | 39 +++ .../public/components/xy_chart.tsx | 4 +- .../axis_extent/axis_extent_settings.tsx | 125 +++++++ .../axis_extent/helpers.test.ts | 166 +++++++++ .../shared_components/axis_extent/helpers.ts | 86 +++++ .../shared_components/axis_extent/index.ts | 15 + .../axis_extent/to_expression.tsx | 34 ++ .../shared_components/axis_extent/types.ts | 10 + .../lens/public/shared_components/index.ts | 9 + .../shared_components/range_input_field.tsx | 80 +++++ .../__snapshots__/to_expression.test.ts.snap | 18 + .../xy_visualization/axes_configuration.ts | 38 +- .../public/xy_visualization/to_expression.ts | 26 +- .../lens/public/xy_visualization/types.ts | 1 + .../axis_settings_popover.test.tsx | 65 +++- .../xy_config_panel/axis_settings_popover.tsx | 331 +++++++++--------- .../xy_config_panel/index.tsx | 21 +- .../xy_config_panel/xy_config_panel.test.tsx | 2 +- 23 files changed, 891 insertions(+), 210 deletions(-) create mode 100644 x-pack/plugins/lens/public/shared_components/axis_extent/axis_extent_settings.tsx create mode 100644 x-pack/plugins/lens/public/shared_components/axis_extent/helpers.test.ts create mode 100644 x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts create mode 100644 x-pack/plugins/lens/public/shared_components/axis_extent/index.ts create mode 100644 x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx create mode 100644 x-pack/plugins/lens/public/shared_components/axis_extent/types.ts create mode 100644 x-pack/plugins/lens/public/shared_components/range_input_field.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 e9810d2764025..d5eaf3b086e65 100644 --- a/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts +++ b/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts @@ -115,6 +115,10 @@ export const createArgsWithLayers = ( yLeft: false, yRight: false, }, + xExtent: { + mode: 'dataBounds', + type: 'axisExtentConfig', + }, yLeftExtent: { mode: 'full', type: 'axisExtentConfig', 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 3c1d36c758f71..ceb24da6a3ec4 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 @@ -37,6 +37,11 @@ export const commonXYArgs: CommonXYFn['args'] = { types: ['string'], help: strings.getYRightTitleHelp(), }, + xExtent: { + types: [AXIS_EXTENT_CONFIG], + help: strings.getXExtentHelp(), + default: `{${AXIS_EXTENT_CONFIG}}`, + }, yLeftExtent: { types: [AXIS_EXTENT_CONFIG], help: strings.getYLeftExtentHelp(), 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 33750f627c206..def7a6170232c 100644 --- a/src/plugins/chart_expressions/expression_xy/common/i18n/index.tsx +++ b/src/plugins/chart_expressions/expression_xy/common/i18n/index.tsx @@ -41,6 +41,10 @@ export const strings = { i18n.translate('expressionXY.xyVis.yRightTitle.help', { defaultMessage: 'Y right axis title', }), + getXExtentHelp: () => + i18n.translate('expressionXY.xyVis.xExtent.help', { + defaultMessage: 'X axis extents', + }), getYLeftExtentHelp: () => i18n.translate('expressionXY.xyVis.yLeftExtent.help', { defaultMessage: 'Y left axis extents', 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 6bd8eb5d37b7c..e87704a919838 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 @@ -196,6 +196,7 @@ export interface XYArgs extends DataLayerArgs { xTitle: string; yTitle: string; yRightTitle: string; + xExtent: AxisExtentConfigResult; yLeftExtent: AxisExtentConfigResult; yRightExtent: AxisExtentConfigResult; yLeftScale: YScaleType; @@ -230,6 +231,7 @@ export interface LayeredXYArgs { xTitle: string; yTitle: string; yRightTitle: string; + xExtent: AxisExtentConfigResult; yLeftExtent: AxisExtentConfigResult; yRightExtent: AxisExtentConfigResult; yLeftScale: YScaleType; @@ -261,6 +263,7 @@ export interface XYProps { xTitle: string; yTitle: string; yRightTitle: string; + xExtent: AxisExtentConfigResult; yLeftExtent: AxisExtentConfigResult; yRightExtent: AxisExtentConfigResult; yLeftScale: YScaleType; diff --git a/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx b/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx index 9f6237f965842..42d66570e8fe6 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx @@ -15,7 +15,7 @@ import { getAccessorByDimension, getColumnByAccessor, } from '@kbn/visualizations-plugin/common/utils'; -import type { CommonXYDataLayerConfig } from '../../common'; +import type { AxisExtentConfigResult, CommonXYDataLayerConfig } from '../../common'; export interface XDomain { min?: number; @@ -43,7 +43,8 @@ export const getXDomain = ( layers: CommonXYDataLayerConfig[], minInterval: number | undefined, isTimeViz: boolean, - isHistogram: boolean + isHistogram: boolean, + xExtent?: AxisExtentConfigResult ) => { const appliedTimeRange = getAppliedTimeRange(layers)?.timeRange; const from = appliedTimeRange?.from; @@ -59,6 +60,16 @@ export const getXDomain = ( : undefined; if (isHistogram && isFullyQualified(baseDomain)) { + if (xExtent) { + return { + extendedDomain: { + min: xExtent.lowerBound ?? NaN, + max: xExtent.upperBound ?? NaN, + minInterval: baseDomain.minInterval, + }, + baseDomain, + }; + } const xValues = uniq( layers .flatMap(({ table, xAccessor }) => { 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 a6c832045543f..a19132ba8193e 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 @@ -496,6 +496,33 @@ describe('XYChart component', () => { }); }); + describe('x axis extents', () => { + const { args } = sampleArgs(); + + test('it passes custom x axis extents to elastic-charts settings spec', () => { + { + const component = shallow( + + ); + expect(component.find(Settings).prop('xDomain')).toEqual({ + min: 123, + max: 456, + }); + } + }); + }); + describe('y axis extents', () => { const { args } = sampleArgs(); @@ -2256,6 +2283,10 @@ describe('XYChart component', () => { yLeft: 0, yRight: 0, }, + xExtent: { + mode: 'dataBounds', + type: 'axisExtentConfig', + }, yLeftExtent: { mode: 'full', type: 'axisExtentConfig', @@ -2347,6 +2378,10 @@ describe('XYChart component', () => { yLeft: 0, yRight: 0, }, + xExtent: { + mode: 'dataBounds', + type: 'axisExtentConfig', + }, yLeftExtent: { mode: 'full', type: 'axisExtentConfig', @@ -2423,6 +2458,10 @@ describe('XYChart component', () => { yLeft: 0, yRight: 0, }, + xExtent: { + mode: 'dataBounds', + type: 'axisExtentConfig', + }, yLeftExtent: { mode: 'full', type: 'axisExtentConfig', 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 e01086428f393..ef9b91539bffc 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 @@ -167,6 +167,7 @@ export function XYChart({ gridlinesVisibilitySettings, valueLabels, hideEndzones, + xExtent, yLeftExtent, yRightExtent, valuesInLegend, @@ -280,7 +281,8 @@ export function XYChart({ dataLayers, minInterval, isTimeViz, - isHistogramViz + isHistogramViz, + xExtent ); const getYAxesTitles = (axisSeries: Series[]) => { diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/axis_extent_settings.tsx b/x-pack/plugins/lens/public/shared_components/axis_extent/axis_extent_settings.tsx new file mode 100644 index 0000000000000..77f75ae6845ab --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/axis_extent_settings.tsx @@ -0,0 +1,125 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { EuiFormRow, EuiButtonGroup, htmlIdGenerator } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { RangeInputField } from '../range_input_field'; +import { validateAxisDomain } from './helpers'; +import { UnifiedAxisExtentConfig } from './types'; + +const idPrefix = htmlIdGenerator()(); +export function BucketAxisBoundsControl({ + testSubjPrefix, + extent, + setExtent, + dataBounds, +}: { + testSubjPrefix: string; + extent: UnifiedAxisExtentConfig; + setExtent: (newExtent: UnifiedAxisExtentConfig | undefined) => void; + dataBounds: { min: number; max: number } | undefined; +}) { + const boundaryError = !validateAxisDomain(extent); + return ( + <> + + { + const newMode = id.replace(idPrefix, '') as UnifiedAxisExtentConfig['mode']; + setExtent({ + ...extent, + mode: newMode, + lowerBound: newMode === 'custom' && dataBounds ? dataBounds.min : undefined, + upperBound: newMode === 'custom' && dataBounds ? dataBounds.max : undefined, + }); + }} + /> + + {extent?.mode === 'custom' && ( + { + const val = Number(e.target.value); + const isEmptyValue = e.target.value === '' || Number.isNaN(Number(val)); + setExtent({ + ...extent, + lowerBound: isEmptyValue ? undefined : val, + }); + }} + onLowerValueBlur={() => { + if (extent.lowerBound === undefined && dataBounds) { + setExtent({ + ...extent, + lowerBound: dataBounds.min, + }); + } + }} + upperValue={extent.upperBound ?? ''} + onUpperValueChange={(e) => { + const val = Number(e.target.value); + const isEmptyValue = e.target.value === '' || Number.isNaN(Number(val)); + setExtent({ + ...extent, + upperBound: isEmptyValue ? undefined : val, + }); + }} + onUpperValueBlur={() => { + if (extent.upperBound === undefined && dataBounds) { + setExtent({ + ...extent, + upperBound: dataBounds.max, + }); + } + }} + /> + )} + + ); +} diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.test.ts b/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.test.ts new file mode 100644 index 0000000000000..af36dc765cc82 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.test.ts @@ -0,0 +1,166 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Datatable } from '@kbn/expressions-plugin'; +import { createMockDatasource } from '../../mocks'; +import { FramePublicAPI, OperationDescriptor } from '../../types'; +import { + hasNumericHistogramDimension, + validateAxisDomain, + getDataBounds, + validateZeroInclusivityExtent, +} from './helpers'; + +describe('validateAxisDomain', () => { + it('should return true for valid range', () => { + expect(validateAxisDomain({ lowerBound: 100, upperBound: 200 })).toBeTruthy(); + expect(validateAxisDomain({ lowerBound: -100, upperBound: 0 })).toBeTruthy(); + expect(validateAxisDomain({ lowerBound: -100, upperBound: -50 })).toBeTruthy(); + }); + + it('should return false for invalid ranges', () => { + expect(validateAxisDomain(undefined)).toBeFalsy(); + expect(validateAxisDomain({})).toBeFalsy(); + expect(validateAxisDomain({ lowerBound: 100, upperBound: 50 })).toBeFalsy(); + }); + + it('should return false for single value range', () => { + expect(validateAxisDomain({ lowerBound: 200, upperBound: 200 })).toBeFalsy(); + }); + + it('should return false for partial range', () => { + expect(validateAxisDomain({ lowerBound: undefined, upperBound: 50 })).toBeFalsy(); + expect(validateAxisDomain({ lowerBound: 0, upperBound: undefined })).toBeFalsy(); + }); +}); + +describe('validateZeroInclusivityExtent', () => { + it('should return true for a 0-100 range', () => { + expect(validateZeroInclusivityExtent({ lowerBound: 0, upperBound: 100 })).toBeTruthy(); + }); + + it('should support negative lower values', () => { + expect(validateZeroInclusivityExtent({ lowerBound: -100, upperBound: 100 })).toBeTruthy(); + }); + + it('should return false for ranges that do not include zero', () => { + expect(validateZeroInclusivityExtent({ lowerBound: 50, upperBound: 100 })).toBeFalsy(); + expect(validateZeroInclusivityExtent({ lowerBound: -150, upperBound: -100 })).toBeFalsy(); + }); + + it('should return false for no extent', () => { + expect(validateZeroInclusivityExtent(undefined)).toBeFalsy(); + }); +}); + +describe('hasNumericHistogramDimension', () => { + const datasourceLayers: FramePublicAPI['datasourceLayers'] = { + first: createMockDatasource('test').publicAPIMock, + }; + it('should return true if a numeric histogram is present', () => { + datasourceLayers.first.getOperationForColumnId = jest.fn( + () => ({ isBucketed: true, scale: 'interval', dataType: 'number' } as OperationDescriptor) + ); + expect(hasNumericHistogramDimension(datasourceLayers.first, 'columnId')).toBeTruthy(); + }); + + it('should return false if a date histogram is present', () => { + datasourceLayers.first.getOperationForColumnId = jest.fn( + () => ({ isBucketed: true, scale: 'interval', dataType: 'date' } as OperationDescriptor) + ); + expect(hasNumericHistogramDimension(datasourceLayers.first, 'columnId')).toBeFalsy(); + }); + + it('should return false for ordinal types', () => { + datasourceLayers.first.getOperationForColumnId = jest.fn( + () => ({ isBucketed: true, scale: 'ordinal', dataType: 'number' } as OperationDescriptor) + ); + expect(hasNumericHistogramDimension(datasourceLayers.first, 'columnId')).toBeFalsy(); + }); + + it('should return false for no dimension', () => { + datasourceLayers.first.getOperationForColumnId = jest.fn( + () => ({ isBucketed: true, scale: 'ordinal', dataType: 'number' } as OperationDescriptor) + ); + expect(hasNumericHistogramDimension(datasourceLayers.first)).toBeFalsy(); + }); + + it('should return false for no operation found for the columnId', () => { + expect(hasNumericHistogramDimension(datasourceLayers.first, 'columnId')).toBeFalsy(); + }); +}); + +describe('getDataBounds', () => { + function createTable( + layerId: string, + columnId: string, + { bounds: [min, max], addEmptyRows }: { bounds: [number, number]; addEmptyRows?: boolean } + ) { + return { + [layerId]: { + rows: [{ [columnId]: max }, ...(addEmptyRows ? [{}, {}] : []), { [columnId]: min }], + }, + } as Record | undefined; + } + + it('should return data bounds for a table', () => { + expect( + getDataBounds('layerId', createTable('layerId', 'columnId', { bounds: [5, 10] }), 'columnId') + ).toEqual({ + min: 5, + max: 10, + }); + }); + + it('should return no data bounds for empty table', () => { + expect(getDataBounds('layerId', undefined, 'columnId')).toBeUndefined(); + }); + + it('should return no data bounds for missing layer in table', () => { + expect( + getDataBounds( + 'layerId', + createTable('otherLayerId', 'columnId', { bounds: [5, 10] }), + 'columnId' + ) + ).toBeUndefined(); + }); + + it('should return no data bounds for missing column in table', () => { + expect( + getDataBounds( + 'layerId', + createTable('layerId', 'otherColumnId', { bounds: [5, 10] }), + 'columnId' + ) + ).toBeUndefined(); + }); + it('should be resilient to missing values', () => { + expect( + getDataBounds( + 'layerId', + createTable('layerId', 'columnId', { bounds: [5, 10], addEmptyRows: true }), + 'columnId' + ) + ).toEqual({ + min: 5, + max: 10, + }); + }); + it('should be resilient to single values range', () => { + expect( + getDataBounds( + 'layerId', + createTable('layerId', 'columnId', { bounds: [5, 5], addEmptyRows: true }), + 'columnId' + ) + ).toEqual({ + min: 5, + max: 5, + }); + }); +}); diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts b/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts new file mode 100644 index 0000000000000..a8a2cf007210a --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts @@ -0,0 +1,86 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Datatable } from '@kbn/expressions-plugin'; +import type { DatasourcePublicAPI } from '../../types'; + +/** + * Returns true if the provided extent includes 0 + * @param extent + * @returns boolean + */ +export function validateZeroInclusivityExtent(extent?: { + lowerBound?: number; + upperBound?: number; +}) { + return ( + extent && + extent.lowerBound != null && + extent.upperBound != null && + extent.lowerBound <= 0 && + extent.upperBound >= 0 + ); +} + +/** + * Returns true if the provided extent is a valid range + * @param extent + * @returns boolean + */ +export function validateAxisDomain(extents?: { lowerBound?: number; upperBound?: number }) { + return ( + extents && + extents.lowerBound != null && + extents.upperBound != null && + extents.upperBound > extents.lowerBound + ); +} +/** + * Returns true if the provided column is a numeric histogram dimension + * @param extent + * @returns boolean + */ +export function hasNumericHistogramDimension( + datasourceLayer: DatasourcePublicAPI, + columnId?: string +) { + if (!columnId) { + return false; + } + + const operation = datasourceLayer?.getOperationForColumnId(columnId); + + return Boolean(operation && operation.dataType === 'number' && operation.scale === 'interval'); +} + +/** + * Finds the table data min and max + * @param layerId + * @param tables + * @param columnId + * @returns + */ +export function getDataBounds( + layerId: string, + tables: Record | undefined, + columnId?: string +) { + const table = tables?.[layerId]; + if (columnId && table) { + const sortedRows = table.rows + .map(({ [columnId]: value }) => value) + // ignore missing hit + .filter((v) => v != null) + .sort((a, b) => a - b); + if (sortedRows.length) { + return { + min: sortedRows[0], + max: sortedRows[sortedRows.length - 1], + }; + } + } +} diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/index.ts b/x-pack/plugins/lens/public/shared_components/axis_extent/index.ts new file mode 100644 index 0000000000000..45c9e147c2006 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/index.ts @@ -0,0 +1,15 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { BucketAxisBoundsControl } from './axis_extent_settings'; +export { + validateAxisDomain, + validateZeroInclusivityExtent, + hasNumericHistogramDimension, + getDataBounds, +} from './helpers'; +export { axisExtentConfigToExpression } from './to_expression'; diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx b/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx new file mode 100644 index 0000000000000..e16dbb08a6b8d --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx @@ -0,0 +1,34 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Ast } from '@kbn/interpreter'; +import { UnifiedAxisExtentConfig } from './types'; + +// TODO: import it from the expression config directly? +const CHART_TO_FN_NAME = { + xy: 'axisExtentConfig', + heatmap: 'heatmap_axis_extent', +} as const; + +export const axisExtentConfigToExpression = ( + extent: UnifiedAxisExtentConfig | undefined, + chartType: keyof typeof CHART_TO_FN_NAME = 'xy' +): Ast => ({ + type: 'expression', + chain: [ + { + type: 'function', + function: CHART_TO_FN_NAME[chartType], + arguments: { + // rely on expression default value here + mode: extent?.mode ? [extent.mode] : [], + lowerBound: extent?.lowerBound !== undefined ? [extent?.lowerBound] : [], + upperBound: extent?.upperBound !== undefined ? [extent?.upperBound] : [], + }, + }, + ], +}); diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/types.ts b/x-pack/plugins/lens/public/shared_components/axis_extent/types.ts new file mode 100644 index 0000000000000..77b91be17360c --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/types.ts @@ -0,0 +1,10 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { AxisExtentConfig } from '@kbn/expression-xy-plugin/common'; + +export type UnifiedAxisExtentConfig = AxisExtentConfig; diff --git a/x-pack/plugins/lens/public/shared_components/index.ts b/x-pack/plugins/lens/public/shared_components/index.ts index b2428532a72c9..7da63a6947211 100644 --- a/x-pack/plugins/lens/public/shared_components/index.ts +++ b/x-pack/plugins/lens/public/shared_components/index.ts @@ -9,6 +9,15 @@ export type { ToolbarPopoverProps } from './toolbar_popover'; export { ToolbarPopover } from './toolbar_popover'; export { LegendSettingsPopover } from './legend_settings_popover'; export { PalettePicker } from './palette_picker'; +export { RangeInputField } from './range_input_field'; +export { + BucketAxisBoundsControl, + validateAxisDomain, + validateZeroInclusivityExtent, + hasNumericHistogramDimension, + getDataBounds, + axisExtentConfigToExpression, +} from './axis_extent'; export { TooltipWrapper } from './tooltip_wrapper'; export * from './coloring'; export { useDebouncedValue } from './debounced_value'; diff --git a/x-pack/plugins/lens/public/shared_components/range_input_field.tsx b/x-pack/plugins/lens/public/shared_components/range_input_field.tsx new file mode 100644 index 0000000000000..eb7c7e5299e84 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/range_input_field.tsx @@ -0,0 +1,80 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { EuiFormRow, EuiFieldNumber, EuiFormControlLayoutDelimited } from '@elastic/eui'; + +type RangeInputFieldProps = Partial<{ + isInvalid: boolean; + label: string; + helpText: string; + error: string; + lowerValue: number | ''; + upperValue: number | ''; + onLowerValueChange: React.ChangeEventHandler | undefined; + onUpperValueChange: React.ChangeEventHandler | undefined; + onLowerValueBlur: React.FocusEventHandler | undefined; + onUpperValueBlur: React.FocusEventHandler | undefined; + testSubjLayout?: string; + testSubjLower?: string; + testSubjUpper?: string; +}>; + +export function RangeInputField({ + isInvalid, + label, + helpText, + error, + lowerValue, + onLowerValueChange, + onLowerValueBlur, + upperValue, + onUpperValueChange, + onUpperValueBlur, + testSubjLayout, + testSubjLower, + testSubjUpper, +}: RangeInputFieldProps) { + return ( + + + } + endControl={ + + } + /> + + ); +} diff --git a/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap b/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap index 46d7f535f090a..9084f203be2ea 100644 --- a/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap +++ b/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap @@ -210,6 +210,24 @@ Object { "valuesInLegend": Array [ false, ], + "xExtent": Array [ + Object { + "chain": Array [ + Object { + "arguments": Object { + "lowerBound": Array [], + "mode": Array [ + "full", + ], + "upperBound": Array [], + }, + "function": "axisExtentConfig", + "type": "function", + }, + ], + "type": "expression", + }, + ], "xTitle": Array [ "", ], diff --git a/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts b/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts index 786c184e20dfa..f61c7ef1db0a6 100644 --- a/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts +++ b/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts @@ -9,6 +9,7 @@ import { AxisExtentConfig } from '@kbn/expression-xy-plugin/common'; import { Datatable } from '@kbn/expressions-plugin/public'; import type { IFieldFormat, SerializedFieldFormat } from '@kbn/field-formats-plugin/common'; import { FormatFactory } from '../../common'; +import { validateAxisDomain, validateZeroInclusivityExtent } from '../shared_components'; import { XYDataLayerConfig } from './types'; interface FormattedMetric { @@ -31,6 +32,28 @@ export function isFormatterCompatible( return formatter1.id === formatter2.id; } +export function getXDomain(layers: XYDataLayerConfig[] = [], tables?: Record) { + const dataBounds = layers.reduce( + (bounds, layer) => { + const table = tables?.[layer.layerId]; + if (layer.xAccessor && table) { + const sortedRows = table.rows + .map(({ [layer.xAccessor!]: xValue }) => xValue) + .sort((a, b) => a - b); + return { + min: Math.min(bounds.min, sortedRows[0]), + max: Math.max(bounds.max, sortedRows[sortedRows.length - 1]), + }; + } + return bounds; + }, + { min: Infinity, max: -Infinity } + ); + if (isFinite(dataBounds.min) && isFinite(dataBounds.max)) { + return dataBounds; + } +} + export function groupAxesByType(layers: XYDataLayerConfig[], tables?: Record) { const series: { auto: FormattedMetric[]; @@ -126,15 +149,8 @@ export function getAxesConfiguration( } export function validateExtent(hasBarOrArea: boolean, extent?: AxisExtentConfig) { - const inclusiveZeroError = - extent && - hasBarOrArea && - ((extent.lowerBound !== undefined && extent.lowerBound > 0) || - (extent.upperBound !== undefined && extent.upperBound) < 0); - const boundaryError = - extent && - extent.lowerBound !== undefined && - extent.upperBound !== undefined && - extent.upperBound <= extent.lowerBound; - return { inclusiveZeroError, boundaryError }; + return { + inclusiveZeroError: hasBarOrArea && !validateZeroInclusivityExtent(extent), + boundaryError: !validateAxisDomain(extent), + }; } diff --git a/x-pack/plugins/lens/public/xy_visualization/to_expression.ts b/x-pack/plugins/lens/public/xy_visualization/to_expression.ts index ff5a692a76e96..26d4350beb9cd 100644 --- a/x-pack/plugins/lens/public/xy_visualization/to_expression.ts +++ b/x-pack/plugins/lens/public/xy_visualization/to_expression.ts @@ -11,7 +11,7 @@ import type { PaletteRegistry } from '@kbn/coloring'; import { EventAnnotationServiceType } from '@kbn/event-annotation-plugin/public'; import { LegendSize } from '@kbn/visualizations-plugin/common/constants'; -import type { AxisExtentConfig, YConfig, ExtendedYConfig } from '@kbn/expression-xy-plugin/common'; +import type { YConfig, ExtendedYConfig } from '@kbn/expression-xy-plugin/common'; import { State, XYDataLayerConfig, @@ -32,6 +32,7 @@ import { } from './visualization_helpers'; import { getUniqueLabels } from './annotations/helpers'; import { layerTypes } from '../../common'; +import { axisExtentConfigToExpression } from '../shared_components'; export const getSortedAccessors = ( datasource: DatasourcePublicAPI, @@ -248,8 +249,9 @@ export const buildExpression = ( emphasizeFitting: [state.emphasizeFitting || false], curveType: [state.curveType || 'LINEAR'], fillOpacity: [state.fillOpacity || 0.3], - yLeftExtent: [axisExtentConfigToExpression(state.yLeftExtent, validDataLayers)], - yRightExtent: [axisExtentConfigToExpression(state.yRightExtent, validDataLayers)], + xExtent: [axisExtentConfigToExpression(state.xExtent)], + yLeftExtent: [axisExtentConfigToExpression(state.yLeftExtent)], + yRightExtent: [axisExtentConfigToExpression(state.yRightExtent)], yLeftScale: [state.yLeftScale || 'linear'], yRightScale: [state.yRightScale || 'linear'], axisTitlesVisibilitySettings: [ @@ -530,21 +532,3 @@ const extendedYConfigToExpression = (yConfig: ExtendedYConfig, defaultColor?: st ], }; }; - -const axisExtentConfigToExpression = ( - extent: AxisExtentConfig | undefined, - layers: ValidXYDataLayerConfig[] -): Ast => ({ - type: 'expression', - chain: [ - { - type: 'function', - function: 'axisExtentConfig', - arguments: { - mode: [extent?.mode ?? 'full'], - lowerBound: extent?.lowerBound !== undefined ? [extent?.lowerBound] : [], - upperBound: extent?.upperBound !== undefined ? [extent?.upperBound] : [], - }, - }, - ], -}); diff --git a/x-pack/plugins/lens/public/xy_visualization/types.ts b/x-pack/plugins/lens/public/xy_visualization/types.ts index d4a83c6f561a3..67cf7989fce76 100644 --- a/x-pack/plugins/lens/public/xy_visualization/types.ts +++ b/x-pack/plugins/lens/public/xy_visualization/types.ts @@ -86,6 +86,7 @@ export interface XYState { fittingFunction?: FittingFunction; emphasizeFitting?: boolean; endValue?: EndValue; + xExtent?: AxisExtentConfig; yLeftExtent?: AxisExtentConfig; yRightExtent?: AxisExtentConfig; layers: XYLayerConfig[]; diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.test.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.test.tsx index b5c45cd517432..ce1e5e57c62c4 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.test.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.test.tsx @@ -10,6 +10,17 @@ import { shallowWithIntl as shallow } from '@kbn/test-jest-helpers'; import { AxisSettingsPopover, AxisSettingsPopoverProps } from './axis_settings_popover'; import { ToolbarPopover } from '../../shared_components'; import { layerTypes } from '../../../common'; +import { ShallowWrapper } from 'enzyme'; + +function getRangeInputComponent(component: ShallowWrapper) { + return component + .find('[testSubjPrefix="lnsXY"]') + .shallow() + .find('RangeInputField') + .shallow() + .find('EuiFormControlLayoutDelimited') + .shallow(); +} describe('Axes Settings', () => { let props: AxisSettingsPopoverProps; @@ -112,18 +123,64 @@ describe('Axes Settings', () => { expect(component.find('[data-test-subj="lnsXY_axisBounds_groups"]').length).toBe(0); }); - it('renders bound inputs if mode is custom', () => { + it('renders 3 options for metric bound inputs', () => { + const setSpy = jest.fn(); + const component = shallow( + + ); + const boundInput = component.find('[testSubjPrefix="lnsXY"]').shallow(); + const buttonGroup = boundInput.find('[data-test-subj="lnsXY_axisBounds_groups"]'); + expect(buttonGroup.prop('options')).toHaveLength(3); + }); + + it('renders metric (y) bound inputs if mode is custom', () => { + const setSpy = jest.fn(); + const component = shallow( + + ); + const rangeInput = getRangeInputComponent(component); + const lower = rangeInput.find('[data-test-subj="lnsXY_axisExtent_lowerBound"]'); + const upper = rangeInput.find('[data-test-subj="lnsXY_axisExtent_upperBound"]'); + expect(lower.prop('value')).toEqual(123); + expect(upper.prop('value')).toEqual(456); + }); + + it('renders 2 options for metric bound inputs', () => { + const setSpy = jest.fn(); + const component = shallow( + + ); + const boundInput = component.find('[testSubjPrefix="lnsXY"]').shallow(); + const buttonGroup = boundInput.find('[data-test-subj="lnsXY_axisBounds_groups"]'); + expect(buttonGroup.prop('options')).toHaveLength(2); + }); + + it('renders bucket (x) bound inputs if mode is custom', () => { const setSpy = jest.fn(); const component = shallow( ); - const rangeInput = component - .find('[data-test-subj="lnsXY_axisExtent_customBounds"]') - .shallow(); + const rangeInput = getRangeInputComponent(component); const lower = rangeInput.find('[data-test-subj="lnsXY_axisExtent_lowerBound"]'); const upper = rangeInput.find('[data-test-subj="lnsXY_axisExtent_upperBound"]'); expect(lower.prop('value')).toEqual(123); diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx index 2f824cddcd9fc..28544cb12f0ba 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx @@ -5,15 +5,13 @@ * 2.0. */ -import React, { useEffect, useState } from 'react'; +import React, { useCallback } from 'react'; import { EuiSwitch, IconType, EuiFormRow, EuiButtonGroup, htmlIdGenerator, - EuiFieldNumber, - EuiFormControlLayoutDelimited, EuiSelect, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -21,7 +19,13 @@ import { isEqual } from 'lodash'; import { AxesSettingsConfig, AxisExtentConfig, YScaleType } from '@kbn/expression-xy-plugin/common'; import { ToolbarButtonProps } from '@kbn/kibana-react-plugin/public'; import { XYLayerConfig } from '../types'; -import { ToolbarPopover, useDebouncedValue, AxisTitleSettings } from '../../shared_components'; +import { + ToolbarPopover, + useDebouncedValue, + AxisTitleSettings, + RangeInputField, + BucketAxisBoundsControl, +} from '../../shared_components'; import { isHorizontalChart } from '../state_helpers'; import { EuiIconAxisBottom } from '../../assets/axis_bottom'; import { EuiIconAxisLeft } from '../../assets/axis_left'; @@ -201,7 +205,6 @@ const axisOrientationOptions: Array<{ }, ]; -const noop = () => {}; const idPrefix = htmlIdGenerator()(); export const AxisSettingsPopover: React.FunctionComponent = ({ layers, @@ -231,37 +234,25 @@ export const AxisSettingsPopover: React.FunctionComponent { + const { inclusiveZeroError, boundaryError } = validateExtent(hasBarOrAreaOnAxis, newExtent); + if (setExtent && newExtent && !boundaryError && !isEqual(newExtent, extent)) { + if (axis === 'x' || !inclusiveZeroError) { + setExtent(newExtent); + } + } + }, + [extent, axis, hasBarOrAreaOnAxis, setExtent] + ); + + const { inputValue: localExtent, handleInputChange: setLocalExtent } = useDebouncedValue< AxisExtentConfig | undefined >({ value: extent, - onChange: setExtent || noop, + onChange: onExtentChange, }); - const [localExtent, setLocalExtent] = useState(debouncedExtent); - - const { inclusiveZeroError, boundaryError } = validateExtent(hasBarOrAreaOnAxis, localExtent); - - useEffect(() => { - // set global extent if local extent is not invalid - if ( - setExtent && - !inclusiveZeroError && - !boundaryError && - localExtent && - !isEqual(localExtent, debouncedExtent) - ) { - setDebouncedExtent(localExtent); - } - }, [ - localExtent, - inclusiveZeroError, - boundaryError, - setDebouncedExtent, - debouncedExtent, - setExtent, - ]); - return ( )} - {localExtent && setExtent && ( - + ) : ( + + ))} + + ); +}; + +function MetricAxisBoundsControl({ + extent, + setExtent, + dataBounds, + hasBarOrAreaOnAxis, + hasPercentageAxis, + testSubjPrefix, +}: Required> & { + dataBounds: AxisSettingsPopoverProps['dataBounds']; + hasBarOrAreaOnAxis: boolean; + hasPercentageAxis: boolean; + testSubjPrefix: string; +}) { + const { inclusiveZeroError, boundaryError } = validateExtent(hasBarOrAreaOnAxis, extent); + return ( + <> + + - { - const newMode = id.replace(idPrefix, '') as AxisExtentConfig['mode']; - setLocalExtent({ - ...localExtent, - mode: newMode, - lowerBound: - newMode === 'custom' && dataBounds ? Math.min(0, dataBounds.min) : undefined, - upperBound: newMode === 'custom' && dataBounds ? dataBounds.max : undefined, - }); - }} - /> - - )} - {localExtent?.mode === 'custom' && !hasPercentageAxis && ( - { + const newMode = id.replace(idPrefix, '') as AxisExtentConfig['mode']; + setExtent({ + ...extent, + mode: newMode, + lowerBound: + newMode === 'custom' && dataBounds ? Math.min(0, dataBounds.min) : undefined, + upperBound: newMode === 'custom' && dataBounds ? dataBounds.max : undefined, + }); + }} + /> + + {extent?.mode === 'custom' && !hasPercentageAxis && ( + - { - const val = Number(e.target.value); - if (e.target.value === '' || Number.isNaN(Number(val))) { - setLocalExtent({ - ...localExtent, - lowerBound: undefined, - }); - } else { - setLocalExtent({ - ...localExtent, - lowerBound: val, - }); - } - }} - onBlur={() => { - if (localExtent.lowerBound === undefined && dataBounds) { - setLocalExtent({ - ...localExtent, - lowerBound: Math.min(0, dataBounds.min), - }); - } - }} - step="any" - controlOnly - /> + testSubjLayout={`${testSubjPrefix}_axisExtent_customBounds`} + testSubjLower={`${testSubjPrefix}_axisExtent_lowerBound`} + testSubjUpper={`${testSubjPrefix}_axisExtent_upperBound`} + lowerValue={extent.lowerBound ?? ''} + onLowerValueChange={(e) => { + const val = Number(e.target.value); + const isEmptyValue = e.target.value === '' || Number.isNaN(Number(val)); + setExtent({ + ...extent, + lowerBound: isEmptyValue ? undefined : val, + }); + }} + onLowerValueBlur={() => { + if (extent.lowerBound === undefined && dataBounds) { + setExtent({ + ...extent, + lowerBound: Math.min(0, dataBounds.min), + }); } - endControl={ - { - const val = Number(e.target.value); - if (e.target.value === '' || Number.isNaN(Number(val))) { - setLocalExtent({ - ...localExtent, - upperBound: undefined, - }); - } else { - setLocalExtent({ - ...localExtent, - upperBound: val, - }); - } - }} - onBlur={() => { - if (localExtent.upperBound === undefined && dataBounds) { - setLocalExtent({ - ...localExtent, - upperBound: dataBounds.max, - }); - } - }} - step="any" - controlOnly - /> + }} + upperValue={extent.upperBound ?? ''} + onUpperValueChange={(e) => { + const val = Number(e.target.value); + const isEmptyValue = e.target.value === '' || Number.isNaN(Number(val)); + setExtent({ + ...extent, + upperBound: isEmptyValue ? undefined : val, + }); + }} + onUpperValueBlur={() => { + if (extent.upperBound === undefined && dataBounds) { + setExtent({ + ...extent, + upperBound: dataBounds.max, + }); } - compressed - /> - + }} + /> )} - + ); -}; +} diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx index c431c2ae224b4..96f8ac34d85c1 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx @@ -16,12 +16,12 @@ import { State, XYState } from '../types'; import { isHorizontalChart } from '../state_helpers'; import { LegendSettingsPopover } from '../../shared_components'; import { AxisSettingsPopover } from './axis_settings_popover'; -import { getAxesConfiguration, GroupsConfiguration } from '../axes_configuration'; +import { getAxesConfiguration, getXDomain, GroupsConfiguration } from '../axes_configuration'; import { VisualOptionsPopover } from './visual_options_popover'; import { getScaleType } from '../to_expression'; import { TooltipWrapper } from '../../shared_components'; import { getDefaultVisualValuesForLayer } from '../../shared_components/datasource_default_values'; -import { getDataLayers } from '../visualization_helpers'; +import { checkScaleOperation, getDataLayers } from '../visualization_helpers'; import { LegendSettingsPopoverProps } from '../../shared_components/legend_settings_popover'; type UnwrapArray = T extends Array ? P : T; @@ -122,6 +122,7 @@ export const XyToolbar = memo(function XyToolbar( const shouldRotate = state?.layers.length ? isHorizontalChart(state.layers) : false; const axisGroups = getAxesConfiguration(dataLayers, shouldRotate, frame.activeData); const dataBounds = getDataBounds(frame.activeData, axisGroups); + const xDataBounds = getXDomain(dataLayers, frame.activeData); const tickLabelsVisibilitySettings = { x: state?.tickLabelsVisibilitySettings?.x ?? true, @@ -249,6 +250,15 @@ export const XyToolbar = memo(function XyToolbar( }, [setState, state] ); + const setXExtent = useCallback( + (extent: AxisExtentConfig | undefined) => { + setState({ + ...state, + xExtent: extent, + }); + }, + [setState, state] + ); const hasBarOrAreaOnRightAxis = Boolean( axisGroups .find((group) => group.groupId === 'right') @@ -289,6 +299,10 @@ export const XyToolbar = memo(function XyToolbar( } ); + const hasNumberHistogram = dataLayers.some( + checkScaleOperation('interval', 'number', props.frame.datasourceLayers) + ); + // Ask the datasource if it has a say about default truncation value const defaultParamsFromDatasources = getDefaultVisualValuesForLayer( state, @@ -485,6 +499,9 @@ export const XyToolbar = memo(function XyToolbar( useMultilayerTimeAxis={ isTimeHistogramModeEnabled && !useLegacyTimeAxis && !shouldRotate } + extent={hasNumberHistogram ? state?.xExtent || { mode: 'dataBounds' } : undefined} + setExtent={setXExtent} + dataBounds={xDataBounds} /> { }); expect(component.find(AxisSettingsPopover).at(0).prop('setExtent')).toBeTruthy(); expect(component.find(AxisSettingsPopover).at(1).prop('extent')).toBeFalsy(); - expect(component.find(AxisSettingsPopover).at(1).prop('setExtent')).toBeFalsy(); + expect(component.find(AxisSettingsPopover).at(1).prop('setExtent')).toBeTruthy(); // default extent expect(component.find(AxisSettingsPopover).at(2).prop('extent')).toEqual({ mode: 'full', From f931f547975bf07ff7c4bb93bc62bef8f25a5808 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 9 Jun 2022 12:53:53 +0200 Subject: [PATCH 2/5] :recycle: Reuse shared functions --- .../shared_components/axis_extent/helpers.ts | 2 +- .../xy_visualization/axes_configuration.ts | 17 +++++++++-------- .../xy_visualization/xy_config_panel/index.tsx | 8 ++++---- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts b/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts index a8a2cf007210a..42fc313d6a8ef 100644 --- a/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/helpers.ts @@ -58,7 +58,7 @@ export function hasNumericHistogramDimension( } /** - * Finds the table data min and max + * Finds the table data min and max. Returns undefined when no min/max is found * @param layerId * @param tables * @param columnId diff --git a/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts b/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts index f61c7ef1db0a6..d33604ea4508d 100644 --- a/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts +++ b/x-pack/plugins/lens/public/xy_visualization/axes_configuration.ts @@ -9,7 +9,11 @@ import { AxisExtentConfig } from '@kbn/expression-xy-plugin/common'; import { Datatable } from '@kbn/expressions-plugin/public'; import type { IFieldFormat, SerializedFieldFormat } from '@kbn/field-formats-plugin/common'; import { FormatFactory } from '../../common'; -import { validateAxisDomain, validateZeroInclusivityExtent } from '../shared_components'; +import { + getDataBounds, + validateAxisDomain, + validateZeroInclusivityExtent, +} from '../shared_components'; import { XYDataLayerConfig } from './types'; interface FormattedMetric { @@ -35,14 +39,11 @@ export function isFormatterCompatible( export function getXDomain(layers: XYDataLayerConfig[] = [], tables?: Record) { const dataBounds = layers.reduce( (bounds, layer) => { - const table = tables?.[layer.layerId]; - if (layer.xAccessor && table) { - const sortedRows = table.rows - .map(({ [layer.xAccessor!]: xValue }) => xValue) - .sort((a, b) => a - b); + const tableBounds = getDataBounds(layer.layerId, tables, layer.xAccessor); + if (tableBounds) { return { - min: Math.min(bounds.min, sortedRows[0]), - max: Math.max(bounds.max, sortedRows[sortedRows.length - 1]), + min: Math.min(bounds.min, tableBounds.min), + max: Math.max(bounds.max, tableBounds.max), }; } return bounds; diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx index 96f8ac34d85c1..3b862ddd8657f 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/index.tsx @@ -14,14 +14,14 @@ import { LegendSize } from '@kbn/visualizations-plugin/public'; import type { VisualizationToolbarProps, FramePublicAPI } from '../../types'; import { State, XYState } from '../types'; import { isHorizontalChart } from '../state_helpers'; -import { LegendSettingsPopover } from '../../shared_components'; +import { hasNumericHistogramDimension, LegendSettingsPopover } from '../../shared_components'; import { AxisSettingsPopover } from './axis_settings_popover'; import { getAxesConfiguration, getXDomain, GroupsConfiguration } from '../axes_configuration'; import { VisualOptionsPopover } from './visual_options_popover'; import { getScaleType } from '../to_expression'; import { TooltipWrapper } from '../../shared_components'; import { getDefaultVisualValuesForLayer } from '../../shared_components/datasource_default_values'; -import { checkScaleOperation, getDataLayers } from '../visualization_helpers'; +import { getDataLayers } from '../visualization_helpers'; import { LegendSettingsPopoverProps } from '../../shared_components/legend_settings_popover'; type UnwrapArray = T extends Array ? P : T; @@ -299,8 +299,8 @@ export const XyToolbar = memo(function XyToolbar( } ); - const hasNumberHistogram = dataLayers.some( - checkScaleOperation('interval', 'number', props.frame.datasourceLayers) + const hasNumberHistogram = dataLayers.some(({ layerId, xAccessor }) => + hasNumericHistogramDimension(props.frame.datasourceLayers[layerId], xAccessor) ); // Ask the datasource if it has a say about default truncation value From 6a0e634532f3862e67b0178d14e8c5e1c67b8f71 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 9 Jun 2022 14:38:52 +0200 Subject: [PATCH 3/5] :camera_flash: Update snapshot --- .../shared_components/axis_extent/to_expression.tsx | 1 - .../__snapshots__/to_expression.test.ts.snap | 8 ++------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx b/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx index e16dbb08a6b8d..11535ec72a808 100644 --- a/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx +++ b/x-pack/plugins/lens/public/shared_components/axis_extent/to_expression.tsx @@ -11,7 +11,6 @@ import { UnifiedAxisExtentConfig } from './types'; // TODO: import it from the expression config directly? const CHART_TO_FN_NAME = { xy: 'axisExtentConfig', - heatmap: 'heatmap_axis_extent', } as const; export const axisExtentConfigToExpression = ( diff --git a/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap b/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap index 9ace9ea62ed9e..5ccb948dd88f9 100644 --- a/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap +++ b/x-pack/plugins/lens/public/xy_visualization/__snapshots__/to_expression.test.ts.snap @@ -216,9 +216,7 @@ Object { Object { "arguments": Object { "lowerBound": Array [], - "mode": Array [ - "full", - ], + "mode": Array [], "upperBound": Array [], }, "function": "axisExtentConfig", @@ -237,9 +235,7 @@ Object { Object { "arguments": Object { "lowerBound": Array [], - "mode": Array [ - "full", - ], + "mode": Array [], "upperBound": Array [], }, "function": "axisExtentConfig", From 5a62a8596c7be093d71ffaf8b13f4f9dd12a0197 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 9 Jun 2022 15:54:10 +0200 Subject: [PATCH 4/5] :white_check_mark: Fix expression tests --- .../expression_xy/public/components/x_domain.tsx | 2 +- .../expression_xy/public/components/xy_chart.test.tsx | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx b/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx index 42d66570e8fe6..c23c90ed91ef3 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/components/x_domain.tsx @@ -60,7 +60,7 @@ export const getXDomain = ( : undefined; if (isHistogram && isFullyQualified(baseDomain)) { - if (xExtent) { + if (xExtent && !isTimeViz) { return { extendedDomain: { min: xExtent.lowerBound ?? NaN, 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 a19132ba8193e..b488de0b703ce 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 @@ -506,6 +506,12 @@ describe('XYChart component', () => { {...defaultProps} args={{ ...args, + layers: [ + { + ...(args.layers[0] as DataLayerConfig), + isHistogram: true, + }, + ], xExtent: { type: 'axisExtentConfig', mode: 'custom', @@ -518,6 +524,7 @@ describe('XYChart component', () => { expect(component.find(Settings).prop('xDomain')).toEqual({ min: 123, max: 456, + minInterval: 50, }); } }); From e1db13b7c879e7fd483b493721a713648cbefa13 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 15 Jun 2022 11:10:08 +0200 Subject: [PATCH 5/5] :bug: Fix validation issue --- .../xy_config_panel/axis_settings_popover.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx index 28544cb12f0ba..114718b3bf7b7 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx @@ -236,9 +236,13 @@ export const AxisSettingsPopover: React.FunctionComponent { - const { inclusiveZeroError, boundaryError } = validateExtent(hasBarOrAreaOnAxis, newExtent); - if (setExtent && newExtent && !boundaryError && !isEqual(newExtent, extent)) { - if (axis === 'x' || !inclusiveZeroError) { + if (setExtent && newExtent && !isEqual(newExtent, extent)) { + const { inclusiveZeroError, boundaryError } = validateExtent(hasBarOrAreaOnAxis, newExtent); + if ( + axis === 'x' || + newExtent.mode !== 'custom' || + (!boundaryError && !inclusiveZeroError) + ) { setExtent(newExtent); } }