From 6ba1be120b371c60dc96342e01cbdca13408308b Mon Sep 17 00:00:00 2001 From: PavithraCP Date: Thu, 24 Sep 2020 23:27:35 -0400 Subject: [PATCH 1/4] Do not enable histogram mode for multiple layers --- .../xy_visualization/expression.test.tsx | 22 +++++++++++++++++++ .../public/xy_visualization/expression.tsx | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx index 3bd6cc73d6320a..abce1c39109ca2 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx @@ -914,6 +914,28 @@ describe('xy_expression', () => { expect(component.find(BarSeries).at(1).prop('enableHistogramMode')).toEqual(true); }); + test('it does not apply histogram mode to more than one the series', () => { + const { data, args } = sampleArgs(); + const firstLayer: LayerArgs = { ...args.layers[0], seriesType: 'bar', isHistogram: true }; + delete firstLayer.splitAccessor; + const secondLayer: LayerArgs = { ...args.layers[0], seriesType: 'bar', isHistogram: true }; + delete secondLayer.splitAccessor; + const component = shallow( + + ); + expect(component.find(BarSeries).at(0).prop('enableHistogramMode')).toEqual(false); + expect(component.find(BarSeries).at(1).prop('enableHistogramMode')).toEqual(false); + }); + test('it applies histogram mode to the series for stacked series', () => { const { data, args } = sampleArgs(); const component = shallow( diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.tsx index 64e0a3670a9aae..5703f60dc93eaf 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.tsx @@ -586,7 +586,7 @@ export function XYChart({ groupId: yAxesConfiguration.find((axisConfiguration) => axisConfiguration.series.find((currentSeries) => currentSeries.accessor === accessor) )?.groupId, - enableHistogramMode: isHistogram && (seriesType.includes('stacked') || !splitAccessor), + enableHistogramMode: !chartHasMoreThanOneSeries && isHistogram && (seriesType.includes('stacked') || !splitAccessor), stackMode: seriesType.includes('percentage') ? StackMode.Percentage : undefined, timeZone, areaSeriesStyle: { From 9b0682412a6522db6c7a08ffaae0a88cb783f6d9 Mon Sep 17 00:00:00 2001 From: PavithraCP Date: Mon, 28 Sep 2020 21:16:03 -0400 Subject: [PATCH 2/4] Modify logic to disable histogram mode for unstacked bar chart --- .../xy_visualization/expression.test.tsx | 32 +++++++++++++++++-- .../public/xy_visualization/expression.tsx | 7 +++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx index abce1c39109ca2..55530d5afdc75d 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx @@ -896,7 +896,12 @@ describe('xy_expression', () => { test('it applies histogram mode to the series for single series', () => { const { data, args } = sampleArgs(); - const firstLayer: LayerArgs = { ...args.layers[0], seriesType: 'bar', isHistogram: true }; + const firstLayer: LayerArgs = { + ...args.layers[0], + accessors: ['b'], + seriesType: 'bar', + isHistogram: true, + }; delete firstLayer.splitAccessor; const component = shallow( { /> ); expect(component.find(BarSeries).at(0).prop('enableHistogramMode')).toEqual(true); - expect(component.find(BarSeries).at(1).prop('enableHistogramMode')).toEqual(true); }); - test('it does not apply histogram mode to more than one the series', () => { + test('it does not apply histogram mode to more than one the series for unstacked bar chart', () => { const { data, args } = sampleArgs(); const firstLayer: LayerArgs = { ...args.layers[0], seriesType: 'bar', isHistogram: true }; delete firstLayer.splitAccessor; @@ -936,6 +940,28 @@ describe('xy_expression', () => { expect(component.find(BarSeries).at(1).prop('enableHistogramMode')).toEqual(false); }); + test('it applies histogram mode to more than one the series for unstacked line/area chart', () => { + const { data, args } = sampleArgs(); + const firstLayer: LayerArgs = { ...args.layers[0], seriesType: 'line', isHistogram: true }; + delete firstLayer.splitAccessor; + const secondLayer: LayerArgs = { ...args.layers[0], seriesType: 'line', isHistogram: true }; + delete secondLayer.splitAccessor; + const component = shallow( + + ); + expect(component.find(LineSeries).at(0).prop('enableHistogramMode')).toEqual(true); + expect(component.find(LineSeries).at(1).prop('enableHistogramMode')).toEqual(true); + }); + test('it applies histogram mode to the series for stacked series', () => { const { data, args } = sampleArgs(); const component = shallow( diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.tsx index 5703f60dc93eaf..8c86307b790d5d 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.tsx @@ -586,7 +586,12 @@ export function XYChart({ groupId: yAxesConfiguration.find((axisConfiguration) => axisConfiguration.series.find((currentSeries) => currentSeries.accessor === accessor) )?.groupId, - enableHistogramMode: !chartHasMoreThanOneSeries && isHistogram && (seriesType.includes('stacked') || !splitAccessor), + enableHistogramMode: + isHistogram && + (seriesType.includes('stacked') || !splitAccessor) && + (!seriesType.includes('bar') || + seriesType.includes('stacked') || + !chartHasMoreThanOneSeries), stackMode: seriesType.includes('percentage') ? StackMode.Percentage : undefined, timeZone, areaSeriesStyle: { From 6743b8b1dade6635c2830575d28303b3df8bd4f9 Mon Sep 17 00:00:00 2001 From: PavithraCP Date: Thu, 1 Oct 2020 02:37:38 -0400 Subject: [PATCH 3/4] Replace chartHasMoreThanOneSeries with chartHasMoreThanOneBarSeries in enableHistogram mode logic --- .../lens/public/xy_visualization/expression.test.tsx | 6 ++---- .../lens/public/xy_visualization/expression.tsx | 11 ++++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx index 55530d5afdc75d..5fc89d831a961c 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.test.tsx @@ -918,16 +918,14 @@ describe('xy_expression', () => { expect(component.find(BarSeries).at(0).prop('enableHistogramMode')).toEqual(true); }); - test('it does not apply histogram mode to more than one the series for unstacked bar chart', () => { + test('it does not apply histogram mode to more than one bar series for unstacked bar chart', () => { const { data, args } = sampleArgs(); const firstLayer: LayerArgs = { ...args.layers[0], seriesType: 'bar', isHistogram: true }; delete firstLayer.splitAccessor; - const secondLayer: LayerArgs = { ...args.layers[0], seriesType: 'bar', isHistogram: true }; - delete secondLayer.splitAccessor; const component = shallow( layer.seriesType.includes('bar')) + .reduce((sum, layer) => sum + layer.accessors.length, 0) > 1; + function calculateMinInterval() { // check all the tables to see if all of the rows have the same timestamp // that would mean that chart will draw a single bar @@ -589,9 +594,9 @@ export function XYChart({ enableHistogramMode: isHistogram && (seriesType.includes('stacked') || !splitAccessor) && - (!seriesType.includes('bar') || - seriesType.includes('stacked') || - !chartHasMoreThanOneSeries), + (seriesType.includes('stacked') || + !seriesType.includes('bar') || + !chartHasMoreThanOneBarSeries), stackMode: seriesType.includes('percentage') ? StackMode.Percentage : undefined, timeZone, areaSeriesStyle: { From ae31d2c2d4210c557aa2c2d02c5a94421a1490f9 Mon Sep 17 00:00:00 2001 From: PavithraCP Date: Thu, 1 Oct 2020 02:54:39 -0400 Subject: [PATCH 4/4] Modify chartHasMoreThanOneBarSeries logic to match ChartHasMoreThanOneSeries logic --- .../plugins/lens/public/xy_visualization/expression.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.tsx index 93eaaeb6c4c967..d20a5c52579700 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.tsx @@ -286,10 +286,12 @@ export function XYChart({ yRight: true, }; + const filteredBarLayers = filteredLayers.filter((layer) => layer.seriesType.includes('bar')); + const chartHasMoreThanOneBarSeries = - filteredLayers - .filter((layer) => layer.seriesType.includes('bar')) - .reduce((sum, layer) => sum + layer.accessors.length, 0) > 1; + filteredBarLayers.length > 1 || + filteredBarLayers.some((layer) => layer.accessors.length > 1) || + filteredBarLayers.some((layer) => layer.splitAccessor); function calculateMinInterval() { // check all the tables to see if all of the rows have the same timestamp