From 58294a75745aaf160ad1034afd1a3a5d1a2ea81d Mon Sep 17 00:00:00 2001 From: Kanit Wongsuphasawat Date: Thu, 12 Oct 2023 11:18:21 -0700 Subject: [PATCH] fix: support x/yOffset without x/y (#9135) * fix: remove 'replaceOffsetWithMainChannel' * chore: update examples [CI] * fix: support x/yOffset without x/y * chore: update examples [CI] --------- Co-authored-by: Fan Du Co-authored-by: GitHub Actions Bot --- examples/compiled/bar_x_offset_without_x.png | Bin 0 -> 4790 bytes examples/compiled/bar_x_offset_without_x.svg | 1 + ...vg.json => bar_x_offset_without_x.vg.json} | 43 +++++---------- .../bar_x_offset_without_x_broken.png | Bin 6104 -> 0 bytes .../bar_x_offset_without_x_broken.svg | 1 - ...vl.json => bar_x_offset_without_x.vl.json} | 2 +- src/compile/mark/encode/position-rect.ts | 13 ++++- src/compile/scale/parse.ts | 14 +---- src/compile/scale/range.ts | 50 +++++++++++++----- src/encoding.ts | 4 -- src/log/message.ts | 4 -- src/stack.ts | 16 +++--- .../compile/mark/encode/position-rect.test.ts | 27 ++++++++++ test/compile/scale/range.test.ts | 42 +++++++++++++++ test/encoding.test.ts | 20 +------ test/stack.test.ts | 37 ++++++++++++- 16 files changed, 178 insertions(+), 96 deletions(-) create mode 100644 examples/compiled/bar_x_offset_without_x.png create mode 100644 examples/compiled/bar_x_offset_without_x.svg rename examples/compiled/{bar_x_offset_without_x_broken.vg.json => bar_x_offset_without_x.vg.json} (80%) delete mode 100644 examples/compiled/bar_x_offset_without_x_broken.png delete mode 100644 examples/compiled/bar_x_offset_without_x_broken.svg rename examples/specs/{bar_x_offset_without_x_broken.vl.json => bar_x_offset_without_x.vl.json} (92%) diff --git a/examples/compiled/bar_x_offset_without_x.png b/examples/compiled/bar_x_offset_without_x.png new file mode 100644 index 0000000000000000000000000000000000000000..dddf972e29c53cb33c2a62a8358349b78af0fa25 GIT binary patch literal 4790 zcmaKw1yGdV-p7~jM!I%EKvQv-xc-A2(Cfk{e_em ze9Ewju7V4(owhm>a)Emlc9f?rtx>r&TBh{qYv%we+6#HLsMkG^>n^ zjCq@%2wX(u`|sbVyPtH8pPp zZ1IGgtO&dwkoBC0AYpXXR<9#+a@Ycbf`oHV?mJXLQZ%CSZu%>mnKA$VjE=0T5*8O1 zx3RM;_T<*p)#Z>2WW=qZ+PbaQZ`}fyzkwv~Cv|lQS2s60Hn#ZTVJ-Nbc9!buYVV}$ zUO$kyC4vxaZEbZmxlJ=3X!2UB8k?9vVKDsG*49`QDiR3`uc^5T>B3@JcRH;a>gtrN zta5wotIg~1g-u^2i`3kG)2YNxK|z63Rh78HJ7h{HI#oXp={mU5^CO@vlSH; zp*gF#_$*bCNF)yoR{Zwl`E@7irT5vOESKe;^JWcZYL&GF?hA!n{PAaGVv?LX_>8vH z(9~2iF}Z{i>|+;8S>N1DQbjRAMMbGcMn-OgoVCpOSvI+Lr(ZFu4LWqEWS68|TwH|z zpuK9lli%@Z*D)(Q8xNAmB^OV{F4^_&UDciEVDYak8p*S>v&9eyB~MQ=J3Biu{|zgq z6jCuZYz>m&swm&`>Z+u3T1EyDJ3BkpX`GH%jpy>^XjP6Mmvu(_t12sNU9s0(XIkpI zn?PW{w1v1jI>vx+^-p^l3u;2|8@3%a8nFl%P+rH<#}Q7H-x}zPWBC64`?yV8XQ#Z7 zkdVW>OP4NTY6$VbutXIV6;uap!KZ%p-p<;WX?>Q*UIm|5iKTJU(?=w7-n2X3S@do` zTplNQ{rYvXRv9fR;nDA!H#SaA0@E8yOTV@IbaI!R3k|!Uk*+ES2Ff^& zoWx+W(t)z#H4PdC?So!Y{JKIBMf=<2>bSfBPf-nqBDyj*yzKweNpc5wi?c zS5`*k<#BdioCiU|z$|B~v`-H=2f)3w?CdC93(U>stL927IXeqlSy>fURlQr&;$Xnq z+S+=5VP|2n*_`XZ+zSp4j&^%g0=nSdy?ZawXjNZdiG~K_ni$b%m1Go5@UGzuVT9Y1 zINZ85U6XrHHI}W`<%_6Bnc>mtsS2E__rnJ&GBPq2i3jOgr%>#Jt~G2uBIv*s5|)|C zbbhvbi~rA<_y`?nVx-F*Ru>_0jC6#|;#E&{_(+oA zDxsvVPO`DFAtov5xUe&HqYW7^}5>bK6|@4tQx{5%rJ zOhz3Y1@H+72*b0=$u*kTJb!-{f#1dumh0t3=nFc0rWrCNXmX#82P*YGSDK*qh!nSg`Ty<&u(+kVLCKz_S@l zhSoMVMx)Up6G1hx%wosOWoUE)>vcz)Un6-5rfi*|7xLZ@A7v|J{NR;iekzTVM2NRIq}`w{KlP; zkumn^Q{nzL6OjmtN=?T}u^rS7+DUV{a(fW0v55)$yd-_7mUt3W%($tE&Z!yGS`bzmSUc z8X+|`HKt9K03W}wrY5PvsQem@`b(xVUQ2Cl3Q(cS`uen1*4BteI}W8UUi8e&v|rw# z6Drl@?tsJL08@r!D7sn7Ff?4Uu@Wwy#myDK(jM=AkkV16bJZ6*1sQC4eC!>$4=hIq zpKeY6Bwx;V)%fS)BJ}E28Yz!Cq>c_HZX|m8kMzf+mx_HH*xA^6#>VuU*5>EKKz5X-8w(ry#CKpcXxJZnVHQz^0Kp8@$vDqqG4YP zjhmXArS$S;uULMboQzIOW7yl@&nD(IPBO%1W@gqlG`PmW3GEs46Vz2zyFYz0{{99S zWTS(gGLaw$c5-qu;9xDghf`QcD2ovGwMgF&I9^0#q~$e=?kxU`_}#gVpq7WfsQc^? zVa%J0u$0)_V#@pCng^dMjOf_eEj_F~J!^XeJ{g;t@yf|@0@q}be;m*uBjk~mlM{b- z_BbFQpsTO1lyy9Aso3}+nt~Z8`R=o=MWv;LV19$|-t`7;c9eS-eEn*UR8#vgUZO5` z?V6$BO*otqR0GQmPoAEhp5)wIN_+bg13BUJJM5z$K12gpF-2Pt2?z)PR>B##0TzIq zhMr!3p;9Dc(-pvtnIe{+KL_bv-RWTC;^G1bd+r}fG|rdf^Q{GN)zha>J$ILM9UL8p zMYB68Zr{GWjIA$U^K+1rk^-gFF@cEdH;_rNynP$0pul}_a3BbWBY_X$K}yt^)@wg?W^umk|+-anVx%#)Fnl&m%j z=me5IN0xJ@+&KG_tE#=-$`;R3$oQ144CC>wGeJK1q-$`H6bgk3opQC|WN2hW4G6Ee zgoM}nRAV6b;?GN!y2lphkstT=_86IJ%aj{y3{Hv_{5LoP_g9IWoSfFa)|HG8D7F{Z z*3x=;dA0a&a*Y%yBmt}@2tWh|KYk2>aLV{}y)-D<8WJe5xj6p=0;6VQGk^138fpJ` z3Hbj0{#LKOiMqSHYcZIf@`8}~wKdo1Q=Box#l_LaA#qfWt-m^ayTiI|?^i)+Oi2kZ zroJBNsDaP=lnGHNeGzHd#8DU?f#=EIec6C52m*ol+7Tor=`r_oYRV#-*<`L->x^7Z zP7a{nU}m&3lnHLWMZF!jG*_P z1+efBtd#0w>2#`)urR`7PL_~_#2&zhND9g(%n&OtFQ0|5C-(96wS>`h)0Sb{d{#cv zOV5%0u5@`BPnprCYc#-WvBc!vY*J5I7Sm8s*%$9@W=1`frgO|5Ee911n8SpBVr)$v z{0~Zpe14LPt}%%m$-60KUNhdw$hMT|eVD>@ACoxg2)0u1`jpeKr?&sn}_uoYG|5a1mA8xBlM5?k(K2d64E{0}J5B-B6dnnp@1=wk}Id|%5 z=dcS1Oiz{xza4?Arilp#8gBXY>5=hqx~8TkoL>SE`^(DG zO4Dyb6}-K@lT%WF$5GqNwLdyqZ~n&CAS^1X=I$;Ur3^)}^jV6rVo7MYkU%2A9zqOA zq*Z$WU0GQfycL#%$AAUHM>RI?qo~Ej$I7&sfByRQ&J7G4+S1a3^Wnc{;bz57;=1k>`k%V zF61OaA18Kim@)UJzaD>^v`Ji5Wfh}iMdcD&C($lz5cH=S5LrMdkx@~Ed#jTaYHDgY zM%2JPGL6j7j;%`ubjA=lYhG6mD*AOo-TKn?Ry<-SI9ILt}HnhDxy|b*t6z ze<-m3=CS{^T>j0Ki*+YOCqE_^c2v7Z?yonzMJK;lAAjM1y38hc#lxJptm@~csm8=X z;lJwA<{&>WEiDqTXIFPOUWr;l>a-dx1&5eG{Gz}H4;%oPzlC-=JA7+@#K0YL1Z`++ ztn*q>i+b|po!jAC=DYLH?uLY$!~3_s_im3WT-3&@v_}mM&b9gq1CQU@9^mdB9i`FJ z(>q$LA1wqb?6P}y{IIXDZ+`R5MGU|tB_+J#;^M8vXLm8N$guIjk-@>R`ugjTLX{`f zfFe}DkwAwe7K^QIZhn?g;OFc6w$bH_c^~vn5AvYiY19TVo28_hqGA%95wXI#bv9GNgd2n{T z3r-{`sHmto z-1C|T63fTO2R1@nz+Z?V#>U1r002oTCQOMY$A?|GF d|Iq7#B;!#=N~EYX6*!23XsYTVtCejc{s%GI*v \ No newline at end of file diff --git a/examples/compiled/bar_x_offset_without_x_broken.vg.json b/examples/compiled/bar_x_offset_without_x.vg.json similarity index 80% rename from examples/compiled/bar_x_offset_without_x_broken.vg.json rename to examples/compiled/bar_x_offset_without_x.vg.json index 7224919868..c3fb601dde 100644 --- a/examples/compiled/bar_x_offset_without_x_broken.vg.json +++ b/examples/compiled/bar_x_offset_without_x.vg.json @@ -1,8 +1,9 @@ { "$schema": "https://vega.github.io/schema/vega/v5.json", - "description": "xOffset without x will be replaced as x", + "description": "xOffset without x", "background": "white", "padding": 5, + "width": 20, "height": 200, "style": "cell", "data": [ @@ -39,13 +40,6 @@ ] } ], - "signals": [ - {"name": "x_step", "value": 20}, - { - "name": "width", - "update": "bandspace(domain('x').length, 0.1, 0.05) * x_step" - } - ], "marks": [ { "name": "marks", @@ -59,8 +53,12 @@ "description": { "signal": "\"value: \" + (format(datum[\"value\"], \"\")) + \"; group: \" + (isValid(datum[\"group\"]) ? datum[\"group\"] : \"\"+datum[\"group\"])" }, - "x": {"scale": "x", "field": "group"}, - "width": {"signal": "max(0.25, bandwidth('x'))"}, + "x": { + "signal": "width", + "mult": 0.5, + "offset": {"scale": "xOffset", "field": "group"} + }, + "width": {"signal": "max(0.25, bandwidth('xOffset'))"}, "y": {"scale": "y", "field": "value_end"}, "y2": {"scale": "y", "field": "value_start"} } @@ -68,14 +66,6 @@ } ], "scales": [ - { - "name": "x", - "type": "band", - "domain": {"data": "data_0", "field": "group", "sort": true}, - "range": {"step": {"signal": "x_step"}}, - "paddingInner": 0.1, - "paddingOuter": 0.05 - }, { "name": "y", "type": "linear", @@ -84,6 +74,12 @@ "nice": true, "zero": true }, + { + "name": "xOffset", + "type": "band", + "domain": {"data": "data_0", "field": "group", "sort": true}, + "range": [{"signal": "-width/2"}, {"signal": "width/2"}] + }, { "name": "color", "type": "ordinal", @@ -95,7 +91,6 @@ { "scale": "y", "orient": "left", - "gridScale": "x", "grid": true, "tickCount": {"signal": "ceil(height/40)"}, "domain": false, @@ -106,16 +101,6 @@ "ticks": false, "zindex": 0 }, - { - "scale": "x", - "orient": "bottom", - "grid": false, - "title": "group", - "labelAlign": "right", - "labelAngle": 270, - "labelBaseline": "middle", - "zindex": 0 - }, { "scale": "y", "orient": "left", diff --git a/examples/compiled/bar_x_offset_without_x_broken.png b/examples/compiled/bar_x_offset_without_x_broken.png deleted file mode 100644 index 78a74f2ff1bd72204b4b8c45d7b6c7609105f1dc..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6104 zcmcgwbyU<*xBaQKv?v1-N;d;2Eg>UFDgqMHBi$t(QW66=(j@{)h?H~*5(82K0wO8h zF_cKYJKy`>da=H@-hXegF2ni_bLZZ3_St9e6Zu#}iHwAv1cD$k<%ftT;58SlEg}N& zTjyhqFnA#_S5-nl7r38yP5JQr@wkk|Ig*qrj#)?WTgxU&u42qt~E!PUJK6s*^h zbsZV`?(3~IE_zRVR-2*6NFfZOkC?+ThGRShX^6&Cc|QVil6a6NcCf=xX;BeG26B|w zsh(R+&oCz_7hVbR5}Zu+mRa`NC*Stp7CAm>^4t@FU1M`>d_bC*(Izy1F`Gh9gWp- zColgphXN9J{tbc8FZHH|w6w^jq@}Tli^CQdr?#cIPzLYby@SwbbU|sUlA0Q+u!u-v ztlzI+zu<6qOA`MxQzxgmvNHJTrc+?sdTDd)!TL{DUf!#>Z{POXUqPEQmg`N%} zesFN`yP~3KR;5_eqn+8V-d^qTt!5<&D=RB6Z|~XeL|(7$raz`;W-n1F(y_5I)7DU8 zubtnQK*&CS{_MOlCUm+e7^peEJeaF=J5j}Db%?#PvhvoQJCzegQZXn4IiAG1-(N%E z!osbi#m{E9w<#DI8B@~JJsV>6_!Mnzd2oSOctvj3lf>^e8%FEAyP*7#japnv3JFec zZEueNfj>PvOFR_uJ+#k|^dPaZu^}QMX`OEJZJ6Q!-;h&SNZjDL9gap*y@`*%o!mop z;|2k=b8ry$IlU&Ff;o0!VZrg_bR{=#XHbcmJE_KD;!ykMvuDrRJ30E8w+n8>;3AW#=*(}y8zs&pelUJU{N(!2YK+xkzrO?*VIT$-!>PzqX_Dx<(>l*V1%Db(qFj_vnpphb7RtX8ZFJHchd+)v< zdf|Jx@wL%=&-rM_Ozo{S-J`eCv$+g899|NZr*qMQrZzUj&_i|gj>F9fDr)MX59&xH ztIYWk(y-wL+0xQdL{!v$b8{Bsz%vgS6&Bl|WOTVmgj{E&HY;mx{x}T{4K9mOZbn*p zYVsNywPrOkGGbH`XJBBEQ(vF<-L-NY+0+4&^7T7%L+IqCLkby-SfG#wzjsjl^(j zgpPtn0Q^ls6O=LN1mgjBh!_E>gwuhS5HqHU>xu;2K8Q-l$4Yb1|y0@wDxQwp)mCR6JK8` zu(R{&k;`~EL5qqKP*xVQ+t8sSo6Wu+n|0c%{{ zE^_y$kEd*||C8-6-oX@igh4|AB%F|uf$~2&yaX}H_z_`XyL)>QGvs0D0vaZ!*qt5c z%90;%U zee@YJ{`AMk$Bho)%G$@fJ%WQnL%|k3Ntaeqx{LEuOOB(!^YJ=2&KFyg-%ehvTp|4z zslcVe5#85nkdT>RBNJeI+w|{kT0im(R=E~}-*|#_B;$AzZ zaUvh4>)Z$*Jb2J(Y|gS4l8koQ7%Rt7dv9;=(aKxvf=jZNfckjuD=47T(j*5yyuKv* zPStgax%GS@iLe z(izd2|8BNP+t}!6c#&>tYiB26OiT=AbjQHYpE&Hgy16w>_Lo#wd~_X-du^$+DcHy$ zvb4O+3fiJ@Dk%=hyrqqop@QP#j`zwyZ?Lno!-UN7FmcQjoSdBE;^L;$(zA~!36iG{!>^yZs;TzxUS^?f%tH1>S~=~EMS)FfB*hXk@1%#Afs>h^hZP&FwiZ! zO5~}f2B)OJ-oJk@XJWz}eS`l#IA`FnQe{Lp+|t5gwv#!q{gZwL1jWY20+yJ!ctW2u zr5%p#&y*^ws8F!BE||L;i3mPE_A$Q7I<8~^0}asS&z8}`;$n7rIn|8c9@%qYI?$-5 znGdM=b9S%vg0TMH+A8rHcf!tfz=edqp8nnQ7L2u~O=Qh2DG|`fl8Nc+dI%U2xiwh_ zSTQVyY>HdGcBDv43Wy~f^UYLI^5+YqI%65zKu^l@IRi=PIN5IguBwTUDkMdEc-##* zUd_)p{&R4!_~BMglQxq`f0itBM@I*jfB*&v8FWet5$KUXw)|20_fQa0Frfg&@g_}8 zO}T*P1p?C2))w+}F5-nIonoq5IH*nTr1iBm=i_~w(R419ci^=DN+2W#h_;;mmgDq(<%86F;XZ!lL-xFU#8wK?Jd%gxE5yncNo3*7wY zy3_3j{}WLa6_tU}QG$2#dQW%~cb~!rWtvFlK`hY;3B!w31qECiQw_Q{Ct$OM<>h&x zLcOpZv<-f7e=fTF`am_}w2`T!<2mVvYwF-!v;u~5Po7X`WMu3dVUc4K6S?4Kxr#As zFEZc1XJZg|hCH{Y!%|cAYs^qFF%OZfs2Nxs{ay3wr--in1zicidf5lZuU+ z7}wLie0}GE!XhFjUKK*ACJQ`x@j{eCEoHzV9iV07R|O$AH}}y({3A*lnovN@Uz)Dw zQ2888R)&S)17>hOJN5voK;1=o#~U>MmX3}G*jhU|IcRrx_ifRmoN4W?JVpkF$C{d# z#KgpaLizgR$2DkYNLBFO&v0gGuK^1ZQ#Us^pbIbRj@j1KEnsJ-rF*v9Zi`v3q&E48gK0EOb2I8Q0(!wCc#BTC#IjkXiltFHfE}uk}%W6w^I=OH0d$ z*x02F8AV!JTA*fgnwl6vH75(2Y`K0cXo%#iYyBCTatCcOQz&gI?R&^4EiL_feLWfg z$Ahk~UQ0(OY+>Q~kwv3d5=0@Z-OKZ^Pk~$pS|AwY6y&7#MaAC!BK2%E~4@aIGR$ji69F3PdzR;)jqC z=AxgmHt-?dlABGjl-B24|V1t2Rkiu_pIu8W=>)&6y`8CSr}Q4L)h|*)Rpg zTkvTkg`@BLcM6~z-#I1R%2*#k;^tuQ{MXCY^X=fTw-cTE9`=pmn{oS@<6-5sjVGmst5>}juWOvJ`YScq{OgbYZ(IJ) z=eC}1{6F2p&t~M3<<1q>+qjzD0^X_Kx&V#;KR0d+)f7lZ_NCp$DYxK|5KZF}zZW!u zf`UNoxBdD>S5i{qmvDcWNRZi-nwr{q9z$kW=Zdcs$J7C81bg;pMs}p6y7~qvOwg*9 zb4hb7z3AxT7^T7i8Lq6Xm;y39!gexi>gslO=0y;IhO)T0=XIu|63! zJRMXD?E5(0;u3hL$aV%xjalqTX65801Zn~%4=iYjw;V$u-h#(ADod4$)KHkSD?nS9Giq`U|4 zkC2e?OZRP@F~S}ntb6@k zOa?c7#VGZ{6p)I_#M$bjz}A|XfM_7Q8;|CqYdyD_{$Y}Ox^Om-MN*Q!`QmJwiiwHb z$;s&y8y6U=$M>|-dw+!rumW~F@Zu)Wia-$~Kwp~n|6}w|7ls8G1)dWhBBc|?3n3u8 zZ((7PEE*OT2B4&+w!OZ+9oLgAxTi_3d95+DJTH$RFE1|&XBp_ee@grrh%tHDRZ`n~P4q!l{u_pmSzJ z0;rkHohlTfmlS^Srf1RLxTdNqu}$TnleHQQjnz2$I#GF59o!glcM5c z3#C{FGvIuCfBcAdn5s|I(bfG^C#$8UwY0tc9sCnbDLx+Fex0oPdm%6bCgmCXK7G>W%aRRz8^d(3{`;APghV;uj|!Y&52yyc z+-ggZjSYca22ePBC42fZ5|JbGGe7dq@vsIMH2?}au5>gqGD>cUI@(zoWCg<*v(SATYOi4e|TxBAW(MbH*bO^+?H+u&tT$*#V;{# zimI9Rxtf}qYVGKVY1sY}ot8#_3T6U0GP1X~hkI@rg3-%msM>KV>dhPKb_}L2QH2ZW8raODp8A-RSR?!Q9t9ixCkKL7ACT(1kkDWXs#7g_-lS6FSJ! z+`JV)00;gnE$64pvLYDRRWdRQCmGW@Z~;zS7&>9TnMOzO=sH1J0ZZAlTqM zdl`u;$If?(Kv6xxVCaB^Qq(w*D50g_zgwU(&0NLufq};yH=FMhyKajgm z7EZ^vw4$1+5?k5 z2r@P{HVP2r-#kvSUp{*S(4um}06ZAP#U#eh%e%flF+f7ktmx_K3CG-u|28l-21HO^ zrO~q3GbkuOpA))u?_N{SHW;p48o>67K{&}(RaI5%JR3YD18@!uaE@Leh&V$dE1Bp( zC$XQD%N(8v9BvAwynK0SZ+F)m8Uv%Re%Q%D3U;#2Ey}~z);1+BF3y}0>izaD@;WUo zh`E&C5&TLE(88%8?iDpPx<0+)j+5efMMXUj+{DDBLu3vi5z{PcWMUEr@IMP?3_JV# z?S#yJLHqmeUC)KpQibsW)7QE!lUw$s;q&tHcKB>=ZgzEd-?y>hws&w4^4?t#5Hre< z@+1ecAVu7T1!pV`4N=J^0d&AefbNd~dJd?-h>Z<-#kZioRr+(7_`R+n&XA=xC zdV&9}YiJMwZBI2S*E7(fXS#kUew zyr(Qd-M*Ff4({se5-_>)&?gUT# \ No newline at end of file diff --git a/examples/specs/bar_x_offset_without_x_broken.vl.json b/examples/specs/bar_x_offset_without_x.vl.json similarity index 92% rename from examples/specs/bar_x_offset_without_x_broken.vl.json rename to examples/specs/bar_x_offset_without_x.vl.json index 8a702c5fd2..d106c6c338 100644 --- a/examples/specs/bar_x_offset_without_x_broken.vl.json +++ b/examples/specs/bar_x_offset_without_x.vl.json @@ -1,6 +1,6 @@ { "$schema": "https://vega.github.io/schema/vega-lite/v5.json", - "description": "xOffset without x will be replaced as x", + "description": "xOffset without x", "data": { "values": [ {"category":"A", "group": "x", "value":0.1}, diff --git a/src/compile/mark/encode/position-rect.ts b/src/compile/mark/encode/position-rect.ts index afbd417d48..b2229b1433 100644 --- a/src/compile/mark/encode/position-rect.ts +++ b/src/compile/mark/encode/position-rect.ts @@ -166,7 +166,14 @@ function positionAndSize( const hasSizeFromMarkOrEncoding = !!sizeMixins; // Otherwise, apply default value - const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type'), useVlSizeChannel}); + const bandSize = getBandSize({ + channel, + fieldDef, + markDef, + config, + scaleType: (scale || offsetScale)?.get('type'), + useVlSizeChannel + }); sizeMixins = sizeMixins || { [vgSizeChannel]: defaultSizeRef( @@ -190,7 +197,9 @@ function positionAndSize( */ const defaultBandAlign = - scale?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding ? 'top' : 'middle'; + (scale || offsetScale)?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding + ? 'top' + : 'middle'; const vgChannel = vgAlignedPositionChannel(channel, markDef, config, defaultBandAlign); const center = vgChannel === 'xc' || vgChannel === 'yc'; diff --git a/src/compile/scale/parse.ts b/src/compile/scale/parse.ts index a2334bcf7a..e6baf799af 100644 --- a/src/compile/scale/parse.ts +++ b/src/compile/scale/parse.ts @@ -1,7 +1,6 @@ -import {getMainChannelFromOffsetChannel, isXorYOffset, ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel'; +import {ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel'; import {getFieldOrDatumDef, ScaleDatumDef, TypedFieldDef} from '../../channeldef'; import {channelHasNestedOffsetScale} from '../../encoding'; -import * as log from '../../log'; import {GEOSHAPE} from '../../mark'; import { NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES, @@ -56,17 +55,6 @@ function parseUnitScaleCore(model: UnitModel): ScaleComponentIndex { } let specifiedScale = fieldOrDatumDef && fieldOrDatumDef['scale']; - if (isXorYOffset(channel)) { - const mainChannel = getMainChannelFromOffsetChannel(channel); - if (!channelHasNestedOffsetScale(encoding, mainChannel)) { - // Don't generate scale when the offset encoding shouldn't yield a nested scale - if (specifiedScale) { - log.warn(log.message.offsetEncodingScaleIgnored(channel)); - } - continue; - } - } - if (fieldOrDatumDef && specifiedScale !== null && specifiedScale !== false) { specifiedScale ??= {}; const hasNestedOffsetScale = channelHasNestedOffsetScale(encoding, channel); diff --git a/src/compile/scale/range.ts b/src/compile/scale/range.ts index a25d382470..e114d5625e 100644 --- a/src/compile/scale/range.ts +++ b/src/compile/scale/range.ts @@ -221,11 +221,39 @@ function parseScheme(scheme: Scheme | SignalRef): RangeScheme { return {scheme}; } +function fullWidthOrHeightRange( + channel: 'x' | 'y', + model: UnitModel, + scaleType: ScaleType, + {center}: {center?: boolean} = {} +) { + // If step is null, use zero to width or height. + // Note that we use SignalRefWrapper to account for potential merges and renames. + const sizeType = getSizeChannel(channel); + const sizeSignal = model.getName(sizeType); + const getSignalName = model.getSignalName.bind(model); + + if (channel === Y && hasContinuousDomain(scaleType)) { + // For y continuous scale, we have to start from the height as the bottom part has the max value. + return center + ? [ + SignalRefWrapper.fromName(name => `${getSignalName(name)}/2`, sizeSignal), + SignalRefWrapper.fromName(name => `-${getSignalName(name)}/2`, sizeSignal) + ] + : [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0]; + } else { + return center + ? [ + SignalRefWrapper.fromName(name => `-${getSignalName(name)}/2`, sizeSignal), + SignalRefWrapper.fromName(name => `${getSignalName(name)}/2`, sizeSignal) + ] + : [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)]; + } +} + function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange { const {size, config, mark, encoding} = model; - const getSignalName = model.getSignalName.bind(model); - const {type} = getFieldOrDatumDef(encoding[channel]) as ScaleFieldDef | ScaleDatumDef; const mergedScaleCmpt = model.getScaleComponent(channel); @@ -245,18 +273,7 @@ function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange { } } - // If step is null, use zero to width or height. - // Note that we use SignalRefWrapper to account for potential merges and renames. - - const sizeType = getSizeChannel(channel); - const sizeSignal = model.getName(sizeType); - - if (channel === Y && hasContinuousDomain(scaleType)) { - // For y continuous scale, we have to start from the height as the bottom part has the max value. - return [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0]; - } else { - return [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)]; - } + return fullWidthOrHeightRange(channel, model, scaleType); } case XOFFSET: @@ -374,6 +391,11 @@ function getOffsetStep(step: Step, offsetScaleType: ScaleType) { function getOffsetRange(channel: string, model: UnitModel, offsetScaleType: ScaleType): VgRange { const positionChannel = channel === XOFFSET ? 'x' : 'y'; const positionScaleCmpt = model.getScaleComponent(positionChannel); + + if (!positionScaleCmpt) { + return fullWidthOrHeightRange(positionChannel, model, offsetScaleType, {center: true}); + } + const positionScaleType = positionScaleCmpt.get('type'); const positionScaleName = model.scaleName(positionChannel); diff --git a/src/encoding.ts b/src/encoding.ts index 7052be9662..0dafbc975e 100644 --- a/src/encoding.ts +++ b/src/encoding.ts @@ -558,10 +558,6 @@ export function initEncoding( continue; } } - } else { - // no x/y, replace it with main channel - channel = mainChannel; - log.warn(log.message.replaceOffsetWithMainChannel(mainChannel)); } } diff --git a/src/log/message.ts b/src/log/message.ts index adb059e2fe..4f2cfd11d3 100644 --- a/src/log/message.ts +++ b/src/log/message.ts @@ -154,10 +154,6 @@ export function offsetNestedInsideContinuousPositionScaleDropped(mainChannel: Po return `${mainChannel}Offset dropped because ${mainChannel} is continuous`; } -export function replaceOffsetWithMainChannel(mainChannel: PositionScaleChannel) { - return `There is no ${mainChannel} encoding. Replacing ${mainChannel}Offset encoding as ${mainChannel}.`; -} - export function primitiveChannelDef( channel: ExtendedChannel, type: 'string' | 'number' | 'boolean', diff --git a/src/stack.ts b/src/stack.ts index db05480795..00e7bb3b38 100644 --- a/src/stack.ts +++ b/src/stack.ts @@ -176,16 +176,16 @@ export function stack(m: Mark | MarkDef, encoding: Encoding): StackPrope groupbyChannels.push(dimensionChannel); groupbyFields.add(dimensionField); } + } - const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset'; - const dimensionOffsetDef = encoding[dimensionOffsetChannel]; - const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined; + const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset'; + const dimensionOffsetDef = encoding[dimensionOffsetChannel]; + const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined; - if (dimensionOffsetField && dimensionOffsetField !== stackedField) { - // avoid grouping by the stacked field - groupbyChannels.push(dimensionOffsetChannel); - groupbyFields.add(dimensionOffsetField); - } + if (dimensionOffsetField && dimensionOffsetField !== stackedField) { + // avoid grouping by the stacked field + groupbyChannels.push(dimensionOffsetChannel); + groupbyFields.add(dimensionOffsetField); } // If the dimension has offset, don't stack anymore diff --git a/test/compile/mark/encode/position-rect.test.ts b/test/compile/mark/encode/position-rect.test.ts index a053a2d4d3..f042b1f725 100644 --- a/test/compile/mark/encode/position-rect.test.ts +++ b/test/compile/mark/encode/position-rect.test.ts @@ -29,6 +29,33 @@ describe('compile/mark/encode/position-rect', () => { }); }); + it('produces correct x-mixins for xOffset without x', () => { + const model = parseUnitModelWithScaleAndLayoutSize({ + data: {values: []}, + mark: 'bar', + encoding: { + xOffset: { + field: 'a', + type: 'nominal' + }, + y: { + field: 'b', + type: 'quantitative' + } + } + }); + + const props = rectPosition(model, 'x'); + expect(props.x).toEqual({ + signal: 'width', + mult: 0.5, + offset: {scale: 'xOffset', field: 'a'} + }); + expect(props.width).toEqual({ + signal: `max(0.25, bandwidth('xOffset'))` + }); + }); + it('produces correct x-mixins for timeUnit with bandPosition = 0', () => { const model = parseUnitModelWithScaleAndLayoutSize({ data: {values: []}, diff --git a/test/compile/scale/range.test.ts b/test/compile/scale/range.test.ts index 36cb40bec6..2e92b6866e 100644 --- a/test/compile/scale/range.test.ts +++ b/test/compile/scale/range.test.ts @@ -132,6 +132,48 @@ describe('compile/scale', () => { ); }); + it('should return [-width/2, width/2] for xOffset without x', () => { + const model = parseUnitModelWithScaleExceptRange({ + mark: 'bar', + encoding: { + xOffset: {field: 'xSub', type: 'nominal'}, + y: {field: 'y', type: 'nominal'} + } + }); + + expect(parseRangeForChannel('xOffset', model)).toEqual( + makeImplicit([ + { + signal: '-width/2' + }, + { + signal: 'width/2' + } + ]) + ); + }); + + it('should return [-height/2, height/2] for yOffset without y', () => { + const model = parseUnitModelWithScaleExceptRange({ + mark: 'bar', + encoding: { + yOffset: {field: 'xSub', type: 'nominal'}, + x: {field: 'y', type: 'nominal'} + } + }); + + expect(parseRangeForChannel('yOffset', model)).toEqual( + makeImplicit([ + { + signal: '-height/2' + }, + { + signal: 'height/2' + } + ]) + ); + }); + it('should return padded duration range when there is a nested offset with year time scale and default padding', () => { const model = parseUnitModelWithScaleExceptRange({ mark: 'bar', diff --git a/test/encoding.test.ts b/test/encoding.test.ts index 36cd19c454..847bda7c6d 100644 --- a/test/encoding.test.ts +++ b/test/encoding.test.ts @@ -87,25 +87,6 @@ describe('encoding', () => { }) ); - it( - 'replaces xOffset with x if there is no x', - log.wrap(logger => { - const encoding = initEncoding( - { - xOffset: {field: 'a', type: 'quantitative'} - }, - 'point', - false, - defaultConfig - ); - - expect(encoding).toEqual({ - x: {field: 'a', type: 'quantitative'} - }); - expect(logger.warns[0]).toEqual(log.message.replaceOffsetWithMainChannel('x')); - }) - ); - it( 'drops xOffset if x is continuous', log.wrap(logger => { @@ -125,6 +106,7 @@ describe('encoding', () => { expect(logger.warns[0]).toEqual(log.message.offsetNestedInsideContinuousPositionScaleDropped('x')); }) ); + it('does not drop xOffset if x is time with timeUnit', () => { const encoding = initEncoding( { diff --git a/test/stack.test.ts b/test/stack.test.ts index 455748d07d..d215d92250 100644 --- a/test/stack.test.ts +++ b/test/stack.test.ts @@ -1,5 +1,5 @@ import {NonArgAggregateOp} from '../src/aggregate'; -import {DETAIL, X, Y} from '../src/channel'; +import {DETAIL, X, Y, YOFFSET} from '../src/channel'; import * as log from '../src/log'; import {ARC, AREA, BAR, PRIMITIVE_MARKS, RECT} from '../src/mark'; import {ScaleType} from '../src/scale'; @@ -327,6 +327,41 @@ describe('stack', () => { } }); + it('should be correct for grouped bar', () => { + for (const stackableMark of [BAR, AREA]) { + const spec: TopLevel = { + data: {url: 'data/barley.json'}, + mark: stackableMark, + encoding: { + x: {field: 'yield', type: 'quantitative'}, + y: {field: 'variety', type: 'nominal'}, + yOffset: {field: 'site', type: 'nominal'}, + color: {field: 'site', type: 'nominal'} + } + }; + const _stack = stack(spec.mark, spec.encoding); + expect(_stack.fieldChannel).toBe(X); + expect(_stack.groupbyChannels).toEqual([Y, YOFFSET]); + } + }); + + it('should be correct for grouped bar without nesting', () => { + for (const stackableMark of [BAR, AREA]) { + const spec: TopLevel = { + data: {url: 'data/barley.json'}, + mark: stackableMark, + encoding: { + x: {field: 'yield', type: 'quantitative'}, + yOffset: {field: 'site', type: 'nominal'}, + color: {field: 'site', type: 'nominal'} + } + }; + const _stack = stack(spec.mark, spec.encoding); + expect(_stack.fieldChannel).toBe(X); + expect(_stack.groupbyChannels).toEqual([YOFFSET]); + } + }); + it('should stack even for two plain quantitatives with the orient on the axes', () => { for (const mark of [BAR, AREA]) { const spec: TopLevel = {