From a78fb4fafed90459f3321c2118223033049d63af Mon Sep 17 00:00:00 2001 From: Nick Partridge Date: Tue, 21 Jul 2020 14:00:15 -0500 Subject: [PATCH] fix: custom domain error with fallback ordinal scale (#757) Ignores `xDomain` prop when using a fallback ordinal scale. fixes #756 --- ...fallback-visually-looks-correct-1-snap.png | Bin 0 -> 10502 bytes .../xy_chart/domains/x_domain.test.ts | 14 ++++- .../chart_types/xy_chart/domains/x_domain.ts | 9 ++- .../stories/scales/6_x_scale_fallback.tsx | 55 ++++++++++++++++++ .../stories/scales/scales.stories.tsx | 1 + 5 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 packages/osd-charts/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-scales-x-scale-fallback-visually-looks-correct-1-snap.png create mode 100644 packages/osd-charts/stories/scales/6_x_scale_fallback.tsx diff --git a/packages/osd-charts/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-scales-x-scale-fallback-visually-looks-correct-1-snap.png b/packages/osd-charts/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-scales-x-scale-fallback-visually-looks-correct-1-snap.png new file mode 100644 index 0000000000000000000000000000000000000000..4334b70a0ef924b930183bde631a1cfc822beb82 GIT binary patch literal 10502 zcmd6N2UJwqwk_QTj2PO0L_;BB0fK^LFn~}XAUOv`P(X5$RE2Fo6h#UF1)-Fjkz_$3 zpces=)MN=oNv4QRjzzt-z5V~~`~My9`S0)l$73+YfjV{e*=O&y=9+V^6LdvGdFM8c zZ7eJ-J8>$C+AJ(T(^*)4y18XDd@}mMdJGOfxoIn3V9BcIf-{F%aEj-3Z@-$L65Ms? zW0=$JwV2EJ&zD8`j^EsP@h~QaD>UJj;*(qV^{gU}J&ZcRcvBPNF|9nBbrxIK5znJT zp1>KIwDBCMe}_G6=UJe2FFQB+oL!Td<=y)jJ>t%@Dsk+!J1(5A&7R^FY`&CV{q5fB zM_JzCfzRDVZd0plTz)gXHgA=VhCYOOv9Omr&Z8sCgQHD9!J&LN%#bDE>O*%pd=OET zhJz*hQEoVh>+3|{Bp=jum&dhnDi z(^ku>J{K?E{ZdmiA2a2rOjx)l#>eQ9Vg(9{fB!#xBqn!|5-h1W3gTe>4!CDbLlY6&pY-wvtdjDP)r+xM6n;zdO zIUKQTBBFgP++F%pLqJ*Ino$?LP7h zL#KMtT0aH-!iU?mX;WvhS2iZKq{Pg~$jDVX=H!VJSM2T6;qg~iS6ivn% zxdoa`eolWttJOEzx*%|;i1p3&9euD7E(9CO#a<0{L{Vl zv&}QZ_%&6D8Zpv|)nWW&vA&a9C-!Ok`3Xy}8v$drK6J3{- zn|s<@mIr$ZVZ=jh&9RcSw393S2hDGvH!yX%k4a5TRQhOnrM0!y&fZ=JLOw#+EbSk8 zaQE(A<2;)uGcz;KU%a^4$_In6xLDUt2;Vt(ZtDH)({0^mc9ldhGe{;OhcL$xFQDwxv)%bE&2j;q641Ix=N ze;E@+5qA6Z(I(c-o97_L=~=~-j~v~KsSwvuVz$Jip(^g1sDrN6WAm35XW_H64xJAh z=BKN8cNJ@5_6B8*pm_~P?;#hD)5@3`PV0gvPW;wW;>(Adn3(wT z#no0CEqij(ad2qp>5?g(PA|tNCM0}faG*(qp1#KCPb>AOtgnm(-ij8tkC2v;(K9hg zfBt-*p1!`SGUc}R%p*6MEX1r!jXk=?niO^Kf~0&zDoQlKA|+36Br%^`T3t6$@V?(y`$uO-$BBU zj*UTpB@PT+hjlo4>==OW*RLr_Nyj^GorSEg?w>aF+YgU;Zq1N~URPHq?K77G>ts#t z@>pAP!D*|hJ^uCMYxo3^BI&_{%|d1rU4MUnJ4Z)-J-zVFtgHn#bsPW|0-~ageOR<= zedPeUQPI&J{ZAxL9yxLzz{HVkLTu^mOpA*we;_9>dE z-%CRZ(UIIGj$gaI|G)uNB4pJ3XnM9PvD=aS-aPxbdQ^f6X0Mr%7gPyAYOC8N=IYUB z9X<_@PfhW9l3;N7Vb4aC<>9WHDI9aupe*e!>n>Fy8;@*CZS7^8Hbn2jBo&LZb8<4! z*N=Fmk-Tl&Hdu<2Wwh+|b-ag{7gj>T>(j8Bx;pnl4c)zxc>%>Uz;?6QP+fE~+>hQ~ zwq5|Zoqv1!8m=kBD6gfXBek>B7@|KVH&>hZJT^9qNZbp%T3OKS^v=0} zuA4A3%R@EC$<3_q?8g zL3(DUI`9!=iaQFHmhSEh!0erDY<6`~;y?p*u3d{$YDTCvZ9P6A{6pwl(Tqy_wAClo z71&WoEOD#K9gungkQ?$OP2os2A~2@`*CIe(^y^t1pRKN@7S=GFXNL&o;Yna3Gc&G5 zRL2-;57kqpOL3kPor&25#u$*piwmoB{k`u9u^&Hvyt=q`^W8?@0Te;Ai24AWqoLiU z^-N7O$^&<){Sny-%}4My)_}}PWHn;@*psZn5XHm0*%z?;wpA=1MVGS!8YqYGguDC9 zuS}3>`ZhL6nAF#=&-pG+hGPoIWGoh|rje`zw8YNAA)Gd4Kp>P+ii^XeqhvC%mH-(!0jfxi--wEc{Ztec{&9J1*$llmbG;vMnF!^ z>|f$^%`wLM>J+nff-Ugk{{8z&%ci!rsj6yf%5H*igAb>nQlUk*OnGt4pSjK$rPB}P z3bF6_GPPLcZ~~nWd@R*8RW!`w0MzR9yVi7ru_*hjpsp+EYQwXAzD=63vWFNhBJ9EP zY=PyY6BAFE*U>q4@jex1zf(&FkY0a(#sXS&cNlr13o=m zdTqTb7*H0!RL(mZlO%6;#-{c+5HHGBJn*SyEj<7B?b{2yXKna^ zV8UgPI*r3nWBa7J;0q!LD!AyJXK%63&=f9xFLoBDwV~?;+5Qes zcD^DdCFR16PO_)Nn8PFj{&WAGJ3mngOvXt;!QekO#fBNjs-mIM){?4I;L!E%b}h=f zU~kdv1VTS2?HRBZDxNasFHgcVS7}%|Il0Ml1YuTMwV^#dJ@|$5d-vx1EzcD0KvXo? z16D8uKNo)_B#cOW9vz*Y?Kg;Z;Y9kAy#{7c!dQxn z?f!)$LKdZ_@Zqr7he>L2cvx8c)?beA=$HhwaxKE(&PJ;PiUF3vuOnH8Vqt6?AMT$V3ZLul zHg%wtnu&{xQ~DlZ-A8^qg8nry^U$m1pPHCx&9SPABeDOo?f%?d-nDatRWbof?h?vw zPsfEF7uXx5XwYUR9QoedX^!1I+n_l~opV|$ghx&~jsdmwS}_XP!y2&jtCPi=Sc9yV zwoDVI^Rs&!Gfj(54}}d63?P!Ua=SH6&u6Yt8DLxQ%9Yn8zKbUn6fFLUowj9LWP8sJ z*;-njqf)6rBiW~=_%&Whj?05yx_%#}Uqp~p)8j{v@~@U}E?JwYk#-$;h{%P~;{&#k zi7wo?+A@qr$H#RY95P{Yx<*CX*dQt5}33EeHodJ11I}nsAZg)~(5_@=(|g9Xixm>hIefh_bda?!_^= z-CJ%tJ3D`V2bsR)*#0pQWeiwU9_)y${i)|zjCwo zJ8DWn@`4i1jEdr5G%GNgjk-{R`MJQybrQ4;5Z?eI@+*6Sp{i1rhoY6O0M5?VR!f9k*TbV=sDYFX937`^VL<{04&afv zAFH48=FRy#=RSP-J2U<=qXOmmJqA+b#J+7xWb(Ur5VI=8=Oj{gY%I?*0}llmWWi^; z`-$O^7u2!GX%8EcHDu0Hnf!c0>vi1GOClCCEyoEUA+t z_RW8&NrWrxJ1;3BCXwYVnSp@GcVWEIquneSRONLNQkBSbaekA#<{5cr+_N}~GJ29* z()k<_=+NN24_^%1^cn1!a3q#uMuePpukgn_aO;6weg#Q;= zNo;Cv*3;IGqfn$fCOpF3}T#raZK*ZQ~n+XtN`_iekc zYhjU{l_ku_&z}y$1R#Kz01g@ec(qMlYDUIoo{*}(bo8WsRWspphtAlH6w?@4Jl+F) zxE1iGPzfS1pkGh0??{6PpB-*608a#ynw_l)x-=~_6Jgb)?uF<@B4Xfwu4zu6J#j!Z5gWg515>vYGsJ6BFiJ=F zw6?dW10x1g=&9rCoIGi<(=d3T?ZuT7m}2S+g);QrV9}#ZWJl`BVro{|(&Ms+|H2zl zataE?K+yt{^km5t>IOTC96=z@T}o)FT&^vrapFhaz*9m( z4qvlDq!wE6!&oBP5m^a=BPg9eUF2S-86F}In ztoV3%cnC;I89`0?^7-?_(xlW>M0BjJt-)T?H%FPf+aMx40hq$gj$CVSk}tj^|5)@^ zjKIYoQUI_{mCR|YzYyuo_%VvM%<~;e#UqqghNr>8ZuN++J04lXM{1C`UW@@fkL+vl z<7jGLm6v=ldzZsP0=p1&o2CddD;pd7zoV(RZ4(4==*Tt-z&6Og$>Dh)afi_RjX@awSgdhPFOM(uGEJ}Lbre29Q5BIR+mv0l?qfz^%-?YE`opqofBttc%~wWL zL+~odayWi4{O=ZdFE*yLvRFP@*STw)W1o&J^Ow)1%@s<0Vq2|1BR^DOtxVDBeT$#% zUHxuir`iA7dW?nT+K2DY6L3}db2#cp{1cjvhQgx;6c&9uTJjH+{PyiN(8+i(fZq`A3WGSA^b0#;nwdyr;FQ1CCc|A{zzWq?^x%26h=N=DqPrQCNH!%=uA3vr2hKr&zTlw=Cea}%`hKuD)h9pMElxFW1RQh&PYn8fGF>n zIM^Mdw?y0)SPq>JU2SdR_tBX0hqi27V*V1mhb<6X=w)%+obAF;_y4)@&sL+TThfND zFe5^}02A469%+mZDc^XI0{xM;V9~er3dByo)jSRVziK6J)!!9xDjC{f4js8lB_$=O zX{BpucynHby}y1fO4g>zbHgi(|yiu)FE(`wv?9;;j^!AOg&xw*NK5C*;s_P9dlY6d58*M#Qu(NL97eYOG3Wd|&LC&a0K zvvv9*r-z^sYFXPsb|NKRei?UfBYyimB-Y)>zlxf9wCaJPptSZ|26vVCl3iV0#jgHs zW4iL8SwkGFND`Nnt0Hrm#7ymSEUp{XV!<_)?^4LnOV>yA17w|k$X(HKXvcQKwXnUJ zJe#2P8+-(u@XmbOu*uR@M?haAIkw&5&+RM^!Vi$uR$*X zA*X{c{mw@)_$rtuX{o6}^5zA0;XM9xTJqG2ql(g(Dxg`LZ`*u)=Z+mC$U%yBDoqXK z#eW^m5qSW8)iIa@xae-~T_EdrGb-i2`Ei1+@L(sWd*7$6pz-ToN$CI+$oCOsdGJ%#_PY!}8m;O{n8UccRuZW$(I4LIpDQ_Zj7 zMM8o?g6baj=<#EZDsHbGzP0|V3#o;LhU6Zfv4=cWUH*$ygbF3@nSn5&E&1qO0O<6@ zyRL97LtZ2TBKPFJZMNX?4G;Q(hHTzNSh|QqywRPW7#SNm`!mZmXu*Ft zv1RjJRTcJi^4IyXc2c12`ZvZR%|Mb1*_;tVCZ6UEJ6YJJpn-qsTV&Zx`Jxb{?v$9F$xd3G4f#g?9vqXnuAT%qPO4YKh&|@>DYl zD!IjOHx3EMR}q3nam7=mG86_RRhu%eKWdDC`K<))_TOGfxnXe7Z(ON3g@1!O^kAn! z`_Q=5uLzWnSAOJG!+&0d2ylDz@oMC38%->?^mH=ibyTM&GR~&$8`S< z0h;}>?kwz;gtaQdNHssHCG?oG7-8KH*Hp&nDx;?+$8zM-P;f)p(plumSb!>+n&3EWJi_?a;1;7z^ACB=Ft(TbswL(H&U2Yjq zh&WhoG)3ZbA~EjKqitqxb>e7I<(CJ3p}N)xt)!Q&V4rYGXn-##T8E7>Po(8qKZdH* z(%zoT%Ur5LGzWuYu9h*=*Vg>}SG#C!J0-ebx3%fZt&GI4{9e9`4@YCJFV3H0&YXg# zT=Nr&o)jd3N)~%KUDp7A`D1Tiknx&q0l;OxyfeADs;8q9A0oe^LR)DgyRS}=fnp2! zG!Q@(!S>F9`#TDt&{oiHo<2D_>E`1EQfR;2`cgmis*H252N8)xT?2z;WQ`;(?qreQ zj-|7R1Os$y5$9k!V{Il*ty2W1FN(6RcMi?RLS*++Pl2xwEv`pB7Usx>@)>y(i2692N;&9Er(w$rlr1rZ%SB>Bp~cmZ=$!}fbws#)^`wL#@q98mijaso_&Ai6(jIb*V83^d`6 zYrIO?$$9oB^sXpFQTB5sQ~sL$l}~gexguy&?EramU9@$5RyM9HE-pEcT}N@KV_v$_ zntf?_#cHfAQ^(5-!S#Fyb18)6Eu(dKH&|QGp$P^KFO&de5RWuezYHja2%fKV!n=L; zzUJ>F;nc$=*CyU>3dQIeW8Q*UfI_Tzl7dZIWMh$kqQV+*6%`Dkd&PbO6{2w2n(NQ9P6Pcmj5Nk8^Wgxli3#7|%y0vzkC^aSn4&@y6A?L#>ntObq40x#AzRSb z5QTNgp)u)x!0EKi%+^>rUolC^+x1nKH8msQy%7w`RAL-<5KzNsvS`?S;PW0}@JEIv zXV*I?JooS0r$}0Lzeh5fE}d^PQCP$?(TxbR5`=g_xUnGq*k_o;q@+=3sJ25>AbuE( zcG)r6oKo@){3NrNcm3+;g`OUBXmp(v5TKv2U}ZV>5qPIaFnFmrA{)0fhVCx-;KC8u zwT1>~>6nY=5-xp;xJNy9ja^-kZPHw9IO(yTp(@?K8?VGLwE$A5!rHaFo!Fag?#{>CtRhi5`8 z2t$87CYbT~OryNxD+NCDqkH_DMOfAZ)`Wj%DKBI!bj>rS2o|9Dd?3%}6_~RM!~{;K z_o5WnH6up~{W!Pm?JM?e8I5RekPC!fqx!Yp!(ddvI~M@kE;2R{lI>%yaE(iSl@W9g zlhmURl(gmM<{ruM2f{i*E3*I}t=38{WCm)`%GV+LV2#P70hA&2EVX0uEIP|UtCZ==~9X9tjP5jI+$S%X-aHxU5 z4#+4qw738$*P^2-mk4SWnN$0=(Z7AXyKUpol!Z?F2D;qFfHOdT03mMD1*&lklh$=v zck#678JLL=6cCt;gly}Y2wV7e=R1dJHZIB6%{Tn3(tbE&|XYb$c19=Lq8Eita z0&`0(D^pmht6=$E>FZI8fhiSaB!1!spafa$WxdP%NNI+lS#U}kidh#d GfBg^b1{VeZ literal 0 HcmV?d00001 diff --git a/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.test.ts b/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.test.ts index f3ee4c5d0a7a..5650296ee1ea 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.test.ts +++ b/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.test.ts @@ -470,9 +470,12 @@ describe('X Domain', () => { ], }; const specDataSeries = [ds1, ds2]; + const customDomain = { + min: 0, + }; const { xValues } = getSplittedSeries(specDataSeries); - const mergedDomain = mergeXDomain( + const getResult = () => mergeXDomain( [ { seriesType: SeriesTypes.Bar, @@ -480,14 +483,21 @@ describe('X Domain', () => { }, { seriesType: SeriesTypes.Bar, - xScaleType: ScaleType.Ordinal, + xScaleType: ScaleType.Linear, }, ], xValues, + customDomain, + ScaleType.Ordinal, ); + + expect(getResult).not.toThrow(); + + const mergedDomain = getResult(); expect(mergedDomain.domain).toEqual([0, 'a', 2, 5, 7]); expect(mergedDomain.scaleType).toEqual(ScaleType.Ordinal); }); + test('Should merge multi bar/line ordinal series correctly', () => { const ds1: BasicSeriesSpec = { chartType: ChartTypes.XYAxis, diff --git a/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts b/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts index 98d66e536ad2..72373bae72da 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts +++ b/packages/osd-charts/src/chart_types/xy_chart/domains/x_domain.ts @@ -60,7 +60,14 @@ export function mergeXDomain( if (Array.isArray(customXDomain)) { seriesXComputedDomains = customXDomain; } else { - throw new TypeError('xDomain for ordinal scale should be an array of values, not a DomainRange object'); + if (fallbackScale === ScaleType.Ordinal) { + Logger.warn(`xDomain ignored for fallback ordinal scale. Options to resolve: + +1) Correct data to match ${mainXScaleType.scaleType} scale type (see previous warning) +2) Change xScaleType to ordinal and set xDomain to Domain array`); + } else { + throw new TypeError('xDomain for ordinal scale should be an array of values, not a DomainRange object.'); + } } } } else { diff --git a/packages/osd-charts/stories/scales/6_x_scale_fallback.tsx b/packages/osd-charts/stories/scales/6_x_scale_fallback.tsx new file mode 100644 index 000000000000..5bec96e611e6 --- /dev/null +++ b/packages/osd-charts/stories/scales/6_x_scale_fallback.tsx @@ -0,0 +1,55 @@ +/* + * 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 { boolean } from '@storybook/addon-knobs'; +import React from 'react'; + +import { Axis, Chart, BarSeries, Position, ScaleType, Settings } from '../../src'; + +export const Example = () => { + const includeString = boolean('include string is x data', true); + return ( + + + + + + + ); +}; + +Example.story = { + parameters: { + info: { + text: 'Using string values with a `Linear` scale will attempt to fallback to an `Ordinal` scale. Notice how the custom `xDomain` is ignored when the scale falls back to `Ordinal`.', + }, + }, +}; diff --git a/packages/osd-charts/stories/scales/scales.stories.tsx b/packages/osd-charts/stories/scales/scales.stories.tsx index 522543225974..5a6fe64b56da 100644 --- a/packages/osd-charts/stories/scales/scales.stories.tsx +++ b/packages/osd-charts/stories/scales/scales.stories.tsx @@ -31,3 +31,4 @@ export { Example as tooltipInLocalTimezone } from './2_local_tooltip'; export { Example as tooltipInUTC } from './3_utc_tooltip'; export { Example as specifiedTimezone } from './4_specified_timezone'; export { Example as removeDuplicateAxis } from './5_remove_duplicates'; +export { Example as xScaleFallback } from './6_x_scale_fallback';