From e89596bb7bb52238ac5eb01bf401d59610f1173e Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 21 Sep 2020 17:36:26 -0500 Subject: [PATCH] update metrics axes tests with update position logic --- .../timeseries/__mocks__/@elastic/charts.js | 44 --- src/plugins/vis_type_xy/README.md | 2 +- .../__snapshots__/index.test.tsx.snap | 1 - .../options/metrics_axes/index.test.tsx | 290 ++++++++++++++++-- .../components/options/metrics_axes/index.tsx | 21 +- .../metrics_axes/value_axis_options.test.tsx | 50 +-- 6 files changed, 275 insertions(+), 133 deletions(-) delete mode 100644 src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/__mocks__/@elastic/charts.js diff --git a/src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/__mocks__/@elastic/charts.js b/src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/__mocks__/@elastic/charts.js deleted file mode 100644 index 19bfe685cac90..0000000000000 --- a/src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/__mocks__/@elastic/charts.js +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -export const CurveType = { - CURVE_CARDINAL: 0, - CURVE_NATURAL: 1, - CURVE_MONOTONE_X: 2, - CURVE_MONOTONE_Y: 3, - CURVE_BASIS: 4, - CURVE_CATMULL_ROM: 5, - CURVE_STEP: 6, - CURVE_STEP_AFTER: 7, - CURVE_STEP_BEFORE: 8, - LINEAR: 9, -}; - -export const ScaleType = { - Linear: 'linear', - Ordinal: 'ordinal', - Log: 'log', - Sqrt: 'sqrt', - Time: 'time', -}; - -export const BarSeries = () => null; -export const AreaSeries = () => null; - -export { LIGHT_THEME, DARK_THEME } from '@elastic/charts'; diff --git a/src/plugins/vis_type_xy/README.md b/src/plugins/vis_type_xy/README.md index 70ddb21c1e9db..039ec2396e099 100644 --- a/src/plugins/vis_type_xy/README.md +++ b/src/plugins/vis_type_xy/README.md @@ -1,2 +1,2 @@ Contains the new xy-axis chart using the elastic-charts library, which will eventually -replace the vislib xy-axis (bar, area, line) charts. \ No newline at end of file +replace the vislib xy-axis charts including bar, area and line. diff --git a/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/__snapshots__/index.test.tsx.snap b/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/__snapshots__/index.test.tsx.snap index 09e0753d592e5..c5ce09d4d78af 100644 --- a/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/__snapshots__/index.test.tsx.snap +++ b/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/__snapshots__/index.test.tsx.snap @@ -74,7 +74,6 @@ exports[`MetricsAxisOptions component should init with the default set of props /> ({ SeriesPanel: () => 'SeriesPanel', @@ -103,22 +104,24 @@ describe('MetricsAxisOptions component', () => { valueAxes: [axis], seriesParams: [chart], categoryAxes: [categoryAxis], - grid: { valueAxis: defaultValueAxisId }, + grid: { + valueAxis: defaultValueAxisId, + }, }, setValue, } as any; }); it('should init with the default set of props', () => { - const comp = shallow(); + const component = shallow(); - expect(comp).toMatchSnapshot(); + expect(component).toMatchSnapshot(); }); describe('useEffect', () => { it('should update series when new agg is added', () => { - const comp = mount(); - comp.setProps({ + const component = mount(); + component.setProps({ aggs: createAggs([aggCount, aggAverage]), }); @@ -127,9 +130,9 @@ describe('MetricsAxisOptions component', () => { }); it('should update series when new agg label is changed', () => { - const comp = mount(); + const component = mount(); const agg = { id: aggCount.id, makeLabel: () => 'New label' }; - comp.setProps({ + component.setProps({ aggs: createAggs([agg]), }); @@ -138,10 +141,10 @@ describe('MetricsAxisOptions component', () => { }); it('should update visType when one seriesParam', () => { - const comp = mount(); + const component = mount(); expect(defaultProps.vis.type.type).toBe(ChartType.Area); - comp.setProps({ + component.setProps({ stateParams: { ...defaultProps.stateParams, seriesParams: [{ ...chart, type: ChartType.Line }], @@ -152,10 +155,10 @@ describe('MetricsAxisOptions component', () => { }); it('should set histogram visType when multiple seriesParam', () => { - const comp = mount(); + const component = mount(); expect(defaultProps.vis.type.type).toBe(ChartType.Area); - comp.setProps({ + component.setProps({ stateParams: { ...defaultProps.stateParams, seriesParams: [chart, { ...chart, type: ChartType.Line }], @@ -169,12 +172,12 @@ describe('MetricsAxisOptions component', () => { describe('updateAxisTitle', () => { it('should not update the value axis title if custom title was set', () => { defaultProps.stateParams.valueAxes[0].title.text = 'Custom title'; - const comp = mount(); + const component = mount(); const newAgg = { ...aggCount, makeLabel: () => 'Custom label', }; - comp.setProps({ + component.setProps({ aggs: createAggs([newAgg]), }); const updatedValues = [{ ...axis, title: { text: newAgg.makeLabel() } }]; @@ -182,13 +185,13 @@ describe('MetricsAxisOptions component', () => { }); it('should set the custom title to match the value axis label when only one agg exists for that axis', () => { - const comp = mount(); + const component = mount(); const agg = { id: aggCount.id, params: { customLabel: 'Custom label' }, makeLabel: () => 'Custom label', }; - comp.setProps({ + component.setProps({ aggs: createAggs([agg]), }); @@ -202,9 +205,9 @@ describe('MetricsAxisOptions component', () => { }); it('should not set the custom title to match the value axis label when more than one agg exists for that axis', () => { - const comp = mount(); + const component = mount(); const agg = { id: aggCount.id, makeLabel: () => 'Custom label' }; - comp.setProps({ + component.setProps({ aggs: createAggs([agg, aggAverage]), stateParams: { ...defaultProps.stateParams, @@ -217,13 +220,13 @@ describe('MetricsAxisOptions component', () => { it('should not overwrite the custom title with the value axis label if the custom title has been changed', () => { defaultProps.stateParams.valueAxes[0].title.text = 'Custom title'; - const comp = mount(); + const component = mount(); const agg = { id: aggCount.id, params: { customLabel: 'Custom label' }, makeLabel: () => 'Custom label', }; - comp.setProps({ + component.setProps({ aggs: createAggs([agg]), }); @@ -232,8 +235,8 @@ describe('MetricsAxisOptions component', () => { }); it('should add value axis', () => { - const comp = shallow(); - comp.find(ValueAxesPanel).prop('addValueAxis')(); + const component = shallow(); + component.find(ValueAxesPanel).prop('addValueAxis')(); expect(setValue).toHaveBeenCalledWith(VALUE_AXES, [axis, axisRight]); }); @@ -244,16 +247,16 @@ describe('MetricsAxisOptions component', () => { }); it('should remove value axis', () => { - const comp = shallow(); - comp.find(ValueAxesPanel).prop('removeValueAxis')(axis); + const component = shallow(); + component.find(ValueAxesPanel).prop('removeValueAxis')(axis); expect(setValue).toHaveBeenCalledWith(VALUE_AXES, [axisRight]); }); it('should update seriesParams "valueAxis" prop', () => { const updatedSeriesParam = { ...chart, valueAxis: 'ValueAxis-2' }; - const comp = shallow(); - comp.find(ValueAxesPanel).prop('removeValueAxis')(axis); + const component = shallow(); + component.find(ValueAxesPanel).prop('removeValueAxis')(axis); expect(setValue).toHaveBeenCalledWith(SERIES_PARAMS, [updatedSeriesParam]); }); @@ -261,18 +264,241 @@ describe('MetricsAxisOptions component', () => { it('should reset grid "valueAxis" prop', () => { const updatedGrid = { valueAxis: undefined }; defaultProps.stateParams.seriesParams[0].valueAxis = 'ValueAxis-2'; - const comp = shallow(); - comp.find(ValueAxesPanel).prop('removeValueAxis')(axis); + const component = shallow(); + component.find(ValueAxesPanel).prop('removeValueAxis')(axis); expect(setValue).toHaveBeenCalledWith('grid', updatedGrid); }); }); - it('should update axis value when when category position chnaged', () => { - const comp = shallow(); - comp.find(CategoryAxisPanel).prop('onPositionChanged')(Position.Left); + describe('onValueAxisPositionChanged', () => { + const getProps = ( + valuePosition1: Position = Position.Right, + valuePosition2: Position = Position.Left + ): ValidationVisOptionsProps => ({ + ...defaultProps, + stateParams: { + ...defaultProps.stateParams, + valueAxes: [ + { + ...valueAxis, + id: 'ValueAxis-1', + position: valuePosition1, + }, + { + ...valueAxis, + id: 'ValueAxis-2', + position: valuePosition2, + }, + { + ...valueAxis, + id: 'ValueAxis-3', + position: valuePosition2, + }, + ], + categoryAxes: [ + { + ...categoryAxis, + position: mapPosition(valuePosition1), + }, + ], + }, + }); + + it('should update all value axes if another value axis changes from horizontal to vertical', () => { + const component = mount(); + setValue.mockClear(); + const onValueAxisPositionChanged = component + .find(ValueAxesPanel) + .prop('onValueAxisPositionChanged'); + onValueAxisPositionChanged(0, Position.Bottom); + expect(setValue).nthCalledWith(1, 'categoryAxes', [ + expect.objectContaining({ + id: 'CategoryAxis-1', + position: Position.Right, + }), + ]); + expect(setValue).nthCalledWith(2, 'valueAxes', [ + expect.objectContaining({ + id: 'ValueAxis-1', + position: Position.Bottom, + }), + expect.objectContaining({ + id: 'ValueAxis-2', + position: Position.Top, + }), + expect.objectContaining({ + id: 'ValueAxis-3', + position: Position.Top, + }), + ]); + }); + + it('should update all value axes if another value axis changes from vertical to horizontal', () => { + const component = mount(); + setValue.mockClear(); + const onValueAxisPositionChanged = component + .find(ValueAxesPanel) + .prop('onValueAxisPositionChanged'); + onValueAxisPositionChanged(1, Position.Left); + expect(setValue).nthCalledWith(1, 'categoryAxes', [ + expect.objectContaining({ + id: 'CategoryAxis-1', + position: Position.Top, + }), + ]); + expect(setValue).nthCalledWith(2, 'valueAxes', [ + expect.objectContaining({ + id: 'ValueAxis-1', + position: Position.Right, + }), + expect.objectContaining({ + id: 'ValueAxis-2', + position: Position.Left, + }), + expect.objectContaining({ + id: 'ValueAxis-3', + position: Position.Left, + }), + ]); + }); + + it('should update only changed value axis if value axis stays horizontal', () => { + const component = mount(); + setValue.mockClear(); + const onValueAxisPositionChanged = component + .find(ValueAxesPanel) + .prop('onValueAxisPositionChanged'); + onValueAxisPositionChanged(0, Position.Left); + expect(setValue).nthCalledWith(1, 'valueAxes', [ + expect.objectContaining({ + id: 'ValueAxis-1', + position: Position.Left, + }), + expect.objectContaining({ + id: 'ValueAxis-2', + position: Position.Left, + }), + expect.objectContaining({ + id: 'ValueAxis-3', + position: Position.Left, + }), + ]); + }); + + it('should update only changed value axis if value axis stays vertical', () => { + const component = mount(); + setValue.mockClear(); + const onValueAxisPositionChanged = component + .find(ValueAxesPanel) + .prop('onValueAxisPositionChanged'); + onValueAxisPositionChanged(1, Position.Top); + expect(setValue).nthCalledWith(1, 'valueAxes', [ + expect.objectContaining({ + id: 'ValueAxis-1', + position: Position.Top, + }), + expect.objectContaining({ + id: 'ValueAxis-2', + position: Position.Top, + }), + expect.objectContaining({ + id: 'ValueAxis-3', + position: Position.Bottom, + }), + ]); + }); + }); + + describe('onCategoryAxisPositionChanged', () => { + const getProps = ( + position: Position = Position.Bottom + ): ValidationVisOptionsProps => ({ + ...defaultProps, + stateParams: { + ...defaultProps.stateParams, + valueAxes: [ + { + ...valueAxis, + id: 'ValueAxis-1', + position: mapPosition(position), + }, + { + ...valueAxis, + id: 'ValueAxis-2', + position: mapPositionOpposite(mapPosition(position)), + }, + { + ...valueAxis, + id: 'ValueAxis-3', + position: mapPosition(position), + }, + ], + categoryAxes: [ + { + ...categoryAxis, + position, + }, + ], + }, + }); + + it('should update all value axes if category axis changes from horizontal to vertical', () => { + const component = mount(); + setValue.mockClear(); + const onPositionChanged = component.find(CategoryAxisPanel).prop('onPositionChanged'); + onPositionChanged(Position.Left); + expect(setValue).nthCalledWith(1, 'valueAxes', [ + expect.objectContaining({ + id: 'ValueAxis-1', + position: Position.Bottom, + }), + expect.objectContaining({ + id: 'ValueAxis-2', + position: Position.Top, + }), + expect.objectContaining({ + id: 'ValueAxis-3', + position: Position.Bottom, + }), + ]); + }); + + it('should update all value axes if category axis changes from vertical to horizontal', () => { + const component = mount(); + setValue.mockClear(); + const onPositionChanged = component.find(CategoryAxisPanel).prop('onPositionChanged'); + onPositionChanged(Position.Top); + expect(setValue).nthCalledWith(1, 'valueAxes', [ + expect.objectContaining({ + id: 'ValueAxis-1', + position: Position.Left, + }), + expect.objectContaining({ + id: 'ValueAxis-2', + position: Position.Right, + }), + expect.objectContaining({ + id: 'ValueAxis-3', + position: Position.Left, + }), + ]); + }); - const updatedValues = [{ ...axis, name: 'BottomAxis-1', position: Position.Bottom }]; - expect(setValue).toHaveBeenCalledWith(VALUE_AXES, updatedValues); + it('should not update value axes if category axis stays horizontal', () => { + const component = mount(); + setValue.mockClear(); + const onPositionChanged = component.find(CategoryAxisPanel).prop('onPositionChanged'); + onPositionChanged(Position.Top); + expect(setValue).not.toBeCalled(); + }); + + it('should not update value axes if category axis stays vertical', () => { + const component = mount(); + setValue.mockClear(); + const onPositionChanged = component.find(CategoryAxisPanel).prop('onPositionChanged'); + onPositionChanged(Position.Right); + expect(setValue).not.toBeCalled(); + }); }); }); diff --git a/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/index.tsx b/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/index.tsx index b58090ab2ccb3..7836066728de0 100644 --- a/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/index.tsx +++ b/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/index.tsx @@ -182,7 +182,9 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) { ...categoryAxes, position: mapPosition(categoryAxes.position), }; + setValue('categoryAxes', [updatedCategoryAxes]); + const oldPosition = valueAxes[index].position; const newValueAxes = valueAxes.map(({ position, ...axis }, i) => ({ ...axis, @@ -208,14 +210,19 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) { (axisPosition: CategoryAxis['position']) => { const isHorizontalAxis = isAxisHorizontal(axisPosition); - stateParams.valueAxes.forEach((axis, index) => { - if (isAxisHorizontal(axis.position) === isHorizontalAxis) { - const position = mapPosition(axis.position); - onValueAxisPositionChanged(index, position); - } - }); + if ( + stateParams.valueAxes.some( + ({ position }) => isAxisHorizontal(position) === isHorizontalAxis + ) + ) { + const newValueAxes = stateParams.valueAxes.map(({ position, ...axis }) => ({ + ...axis, + position: mapPosition(position), + })); + setValue('valueAxes', newValueAxes); + } }, - [stateParams.valueAxes, onValueAxisPositionChanged] + [setValue, stateParams.valueAxes] ); const addValueAxis = useCallback(() => { diff --git a/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/value_axis_options.test.tsx b/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/value_axis_options.test.tsx index 67d9a296c712e..0b325198c3fe7 100644 --- a/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/value_axis_options.test.tsx +++ b/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/value_axis_options.test.tsx @@ -20,22 +20,17 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { Position } from '@elastic/charts'; + import { TextInputOption } from '../../../../../../charts/public'; import { ValueAxis, ScaleType } from '../../../../types'; import { LabelOptions } from './label_options'; import { ValueAxisOptions, ValueAxisOptionsParams } from './value_axis_options'; import { valueAxis, vis } from './mocks'; -import { Position } from '@elastic/charts'; const POSITION = 'position'; -interface PositionOption { - text: string; - value: Position; - disabled: boolean; -} - describe('ValueAxisOptions component', () => { let setParamByIndex: jest.Mock; let onValueAxisPositionChanged: jest.Mock; @@ -74,47 +69,6 @@ describe('ValueAxisOptions component', () => { expect(comp.find(LabelOptions).exists()).toBeFalsy(); }); - it.todo('replace test with new logic'); - xit('should only allow left and right value axis position when category axis is horizontal', () => { - const comp = shallow(); - - const options: PositionOption[] = comp.find({ paramName: POSITION }).prop('options'); - - expect(options.length).toBe(4); - options.forEach(({ value, disabled }) => { - switch (value) { - case Position.Left: - case Position.Right: - expect(disabled).toBeFalsy(); - break; - case Position.Top: - case Position.Bottom: - expect(disabled).toBeTruthy(); - break; - } - }); - }); - - xit('should only allow top and bottom value axis position when category axis is vertical', () => { - const comp = shallow(); - - const options: PositionOption[] = comp.find({ paramName: POSITION }).prop('options'); - - expect(options.length).toBe(4); - options.forEach(({ value, disabled }) => { - switch (value) { - case Position.Left: - case Position.Right: - expect(disabled).toBeTruthy(); - break; - case Position.Top: - case Position.Bottom: - expect(disabled).toBeFalsy(); - break; - } - }); - }); - it('should call onValueAxisPositionChanged when position is changed', () => { const value = Position.Right; const comp = shallow();