From c1b59f249a1fdfc5d6f714de8db99cbf7a16c6eb Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 15 Apr 2021 11:14:39 +0200 Subject: [PATCH] fix(xy): consider `useDefaultGroupDomain` on scale config (#1119) The #1087 PR introduced a regression where the useDefaultGroupDomain was not considered when computing the scale configs. --- ...p-domain-visually-looks-correct-1-snap.png | Bin 0 -> 4966 bytes .../selectors/get_api_scale_configs.test.ts | 94 ++++++++++++++++++ .../state/selectors/get_api_scale_configs.ts | 6 +- stories/bar/56_test_use_dfl_gdomain.tsx | 52 ++++++++++ stories/bar/bars.stories.tsx | 1 + 5 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-test-use-default-group-domain-visually-looks-correct-1-snap.png create mode 100644 src/chart_types/xy_chart/state/selectors/get_api_scale_configs.test.ts create mode 100644 stories/bar/56_test_use_dfl_gdomain.tsx diff --git a/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-test-use-default-group-domain-visually-looks-correct-1-snap.png b/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-test-use-default-group-domain-visually-looks-correct-1-snap.png new file mode 100644 index 0000000000000000000000000000000000000000..ffeffd75c05b68556e5c5c25bbd4f8c211199634 GIT binary patch literal 4966 zcmd^@X;jl!8pi(?l@^LIZXgh-pkggxv0zY0un+-P1Q97)DzShBL=a3&AkjLq2!v9K zQ8o)hZ^mcAAQK{|U~F&+@~fe?b0gnYOhyfe}G%_Mj%Ipg8t2;J&51b?()x)ftn>LjPNmu?H z?&^boF7Cv2ywpHHdbB{6I_ zI9TL&DXyB(`4gC35VX;>X9@Jl`t%hL)E3=K{(A&!Ux1pqqpkm-)_II~^{>&lB=jn}VFN>5L(q9{pzwL~k*%thna;|G~urTDJ7xw)gv!Kz4{ z|H21Qqaf&D`eHCyI+cV~P8?&^)*2RheEQJ_VX=mbgAU#amL9kxyCpJU z$6Vj55*VwhqSh(rC6n%!bVgkr>g+4t+6I+UB*<_>C)E%{Ewi)HAFo((i^LCP$s4Ku z{{9l0HUy>RR*MR$@@EO+cI@JIaQgP*rqyDli0WWsGQY5ZJ8@#;q1;_-!^6WPpEfSJ zKJ@_v;SUoEMT6{Z(pQ7BqOw#Ptz~?C+zGY}f|!qxv=81v35oA6n)m6W2uZ-qbd9J* zE+=}H_Kk}~X}ST#4J0xd7ZkMl&~-EdsLjM=xuzyAEbQhp4GpM~bc|3KF;crT;M`c| zxKNn1pKjUg8&S75VFluHV&a7h7i?qYr|%~KRe5>HBI&ty1ZOulHw-3h@U8_=i9UJ_ z1l=hI6;c(+_*92H%DgbMmBZmgzJA`eC37Duf?q#yH+;|u_8~a@ppNv#5wfUxYAQ_K zS8#aulP9`nhjJ1FS$Y{68IjXN_iQN&VsuQ5O_((@h&kpbw-R0HJbD>k2I z>O#X12(pK7Dk>^iCppDH4vE>wh!(JrS&4*6;`cQX@(=$+@Pi=YR4+oPYw4G`uV__? z+jUxE7ww)Qiez0}Ts->AC2e=67o()atfi#|tt*C=XSren0%$%=FVO=Cn(@A2*OO+1 znlI{t!C%Ni{fRkf(6uOBLJgrM6aeF!0_0qt18xlJSzX|$(kzqsEwqB-9wVf0;$(~dBm zl9!j4m3M>ICN7J=3^b5=6D^00w9ilrY|lks+`3E4X;i7=?kH#K}E4w&T`;z z_ZyxCo?Dr<{a}Ha`^KIXYXH~0^TL%Xs*?qtJ-Z{1JbiClT%A`b^V}Ao!*76prOBs4 z+WSh62_B@Ic8`o0!Qn+7C&$M3W15DB{KEM4aed@Pp&OO-G%K{SI;wYbjiZZiZ{9F> z%If^)YFTDShtY9=^>YN9-@JBLze;mWKSGE+HI&HaQ|xGlJ68=44}aDr;#GdunTIkU z%cfH=UAlBNFOPOn69z0X>Yk#}COJ7-LL0PJaYhg-MF%xps<(T!lG7=*?Vj4upN~Ad z=%jdwY*4VPcH{LTM6{lZbX5NYZB|>?QXLbV5;qqN28+e2<@_7xyikqij&%{9tFAca zKh@KLpvi#Ybjs{_cf&kyd^{wDMnm`R20=8@uex9a+T_vuV>?0%n@{S2?Makn8lwra!0CD86tpCg4Xm^E6L;$a#988-B&t_X!f+jFl^{IIWRfE|Ga zcFIg21dOsj#Jf-?ku)c*HY@NpZ+ZUwM2;Q7l4k5+1oAL-9u6w7N`IXYy!h-(w5_e) zSk%r_{Cb4%-LS9vB4IHtz`z4#shc4|aEV!;ujqDO@f$^UY+b z_&Lbj(;W&$EC}_4^{W9cmD=@`0E<)1yt)?LjF0dbPPt65o>)e@@oNd^6; zXJC*#Jw5GW^##}n9(XL+XRVZn#}X-kfZ`Jp47RHyVLRUzAYes*khfQ4gKRp|wH z<|HBP&SF5#?#C|!iF+6%Zhd{deBxMx{w2p}XS&=D9B6v{_;A?3?F|pdqQZu&ZGcJ* zb^wpMb2&bDZ<*&-_pO;;Mlz|?8Msp~0N}^lC(zU7K9vA3CGG9)Fux;AuZGi_#t_t> z9lwWk!}IdJd-pP1&zw2Kid&E##j5B}7HukhEUIg2o>=o$Y-wkap5D1KHO}=)dNbhS z=!#ZvLUmqN%7t{_xBMsVjBsw6U#w}3-!p&9zh-obTfriTM!dbf6V)0g&OH6OOf)%3 zewqcqvs&~R*{%$=LHpH@sGV_s;cjpi>X&NIlQplDYkh{46BQjs&(i-`jB2Ivgx7kV{t*NX^%j4JM;2k8itznkBC` zJA`3*r?j=T*%GGx3>_%;)z#HrF-o)Rv9B9#WFmf=R4PqCtc4&CymuiXCWg6m4*c&Q z+Q+Km7S4uo%fC7Lq-=8!olf5aMy<(!`zCQ9U3Y}@)*ujw$Tu%KuIA>Z0)-#NVz*`* zRRvKf=+H{u{gjU&NN8jJ){y>%xe97(YTVT}*$!;-pNw^yaTkR5l`&JjU&pxj!yf+Y zRu*j?Rqx~A|oOqJhx^#hr=Mqf{`EGqw-J>j!K0>q4C7r!eS58%eRjgl&zZ02ug#jxjj8S zI5A}{yt}`D9T>j3xSJ2bxLDx=Wrcx)?hGZIHwzjqv3Wd;w8To-& z4P?19c5Ch5zrUNq5u}wY(GNL!^3yAHdW?ozoOKNi4$jWbqrEqdsQ03|!f>S-Al2oU zcqEno66?0%8UpCA`L3N1(_Eo!D0+yXe98Dq$i|BzM?>mfi zj6NdtJcFw*0AiJs*fbbg{v($G-89h9(1=vX#kP^7nAlf%Y%)OeptPlFDXs}FPkEJi zajq!V3`}+f9yhGXhUa67yh@wFTm^=n67@!bbyyF8a}Ws99B52mc1s?LJgWZM(3vh9 zvibSek=|ZU#i(vWBEZ)HaLEw6Fvve8b)9)om9atxjQo`Y6~SPBw14yH3tBB?{Q%&D zV%){y^7X~p5-WRq!-hpkSvG2w{r2fXr;GkIXD!f+a)BJ|TNe(`+_y;$J78)!_4))f z*{Y5k$rEgZRBz@%VsyxlzU))YCD$qX4s&g8UU@Y{aiAagfqDru!X|S2dC3@zDtDTd z$&loaiQw+X`mv^Q>`K181&NjofmF?Lpav9b*cTcgPc?N>7vCf%f=d{2ohYuXZS~(7 oZ|{|YU`rrKVNCrG*hZy|I!ei(*(ePFw^PvGJ%=#29KZhdA5ha~#sB~S literal 0 HcmV?d00001 diff --git a/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.test.ts b/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.test.ts new file mode 100644 index 0000000000..60c532d8ae --- /dev/null +++ b/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.test.ts @@ -0,0 +1,94 @@ +/* + * 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. + */ + +import { MockGlobalSpec, MockSeriesSpec } from '../../../../mocks/specs/specs'; +import { MockStore } from '../../../../mocks/store/store'; +import { DEFAULT_GLOBAL_ID } from '../../utils/specs'; +import { computeSeriesGeometriesSelector } from './compute_series_geometries'; +import { getScaleConfigsFromSpecsSelector } from './get_api_scale_configs'; + +describe('GroupIds and useDefaultGroupId', () => { + it('use the specified useDefaultGroupId to compute scale configs', () => { + const store = MockStore.default(); + MockStore.addSpecs( + [ + MockSeriesSpec.bar({ + groupId: 'other', + useDefaultGroupDomain: 'a different one', + }), + ], + store, + ); + const scaleConfigs = getScaleConfigsFromSpecsSelector(store.getState()); + expect(scaleConfigs.y['a different one']).toBeDefined(); + }); + + it('have 2 different y domains with 2 groups', () => { + const store = MockStore.default(); + MockStore.addSpecs( + [ + MockSeriesSpec.bar({ id: 'one' }), + MockSeriesSpec.bar({ + id: 'two', + groupId: 'other', + useDefaultGroupDomain: 'a different one', + }), + ], + store, + ); + const scaleConfigs = getScaleConfigsFromSpecsSelector(store.getState()); + expect(Object.keys(scaleConfigs.y)).toHaveLength(2); + expect(scaleConfigs.y['a different one']).toBeDefined(); + expect(scaleConfigs.y[DEFAULT_GLOBAL_ID]).toBeDefined(); + }); + + it('have 2 different y domains with 3 groups', () => { + const store = MockStore.default({ width: 120, height: 100, left: 0, top: 0 }); + MockStore.addSpecs( + [ + MockGlobalSpec.settingsNoMargins(), + MockSeriesSpec.bar({ id: 'one', data: [{ x: 1, y: 10 }] }), + MockSeriesSpec.bar({ + id: 'two', + groupId: 'other', + useDefaultGroupDomain: 'a different one', + data: [{ x: 1, y: 10 }], + }), + MockSeriesSpec.bar({ + id: 'three', + groupId: 'another again', + useDefaultGroupDomain: 'a different one', + data: [{ x: 1, y: 10 }], + }), + ], + store, + ); + const scaleConfigs = getScaleConfigsFromSpecsSelector(store.getState()); + expect(Object.keys(scaleConfigs.y)).toHaveLength(2); + expect(scaleConfigs.y['a different one']).toBeDefined(); + expect(scaleConfigs.y[DEFAULT_GLOBAL_ID]).toBeDefined(); + + const geoms = computeSeriesGeometriesSelector(store.getState()); + const { bars } = geoms.geometries; + expect(bars).toHaveLength(3); + expect(bars[0].value[0].width).toBe(40); + expect(bars[1].value[0].width).toBe(40); + expect(bars[2].value[0].width).toBe(40); + }); +}); diff --git a/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts b/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts index c66aa1077d..ecec96bd0e 100644 --- a/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts +++ b/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts @@ -33,6 +33,7 @@ import { isHorizontalAxis, isVerticalAxis } from '../../utils/axis_type_utils'; import { groupBy } from '../../utils/group_data_series'; import { AxisSpec, BasicSeriesSpec, CustomXDomain, XScaleType, YDomainRange } from '../../utils/specs'; import { isHorizontalRotation } from '../utils/common'; +import { getSpecDomainGroupId } from '../utils/spec'; import { getAxisSpecsSelector, getSeriesSpecsSelector } from './get_specs'; import { mergeYCustomDomainsByGroupId } from './merge_y_custom_domains'; @@ -83,14 +84,15 @@ export function getScaleConfigsFromSpecs( }; // y axes - const scaleConfigsByGroupId = groupBy(seriesSpecs, ['groupId'], true).reduce< + const scaleConfigsByGroupId = groupBy(seriesSpecs, getSpecDomainGroupId, true).reduce< Record >((acc, series) => { const yScaleTypes = series.map(({ yScaleType, yNice }) => ({ nice: getYNiceFromSpec(yNice), type: getYScaleTypeFromSpec(yScaleType), })); - acc[series[0].groupId] = coerceYScaleTypes(yScaleTypes); + const groupId = getSpecDomainGroupId(series[0]); + acc[groupId] = coerceYScaleTypes(yScaleTypes); return acc; }, {}); diff --git a/stories/bar/56_test_use_dfl_gdomain.tsx b/stories/bar/56_test_use_dfl_gdomain.tsx new file mode 100644 index 0000000000..0c308bc77d --- /dev/null +++ b/stories/bar/56_test_use_dfl_gdomain.tsx @@ -0,0 +1,52 @@ +/* + * 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. + */ + +import React from 'react'; + +import { Axis, BarSeries, Chart, Position, ScaleType } from '../../src'; + +export const Example = () => { + return ( + + + + + + + ); +}; diff --git a/stories/bar/bars.stories.tsx b/stories/bar/bars.stories.tsx index c492126d2d..b79a058670 100644 --- a/stories/bar/bars.stories.tsx +++ b/stories/bar/bars.stories.tsx @@ -85,3 +85,4 @@ export { Example as testMinHeightPositiveAndNegativeValues } from './46_test_min export { Example as testTooltipAndRotation } from './48_test_tooltip'; export { Example as tooltipBoundary } from './55_tooltip_boundary'; export { Example as testDualYAxis } from './49_test_dual_axis'; +export { Example as testUseDefaultGroupDomain } from './56_test_use_dfl_gdomain';