From 292f4d908f18fb1ec071214651d62353dc83f274 Mon Sep 17 00:00:00 2001 From: Kanit Wongsuphasawat Date: Fri, 24 Feb 2023 16:54:27 -0800 Subject: [PATCH] fix: orient logic for when bar with x=T + simplify logic (#8739) Co-authored-by: GitHub Actions Bot --- examples/compiled/bar_1d_temporal.png | Bin 0 -> 4608 bytes examples/compiled/bar_1d_temporal.svg | 1 + examples/compiled/bar_1d_temporal.vg.json | 76 ++++++++++++++++++++++ examples/specs/bar_1d_temporal.vl.json | 11 ++++ src/channeldef.ts | 4 +- src/compile/mark/init.ts | 40 +++++------- test/compile/mark/init.test.ts | 9 +++ 7 files changed, 115 insertions(+), 26 deletions(-) create mode 100644 examples/compiled/bar_1d_temporal.png create mode 100644 examples/compiled/bar_1d_temporal.svg create mode 100644 examples/compiled/bar_1d_temporal.vg.json create mode 100644 examples/specs/bar_1d_temporal.vl.json diff --git a/examples/compiled/bar_1d_temporal.png b/examples/compiled/bar_1d_temporal.png new file mode 100644 index 0000000000000000000000000000000000000000..564fb85ea2ba7595c045b7328439a4e3adb36762 GIT binary patch literal 4608 zcmbVQcQjn>wXp}Cl1xBA+o-9cXzZ7@l^tws+>4LQeQWY1b?%y}Dy>Zu zT@XouBdu{M-*b`&@3=KAqRWbVxU~)o)#zQ0nC;v`p2<(HJRVU=pimUFX8!nYIjy-rmgAEgN6l?!AKB3EJ)-jLeYx`-CfkM&mwR_?T`c99TgXsl10|rfv5O9#W%9X4v|leprzw zrap&<=tu-z^qc>EPn=J9{~xE8%AW4e#>P?#Tej>5-%C*0f8+3b=wo+xx3{215Si=G zPxDCL6|%BRl%=ow#%1#lD_wPvzyF9OQ)6E5FyL#Te|dpN#Xv*epI%~L`!-y_m1pGI z!0t6VEGC1A{th6dANp)E=E5%?iiwF46BF}J#ov=td;XkVQBg4>GLp!B?hB!?b(^D6 zPn|thzhW#UEiEZKJ5INRlb;_`O(Y;B#HFB+yT0yh9eiZMbva&T5p+Bs`D0>2;`O>Q zDJ5elO?cd0>B;W1voi_19?EwyF@0EUS6^TB9SMh_8XFXuvqhC@JgdUs+Gxq%!GT>* z>>W%KHeJLE#C`PWQB+(UdSD<yeb18{v>!HkJLc_TZ-SArarc)nq6{gw%(|-l{BTgbhG#;xwY4pu zKVNfS#@qtipj8reo0KG_W+s0e94znTpC5vcK%- z@4q*00ZYuvq6}3lP^=isA9HREIO3P{*?e4OYGh~_Ze62ZWm<2KO$+QFVL?c_<>%#v zO5^;L`{@R74_SwQ5u8o#eRubzsG(Hm|qh_8vKRNg@J}wT!Jz~0hx2&w} zIf$N;p01#etgTI)uz_cDs>gqFnGohe)zu{p4e3YF0F>+>j_MH&HM*FoBGnUEDlM7^ zi;Y+wdPbxQnAQ32SR>bSl9Ritt&l%ky?9o?!w+Wu-I{7Xm~^R`UtUI^9&Pwv;{Ou< z#{0RYE2DY} z=&`ZXlH8DI{>x|CZn>$c=vJ>)f~bUqUR%*2q%X|0>KXda))Z^0m4yX%eXK$ZWE0nJ z(RaBJBl1DUgF{zW*Rsto5nzDG{b=2UTjxU_^J>I(`WDaS8%pH)`T4wjeDfnk+P{wi zHZu-iH_Vn47uWgiK?9D~RTUKp1KL?aySr5evmU;ws1S%@5OJ7la@&f5{AN;9QVQ45 z)HE1b?1`iAO<-kEA~&$I5;{LWj~H`qj5hBGYJH0IsiIMe0@v3%jdHU`&^&nXAYU&R zlHc0;Yukmly1E*3sjaPz29%X@2zm|eQq%5Gk3vIf@ux@h_V)Ij&W)4IQ~9bf-DyG= zo8yE1{r>!^s9`BXV-u74)zt|Dr>v~3PfrEO+4ake z8yXtiS7Nuwl;T2Q}~*ta>@-24s|6_xi6?j8uM{p86qNdK2DT0a1j!lI)2g$0njf1F>J zbPyIg!Y!uYr)wpCfq{!VJ2$6ul6dalr@4Jw&BsRyb&21RvpDjZg2>8ds>a-|4LEYP zNA2#q_UA5aPiwiGiYjPe6baso1gy*cK;7Oi=0nvNAMXxw`hirl-FJ_OJM9> z=~v#Eh&QBE(4U{4clY*61Rc8rHe8!*tQdg!BJ$#X{4g%BsK|f>2SmP}`XKR=nw_29 zvGG~84NBMEo(q5(;}e7K%#jcLhdu~IWCncD6?wDXYjsd}qiGK`U>&Gu$O+cQ^LY39 z^vq1F$KtgdSn>FLoOXI@xXAg7{2&&*_(m-CmGmzS26eH|T5 ze(1Hr3ukk7adCR}s<^m#B`3sp6*gutx$zg|lbDng+W1TiF#uR_vOD-_%&gM7Ju~?1 zPosqounjRb=ivqgY6g~;{Y`FDjPe1J!Ka4?O(lheGz<(Sg@8m~zkaRtxw@KKM5HDj z!yj}wtiH6e0+m9T*Fs5vG+~;6s{GlR{r>s&hAVV(5JpK#G&nezJ-$8FEb{j4TSiLK zVx0^nFR%J1GYS{ScHXkt&~WM5AbM6-R;gFh8U_Zm%gf6zUcB%=$92Lb;1@w^2`mFI zbAx+wFVA19#WE`H;M!BS{<@rj#MK3zcvhMUydk?;TvD6?IibMQ_^!{~8(_6F*i~CZb>v?sk*}q{q+`e-B%3QgOVd2cP8I0^RPl{(}z! zHU)MV{>!x(kQHE4#OY}OF67LOz}jz~#L&=i_wdkooMP@zYjHw?UP;48e24Zns5u=g>-Qq4zJKQd zZVxQh6Cb>5X63s*-3K`7T!{fh|-gN2&OKt+XESXeAU z#glpT_InjB45C|+TY-PK2SA_I`1{ZG+`^u-eT_^8Fx0I&5K3p)(iW~SVu`FU}fBWeVd(f<(z!)yE;2} zI!cNIo7&damQsZ0$?>}egTX)^TU%TIX8o>y)`;QW+S$o6=EQ=ss+QmV#<-!Cbz@p{%{xs1w8_I2OSKRWZHs= zlvY-%6b$b!CF_CTC#M@V9$sFYj~-=>kDHEHTVYu*cFF87;rI=kO7nVB7;NV(O&VYm zS(BJK-~gh{&(Hh$`SFgH*V^?itgS_-r>Be4HOy~r2I);vGt0d1@7IZ-0p8kucTN#O z^5&1D%NZ~<7%cY6#qK_t(#w>LK0gmacnQPdaHyS~8>VkyfJmGR1uuZ~$^}or0nZTKg#kAF!O#H9MOd9)4|ldK#ESIT8|*!<|{y zR4r9qU1~x?LNLj^&(3x;n*pxcCSZ*V5VM+<)ji;T-VJ_Bk=rTF%1 zfB_EoMd<$;)7GZrB9eCA0{nUbRcdQ*_rEw|g;6yrFTEgj$qDnC7 zeE7hqrKNS%hGJsFUmv9M{=|o-E-73YPJ#$f9)fN_>Mp=e{`&PxsbDw?2=F`5#Pdr_ z_c=H@Rb5;vj%Rj4ct~%u2r4NnqeBUah}o<|PVB)IsFML0F&h}<*uXT_{hnQ!*<8NQ zN`oE}{y~R(esPhL02n5o_4Re&|3K2oQ?+^1(%@2=ZHp(@(iF^*179`00Jz8nk=EQ$e#zrWUhhhU4JAg*lt z%*;J@V&HATu(TX3E}KoAtsALT!~uci93Fay>?h~)?hHG*x;FU7x~}wRWQUZR?c>?u ziRvj$>Y>&pBbO|AB)f6V(+ \ No newline at end of file diff --git a/examples/compiled/bar_1d_temporal.vg.json b/examples/compiled/bar_1d_temporal.vg.json new file mode 100644 index 00000000000..627b2982916 --- /dev/null +++ b/examples/compiled/bar_1d_temporal.vg.json @@ -0,0 +1,76 @@ +{ + "$schema": "https://vega.github.io/schema/vega/v5.json", + "background": "white", + "padding": 5, + "width": 200, + "height": 20, + "style": "cell", + "data": [ + { + "name": "source_0", + "url": "data/cars.json", + "format": {"type": "json", "parse": {"Year": "date"}}, + "transform": [ + { + "type": "filter", + "expr": "(isDate(datum[\"Year\"]) || (isValid(datum[\"Year\"]) && isFinite(+datum[\"Year\"])))" + } + ] + } + ], + "marks": [ + { + "name": "marks", + "type": "rect", + "style": ["bar"], + "from": {"data": "source_0"}, + "encode": { + "update": { + "fill": {"value": "#4c78a8"}, + "ariaRoleDescription": {"value": "bar"}, + "description": { + "signal": "\"Year: \" + (timeFormat(datum[\"Year\"], '%b %d, %Y'))" + }, + "xc": {"scale": "x", "field": "Year"}, + "width": {"value": 5}, + "y": {"value": 0}, + "y2": {"field": {"group": "height"}} + } + } + } + ], + "scales": [ + { + "name": "x", + "type": "time", + "domain": {"data": "source_0", "field": "Year"}, + "range": [0, {"signal": "width"}], + "padding": 5 + } + ], + "axes": [ + { + "scale": "x", + "orient": "bottom", + "grid": true, + "tickCount": {"signal": "ceil(width/40)"}, + "domain": false, + "labels": false, + "aria": false, + "maxExtent": 0, + "minExtent": 0, + "ticks": false, + "zindex": 0 + }, + { + "scale": "x", + "orient": "bottom", + "grid": false, + "title": "Year", + "labelFlush": true, + "labelOverlap": true, + "tickCount": {"signal": "ceil(width/40)"}, + "zindex": 0 + } + ] +} diff --git a/examples/specs/bar_1d_temporal.vl.json b/examples/specs/bar_1d_temporal.vl.json new file mode 100644 index 00000000000..6cf8fdf278e --- /dev/null +++ b/examples/specs/bar_1d_temporal.vl.json @@ -0,0 +1,11 @@ +{ + "$schema": "https://vega.github.io/schema/vega-lite/v5.json", + "data": {"url": "data/cars.json"}, + "mark": {"type": "bar", "orient": "vertical"}, + "encoding": { + "x": { + "field": "Year", + "type": "temporal" + } + } +} diff --git a/src/channeldef.ts b/src/channeldef.ts index 2c78153fcf0..7354aef1667 100644 --- a/src/channeldef.ts +++ b/src/channeldef.ts @@ -701,9 +701,9 @@ export function isContinuousFieldOrDatumDef( return (isTypedFieldDef(cd) && !isDiscrete(cd)) || isNumericDataDef(cd); } -export function isQuantitativeFieldOrDatumDef(cd: ChannelDef) { +export function isUnbinnedQuantitativeFieldOrDatumDef(cd: ChannelDef) { // TODO: make datum support DateTime object - return channelDefType(cd) === 'quantitative' || isNumericDataDef(cd); + return (isTypedFieldDef(cd) && cd.type === 'quantitative' && !cd.bin) || isNumericDataDef(cd); } export function isNumericDataDef(cd: ChannelDef): cd is DatumDef { diff --git a/src/compile/mark/init.ts b/src/compile/mark/init.ts index edd32312bd9..3f7379cfc90 100644 --- a/src/compile/mark/init.ts +++ b/src/compile/mark/init.ts @@ -1,6 +1,6 @@ import {Orientation, SignalRef} from 'vega'; import {isBinned, isBinning} from '../../bin'; -import {isContinuousFieldOrDatumDef, isFieldDef, isNumericDataDef, TypedFieldDef} from '../../channeldef'; +import {isFieldDef, isNumericDataDef, isUnbinnedQuantitativeFieldOrDatumDef, isTypedFieldDef} from '../../channeldef'; import {Config} from '../../config'; import {Encoding, isAggregate} from '../../encoding'; import {replaceExprRef} from '../../expr'; @@ -178,39 +178,31 @@ function orient(mark: Mark, encoding: Encoding, specifiedOrient: Orienta // falls through case LINE: case TICK: { - // Tick is opposite to bar, line, area and never have ranged mark. - const xIsContinuous = isContinuousFieldOrDatumDef(x); - const yIsContinuous = isContinuousFieldOrDatumDef(y); + const xIsMeasure = isUnbinnedQuantitativeFieldOrDatumDef(x); + const yIsMeasure = isUnbinnedQuantitativeFieldOrDatumDef(y); if (specifiedOrient) { return specifiedOrient; - } else if (xIsContinuous && !yIsContinuous) { + } else if (xIsMeasure && !yIsMeasure) { + // Tick is opposite to bar, line, area return mark !== 'tick' ? 'horizontal' : 'vertical'; - } else if (!xIsContinuous && yIsContinuous) { + } else if (!xIsMeasure && yIsMeasure) { + // Tick is opposite to bar, line, area return mark !== 'tick' ? 'vertical' : 'horizontal'; - } else if (xIsContinuous && yIsContinuous) { - const xDef = x as TypedFieldDef; // we can cast here since they are surely fieldDef - const yDef = y as TypedFieldDef; - - const xIsTemporal = xDef.type === TEMPORAL; - const yIsTemporal = yDef.type === TEMPORAL; + } else if (xIsMeasure && yIsMeasure) { + return 'vertical'; + } else { + const xIsTemporal = isTypedFieldDef(x) && x.type === TEMPORAL; + const yIsTemporal = isTypedFieldDef(y) && y.type === TEMPORAL; - // temporal without timeUnit is considered continuous, but better serves as dimension + // x: T, y: N --> vertical tick if (xIsTemporal && !yIsTemporal) { - return mark !== 'tick' ? 'vertical' : 'horizontal'; + return 'vertical'; } else if (!xIsTemporal && yIsTemporal) { - return mark !== 'tick' ? 'horizontal' : 'vertical'; - } - - if (!xDef.aggregate && yDef.aggregate) { - return mark !== 'tick' ? 'vertical' : 'horizontal'; - } else if (xDef.aggregate && !yDef.aggregate) { - return mark !== 'tick' ? 'horizontal' : 'vertical'; + return 'horizontal'; } - return 'vertical'; - } else { - return undefined; } + return undefined; } } return 'vertical'; diff --git a/test/compile/mark/init.test.ts b/test/compile/mark/init.test.ts index 11556576cf0..67b01213a09 100644 --- a/test/compile/mark/init.test.ts +++ b/test/compile/mark/init.test.ts @@ -102,6 +102,15 @@ describe('compile/mark/init', () => { }); expect(model.markDef.orient).toBe('vertical'); }); + it('should return correct default for T', () => { + const model = parseUnitModelWithScaleAndLayoutSize({ + mark: 'bar', + encoding: { + x: {type: 'temporal', field: 'bar'} + } + }); + expect(model.markDef.orient).toBe('vertical'); + }); it('should return correct default for empty plot', () => { const model = parseUnitModelWithScaleAndLayoutSize({