From 23634a664ea06a506e3627e091af405b8acfb177 Mon Sep 17 00:00:00 2001 From: Uladzislau Lasitsa Date: Fri, 25 Sep 2020 17:00:08 +0300 Subject: [PATCH 1/4] Fixed useEffect in metric_axes. Update type in params instead of type of vis --- .../public/components/options/metrics_axes/index.tsx | 4 ++-- .../public/components/options/point_series/point_series.tsx | 2 +- src/plugins/vis_type_vislib/public/types.ts | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx index d885f8fb0b12f5..8972c979ebdb90 100644 --- a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx +++ b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx @@ -299,8 +299,8 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) }, [stateParams.seriesParams]); useEffect(() => { - vis.setState({ ...vis.serialize(), type: visType }); - }, [vis, visType]); + setValue('type', visType); + }, [setValue, visType]); return isTabSelected ? ( <> diff --git a/src/plugins/vis_type_vislib/public/components/options/point_series/point_series.tsx b/src/plugins/vis_type_vislib/public/components/options/point_series/point_series.tsx index a27d0be8018c98..46e73c72b7e22a 100644 --- a/src/plugins/vis_type_vislib/public/components/options/point_series/point_series.tsx +++ b/src/plugins/vis_type_vislib/public/components/options/point_series/point_series.tsx @@ -68,7 +68,7 @@ function PointSeriesOptions(props: ValidationVisOptionsProps) /> )} - {vis.type.name === ChartTypes.HISTOGRAM && ( + {stateParams.type === ChartTypes.HISTOGRAM && ( Date: Fri, 25 Sep 2020 17:55:56 +0300 Subject: [PATCH 2/4] Fixed tests --- .../public/components/options/metrics_axes/index.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx index 524792d1460fe3..ce56cbc432618d 100644 --- a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx +++ b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx @@ -146,7 +146,7 @@ describe('MetricsAxisOptions component', () => { }, }); - expect(defaultProps.vis.setState).toHaveBeenLastCalledWith({ type: ChartTypes.LINE }); + expect(setValue).toHaveBeenLastCalledWith('type', ChartTypes.LINE); }); it('should set histogram visType when multiple seriesParam', () => { @@ -160,7 +160,7 @@ describe('MetricsAxisOptions component', () => { }, }); - expect(defaultProps.vis.setState).toHaveBeenLastCalledWith({ type: ChartTypes.HISTOGRAM }); + expect(setValue).toHaveBeenLastCalledWith('type', ChartTypes.HISTOGRAM); }); }); @@ -193,7 +193,7 @@ describe('MetricsAxisOptions component', () => { const updatedSeriesParams = [{ ...chart, data: { ...chart.data, label: agg.makeLabel() } }]; const updatedValues = [{ ...axis, title: { text: agg.makeLabel() } }]; - expect(setValue).toHaveBeenCalledTimes(5); + expect(setValue).toHaveBeenCalledTimes(6); expect(setValue).toHaveBeenNthCalledWith(3, SERIES_PARAMS, updatedSeriesParams); expect(setValue).toHaveBeenNthCalledWith(5, SERIES_PARAMS, updatedSeriesParams); expect(setValue).toHaveBeenNthCalledWith(4, VALUE_AXES, updatedValues); From aee2b2e20ebf4e468632c966bd5b1535c0f296f1 Mon Sep 17 00:00:00 2001 From: Uladzislau Lasitsa Date: Mon, 28 Sep 2020 15:48:32 +0300 Subject: [PATCH 3/4] Fixed tests --- .../public/components/options/metrics_axes/index.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx index ce56cbc432618d..dd794884308ffa 100644 --- a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx +++ b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx @@ -194,9 +194,9 @@ describe('MetricsAxisOptions component', () => { const updatedValues = [{ ...axis, title: { text: agg.makeLabel() } }]; expect(setValue).toHaveBeenCalledTimes(6); - expect(setValue).toHaveBeenNthCalledWith(3, SERIES_PARAMS, updatedSeriesParams); - expect(setValue).toHaveBeenNthCalledWith(5, SERIES_PARAMS, updatedSeriesParams); - expect(setValue).toHaveBeenNthCalledWith(4, VALUE_AXES, updatedValues); + expect(setValue).toHaveBeenNthCalledWith(4, SERIES_PARAMS, updatedSeriesParams); + expect(setValue).toHaveBeenNthCalledWith(6, SERIES_PARAMS, updatedSeriesParams); + expect(setValue).toHaveBeenNthCalledWith(5, VALUE_AXES, updatedValues); }); it('should not set the custom title to match the value axis label when more than one agg exists for that axis', () => { From 7d95bf631c46846e8a1ef0d8f65ef3f8faf76ece Mon Sep 17 00:00:00 2001 From: sulemanof Date: Thu, 1 Oct 2020 18:09:27 +0300 Subject: [PATCH 4/4] Update current chart type definition --- .../options/metrics_axes/index.test.tsx | 36 +++---------------- .../components/options/metrics_axes/index.tsx | 11 +----- .../options/point_series/point_series.tsx | 9 +++-- 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx index dd794884308ffa..0cc737f19e5c6c 100644 --- a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx +++ b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx @@ -134,34 +134,6 @@ describe('MetricsAxisOptions component', () => { const updatedSeries = [{ ...chart, data: { id: agg.id, label: agg.makeLabel() } }]; expect(setValue).toHaveBeenCalledWith(SERIES_PARAMS, updatedSeries); }); - - it('should update visType when one seriesParam', () => { - const comp = mount(); - expect(defaultProps.vis.type.type).toBe(ChartTypes.AREA); - - comp.setProps({ - stateParams: { - ...defaultProps.stateParams, - seriesParams: [{ ...chart, type: ChartTypes.LINE }], - }, - }); - - expect(setValue).toHaveBeenLastCalledWith('type', ChartTypes.LINE); - }); - - it('should set histogram visType when multiple seriesParam', () => { - const comp = mount(); - expect(defaultProps.vis.type.type).toBe(ChartTypes.AREA); - - comp.setProps({ - stateParams: { - ...defaultProps.stateParams, - seriesParams: [chart, { ...chart, type: ChartTypes.LINE }], - }, - }); - - expect(setValue).toHaveBeenLastCalledWith('type', ChartTypes.HISTOGRAM); - }); }); describe('updateAxisTitle', () => { @@ -193,10 +165,10 @@ describe('MetricsAxisOptions component', () => { const updatedSeriesParams = [{ ...chart, data: { ...chart.data, label: agg.makeLabel() } }]; const updatedValues = [{ ...axis, title: { text: agg.makeLabel() } }]; - expect(setValue).toHaveBeenCalledTimes(6); - expect(setValue).toHaveBeenNthCalledWith(4, SERIES_PARAMS, updatedSeriesParams); - expect(setValue).toHaveBeenNthCalledWith(6, SERIES_PARAMS, updatedSeriesParams); - expect(setValue).toHaveBeenNthCalledWith(5, VALUE_AXES, updatedValues); + expect(setValue).toHaveBeenCalledTimes(5); + expect(setValue).toHaveBeenNthCalledWith(3, SERIES_PARAMS, updatedSeriesParams); + expect(setValue).toHaveBeenNthCalledWith(5, SERIES_PARAMS, updatedSeriesParams); + expect(setValue).toHaveBeenNthCalledWith(4, VALUE_AXES, updatedValues); }); it('should not set the custom title to match the value axis label when more than one agg exists for that axis', () => { diff --git a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx index 8972c979ebdb90..18687404b91147 100644 --- a/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx +++ b/src/plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx @@ -18,7 +18,7 @@ */ import React, { useState, useEffect, useCallback, useMemo } from 'react'; -import { cloneDeep, uniq, get } from 'lodash'; +import { cloneDeep, get } from 'lodash'; import { EuiSpacer } from '@elastic/eui'; import { IAggConfig } from 'src/plugins/data/public'; @@ -293,15 +293,6 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) updateAxisTitle(updatedSeries); }, [metrics, firstValueAxesId, setValue, stateParams.seriesParams, updateAxisTitle]); - const visType = useMemo(() => { - const types = uniq(stateParams.seriesParams.map(({ type }) => type)); - return types.length === 1 ? types[0] : 'histogram'; - }, [stateParams.seriesParams]); - - useEffect(() => { - setValue('type', visType); - }, [setValue, visType]); - return isTabSelected ? ( <> ) { const { stateParams, setValue, vis } = props; + const currentChartTypes = useMemo(() => uniq(stateParams.seriesParams.map(({ type }) => type)), [ + stateParams.seriesParams, + ]); + return ( <> @@ -68,7 +73,7 @@ function PointSeriesOptions(props: ValidationVisOptionsProps) /> )} - {stateParams.type === ChartTypes.HISTOGRAM && ( + {currentChartTypes.includes(ChartTypes.HISTOGRAM) && (