From acc218398d8cde1a0f8b450a0e8ab588bf3146f8 Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 17 Oct 2022 14:45:42 -0700 Subject: [PATCH] (core) Fix hidden columns bug when editing data selection Summary: Editing data selection would sometimes cause columns to be hidden in the updated view. A missing conditional was the culprit: generally, field visibility shouldn't be modified after the view is updated, but we make an exception for charts to keep certain fields visible or hidden between updates, so that chart configuration doesn't change too significantly and cause unexpected data to be displayed. This special behavior for charts was erroneously being applied to non-charts as well. Also, when no columns were visible in a view, opening the row menu would cause an error to be thrown. A loop was inadvertently using null control variables - an explicit check for non-null loop variables was added, which skips the loop when no columns are visible. Test Plan: Browser tests. Reviewers: jarek Reviewed By: jarek Subscribers: jarek Differential Revision: https://phab.getgrist.com/D3650 --- app/client/components/BaseView.js | 5 +- app/client/components/GridView.js | 14 ++- app/client/components/GristDoc.ts | 14 ++- test/fixtures/docs/CardView.grist | Bin 0 -> 196608 bytes test/nbrowser/RowMenu.ts | 82 +++++++++++++++ test/nbrowser/gristUtils.ts | 7 ++ test/nbrowser/saveViewSection.ts | 162 ++++++++++++++++++++++++++++++ 7 files changed, 274 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/docs/CardView.grist create mode 100644 test/nbrowser/RowMenu.ts create mode 100644 test/nbrowser/saveViewSection.ts diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index eaafe193..0c35ac0d 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -339,7 +339,10 @@ BaseView.prototype.getAnchorLinkForSection = function(sectionId) { // Note that this case only happens in combination with the widget linking mentioned. // If the table is empty but the 'new record' row is selected, the `viewData.getRowId` line above works. || 'new'; - const colRef = this.viewSection.viewFields().peek()[this.cursor.fieldIndex.peek()].colRef.peek(); + // The `fieldIndex` will be null if there are no visible columns. + const fieldIndex = this.cursor.fieldIndex.peek(); + const field = fieldIndex !== null ? this.viewSection.viewFields().peek()[fieldIndex] : null; + const colRef = field?.colRef.peek(); return {hash: {sectionId, rowId, colRef}}; } diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index 0f3d0225..d09bdccf 100644 --- a/app/client/components/GridView.js +++ b/app/client/components/GridView.js @@ -539,12 +539,16 @@ GridView.prototype.getSelection = function() { rowEnd = this.getLastDataRowIndex(); } - var rowId; - for(var i = colStart; i <= colEnd; i++) { - let field = this.viewSection.viewFields().at(i); - fields.push(field); - colStyle[field.colId()] = this._getColStyle(i); + // Start or end will be null if no fields are visible. + if (colStart !== null && colEnd !== null) { + for(var i = colStart; i <= colEnd; i++) { + let field = this.viewSection.viewFields().at(i); + fields.push(field); + colStyle[field.colId()] = this._getColStyle(i); + } } + + var rowId; for(var j = rowStart; j <= rowEnd; j++) { rowId = this.viewData.getRowId(j); rowIds.push(rowId); diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 7f6177a7..5ded57a8 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -666,7 +666,7 @@ export class GristDoc extends DisposableWithEvents { // if table changes or a table is made a summary table, let's replace the view section by a // new one, and return. if (oldVal.table !== newVal.table || oldVal.summarize !== newVal.summarize) { - return await this._replaceViewSection(section, newVal); + return await this._replaceViewSection(section, oldVal, newVal); } // if type changes, let's save it. @@ -1072,7 +1072,11 @@ export class GristDoc extends DisposableWithEvents { return await invokePrompt("Table name", "Create", '', "Default table name"); } - private async _replaceViewSection(section: ViewSectionRec, newVal: IPageWidget) { + private async _replaceViewSection( + section: ViewSectionRec, + oldVal: IPageWidget, + newVal: IPageWidget + ) { const docModel = this.docModel; const viewModel = section.view(); @@ -1108,8 +1112,10 @@ export class GristDoc extends DisposableWithEvents { // persist options await newSection.options.saveOnly(options); - // persist view fields if possible - await this.setSectionViewFieldsFromArray(newSection, colIds); + // charts needs to keep view fields consistent across updates + if (oldVal.type === 'chart' && newVal.type === 'chart') { + await this.setSectionViewFieldsFromArray(newSection, colIds); + } // update theme, and chart type await newSection.theme.saveOnly(theme); diff --git a/test/fixtures/docs/CardView.grist b/test/fixtures/docs/CardView.grist new file mode 100644 index 0000000000000000000000000000000000000000..a0f6a2f42caf71b17ce6fc121a33b95287a54cc5 GIT binary patch literal 196608 zcmeIbdypJQdMDOBGnf}JXpmfT2tJzw@_<cUSjdP=JU9zh3U3yE5ymFMr=7Gr#;Q^YT~D#Z6I;YV~c_ zESrTRg<`SrdbwOE6sGWh4gc?artw8y>wJp%GSm4n;@_#lD_{Qo11Ov*{ZLkUvGfpopg@=Z>Ja!SIy|#z+r>|7O%_zd=Cyj< zXl`!E?~To{R%vZlv#ym^FRiU?td%!bPM=#VFJ)C(DxZjXdHur1+8b+^$`>!KpI^E3 zZuzaXcTbj=c353ho9le3TYhHk?8=pM8)dCqda>46DqpyAe(lowYPnOa3*Yxq1)Ryd zH`czk(JecE90kprJ0k1)jhJ5(&9`@&ajgov!#Bo_voMHOg)Nm&zy0>PwUrB9!jdOx zEvj(%&I$`eC5vRM+Kk1jKYA?P#51*Uy&BcpD%$?FklDy~;kZ_EP|H-j9UH@hn0GH_o;(3i*MP*-Ich?#fM1)8#f!97b~r6 zarF#qvdvaC{zgk|ejsjU6(4G&aOJ}KSFWtV7SF7Gt(;XcMWz_|qn^6XPnPkt z<=dC%m0$UIJtd0)=9Jd;Upv?*mn>GwC+5rT&nWkzicg&?U#zgWT8^ttaZS|A34dh{ zSjsW)66vs=xP%LDZ@}lAJ9jd9gd!rrOy7a#WmfJywz(xF1l=cGXO)&HpFa2Y>8?jP ze!Sg;GM*soCv{k(Q43?%6uf*RZf>cPG^OR;i*qxJx?a5f)&Qz!EENxQ|h8&y$XuQr;j8j5v3Sg~Di`S!_~nZ-BW zD1P+SfhMhl(pSHU?xveBps#TXn)b05?njrth^Ugv1jb=HiCygz6a0jk;XQvmp z-zXN8$v3`H!ANeCwVJiW1&jq~Qy*JA_tEPIW)@$2t@!EUI7y74F?}cK?+5pO<88yS zex`h4sSDxK@&K%Fvc^`HL;&lG!o3WC8-9Skrh8*D5=qIUPl~t^PNo)>lWOu?b1QDN zrPC#na0E^12z8JMx^86#cz}Qho@#1UAOqjYnd9c*=UNLL48mc4~*3YRb_8o=;DzY zF7|m*t>=r{s(<*I?QXCGZ9lp{HG@uO?hYZ%{*t)cw<*cxAN z$dSOnGdl9U{qE$<;;B=`k43gXx;<*5?o(sMupbF@HGF`zJ`rG~GN!oI5cPrLJ_&Kr z2*;Jz&`;KfB$Q^L5?4PMTVX7=yU!PA7GHk3xZ4`QR`!&Qo|>zl6C;pSh5GZ`TxT&V`?EGKnM^5ga9Ex z2oM5<03kpK5CVh%At3kWn=Ac}1mYFBH(%-hDgEoxAD8~u(*Iogz2q`sl#Rf5-<_B( z>@Lg|fBvNIaLY8A7P*n5>!BGi$8t>16(Y1e;cz=-w#IEO3QdQ(zR86ZSi&|vZbvo` zjEP$<`5s2HuZG;Wru900=B3)2>(az-H1Py}wdDSdA*+N}@ozy@5Y_9VQri)Yt8zcb zgps-kAo(T_f8&cqA7jTXJu+YlWTjC;t84yK!2PlADCI9!1gDu|wbouVc$TwWY}` zVQpJ$$#+(gjd+snO?t4|!f;)#+guBDC-gL7FmBjp#C%OaBoPnoz~!beEC|PU13l7o zO)%599VgH&#jV_DO7N@FI(kjLD%oru!k_as9!GNXJ?x06>Tk$B67`mZa3w)R?mehl zu^~QeYFR|C>|j?9aY=-=I&XaecP874^zXN*#CE3?_h3uvLw9Gxv$=WNgPbg0;k?~G zsR9W|aiV7ms+I)!Y+P@v4EFMCvC8FIy{RfC>ej0AS|W&j!#^omsNU3ig1alshso>* z=#9I4d>_0ZBDqk?cS&s5u6Oii0N2(*SqpZP7)4^gN%Kd^Yn$f;8RYd-xg$h`+RTPMak=ZyOqYcP*CWqU7^-_3i=^;D(^nss(jG% zfunr9lA3y^^7gSrEtY)VQZ}i!8cS?gjuKJ=Ou(0aPb(WlG1tq}ics@2ER2TLjo8tN+yCynZ%U(ck0$FV`xo>7qZ+B4XZl9G>UxmVp8zV|RQ2iAb5o=YN$yF||rZp$2CuNan$y?&4!Wv^xIWCe^ z8yFB^tOZCK%9CAvPf<`d*uhC@Z!>OIgwpmQ*oLj~DNs%eywjY`C>9m2x?9Ct&n9mv z#$b+kXi`3)#OZdW2y`uCz!;SFtHLgwA-<)6`vxujiiFMR+VIQk{mx2KP7R?X#L5pU zOp`&xQ2!xW8Xjb1KQkGT$Z@!IbA6+dl&YvlbaoFPY5xf=+1NfhDfR9d<%qidOm-}6 zMCBCWp52V7n`+Fk-Y55Dg0O!DOMF)jkpn-qW&f#T_;T_&{CQb&@x>?b=a`&HoscLU zm)*!;lb+|L3HkM<1AkPQ`^N|V=-?k8{JXR7&VA$Hip{bcjte5 z;lEw@-r>cWzdo~a`2U=JY+-GBdG0R{zjx@@=3YM3nEus+SEp;!e{ty4;s0#*@67%y z5Tp+wKnM^5gusJrX!^31h#K*+cUi| za+wzT7IP6C)n8OpLId$o%e6rv(j$ipGcfQ--vKk88+ewb1%42Q7Gr^9F+}Hd$G2P` zaXecF>y9ZZf#WzDNI0P$MV=Riwq|OM#XU`nJOouk%`z=5G{Znxg5h6*hB=7;1(C2q z-(O02ahStR9yv@GuH)OjFaVWrd5)_ADI;)9%eJ+^&~(?aG|LJt-}adAgl@zPJ9Kzb@@^$-pMMY;k(CsjEB4OxU z-Y@wyT|M*+JA}Xxj&=ge2<=EjrZ62Z@VRMdn(4E^w%z9x71!`w2W^Q04J|bd-He21 zyP*}CrYk%@6qY57FcKzOW*hkkjw2Q;5jR07j6nA8IUVhcdiMQ}L?xrK|q9{Qop8KVDCKhKY%z_UddD67$= zRYl>^1Q)?W-!!bqXWWFEOBD_ssE2Qv%n#krv_ey}G&|4$o-UxDrX9f)BQt>NuqP8L zM&xqXEVN07Ktw38=3v>r7XU#aIBeDNBP$AQON1J;+{iafjYC;~er5q9+s;!{?D^8%%&j!$L4! zz->6T=NmASD9{ZbauP0xYcPKS+caH2f>pa7fb-4A6%`W}=h@Ju&Uw$ zH9z#Bu@Mu1K)|rU9t3DX$1Dzje9y7JkWh(yNfzFRMa+f%nebSe1Jevc0}KIZG@1w8 zfl4~A9(ZOTG#g$BO9gNek0~nfO{UJ^j(~`71|dXg*^bUd0I=bVpzJ0nz^JXrfrE$) z--D@oP;$dD9rsa11(1PP#%zc~(E%4M3`5Pc0vDdt^nBa!ptWd;%ee?0Q?3~aE#M5g z%O6R2;gB*stLAd%n_lFbg6V!>!)W1~IY+BJU+SUjTUcj;D~@z#@c_E#+P-5RNvPON z6P^nfV_UFzGq82t;xM?t@B%NeAX43hzMwmR8br1iIH6tA5>u9hsUw7dV%}@`F!wD|7=V=hE2i*^C-O@uC zGlU%38m>s^;&W-ruvUWw@JsNZAvdI-_e|5W77{A(Cy}Ka5iHL`<0B}%aBVmQ36`z- zEYcjVGpzNX-@+oC3uADMKm@Q}IKxAViZ0MUgpR}gAi^z%2939D)8)cIyWn`yK?FK@ zlbsB-lWU;^6&BD!>8$xdMa2ybjR{lwP@l;T2PPo>2>K25um;-VhnA~35IWavro)fH zfASFBk*2Z0npae8I62P>12olloDg_GZ(#QjE8MCDTMFS_(fcqB(l!EVMDQa#k}lTt zj5$RGkT5(GH%Iax2mk4CpCK&K^fkkP-;W|!gGUO%qX!3P14I)RCxLhM*@PG0LsO(+ z(MVs@Y+aZ>u<>;C0v;SDKm=vihtqd4Y~Z?UF?1TH6>yYAGl~iexNA7NjeZoYy1pK< zz;NsUZqkOAb3$}39@b7nrlHB`>d}QeJk-%wYH&r~w4x%=O?W=@Z0W|}Ah3exL$0=i zPK)VSbu|K8pwM8jLV>5j8+!1;SpS0_aPNSk;&=`d5qd^bhc}IE7tbrT^(c7_Jfzga9Ex z2oM5<03kpK5CVh%AwUQa0)zk|kcq&-;#A>iv9X0TY5o#w%)c~OsfFy3$*ICiMft6Q zGn*U8270M9F;#e>eMw!A>;IFb-z^~L|94A2%%n~i2?0WY5Fi8y0YZQfAOr{jLVyq; z1PB2_;DJQo$YlS#fAa8Tde%QNH+gg}2^Yxs0g9y`74RQ@2mwNX5Fi8y0YZQfAOr{j zLVyq;1PB2_;GsicsxVbpSdjDog517?Kax4(E?Kf2bc<8=2|zsl+jcH`EP+6_=F{lt=} zp1N{*>08PC|IZ4gKl|}&A#qCx5CVh%AwUQa0)zk|KnM^5ga9Ex2oM6}5I9tvE*zbk zo!wwTMd%Bqzlr_-uNO+cUAkWS)x*D3Dvi6B$_N2MfDj-A2mwNX5Fi8y0YZQfAOr}3 zdyW8RrhhnBEIq!vHoH)o#b;{u3~P#8OE+SEO*FBFy&*nqp00hkbaIKK;B2kF&E(~G z@i}$={HZf%P!cy*Ta9LITi)8K5wS``oLpj+_*!)v$Kv6hiiny^-&(~pugw+R#~0?_ zbLArsT&u^8=9{9jBkG$QbyjUec#5Q@3SN=lUTNOk5v#T8by07gcyaY)`FM|k%z;6})$>a}<|GZEy z{rSCPy!7YAUs_yVE`IVsQ)VD+ssPF6`t}Zj6pj90M^-Pbt!%87H TPrX17v4T{ zXhzcc=xUnInOeABjcVcV`QoMZ^DCF$Ex)z)?#c2J z2Q|J_-dOwEM)}Oz*_A8jHp<73qhLpfdLyeKV2uw%GyA4SxFxpPJGe8hRhPPt>=4#K zw;6AXZ`P_Ji!_))l&9LfESk-@dTsde%OC%>!!wJvUHt2{G$bm~qp=y*Dy{8m8<@Up zrA&DGs*D3^htQw#GIDO7p|ONyR^RAMN${O@1xP+EE_NxSj$2p2>`ym z(*%ygFo0s*kfFX-g)Nm&zy0>PwUrCqhf0)X3{Mpf3%0@nQOSB{tJ;ha$UB3 zNwqBEjn?)ytKYm_Yt=(}LpCftZtPUp&5`BT<3&<=yueW)_bhFW&y?3`I!{G7)c`wy3XMSpUkEHQ4f*wXc=4DyHa^Yt_suCwA0R*ZIjZp0<4Z z^1Sj(AFromF~FSCy8dei`;;$>mGX)Ca{DvNhpXaKr^**AEUuR0DkAryUQYNcbHGxL zd6!6s?Zo3>czXjr>)g4M$s-gI31<2ZG%vGq=dsN#AtC5K;X13dMEUf&w@-IH((&W% zCY136SwE@68jV^QV|{^_Z^X?lRg$K(ynAtOW>MFRx8E8-^=w=by-rYK+PN}60?|(S zo_Lm5-&|XLOZAvvDr?IpyX{gvghv9;p~WpW>np?&28~Z>9m(w%7Nmbynd$Jz$)7o zNvjh3>D<&+orLMKtk=DXsa9PQQMc#pXoCC~&pQY4H=5<7o;0@P!5z5Xot<9Xexq1W zCg1o*C2opM)@s%g7cdr}O?_wK=DA#MXrXp1r0`93ztFGuG+CH^oq+ZN?ba-lJ(RGWTyq2jEn~kQ}8Po@L@xWMZP*wKUg)SaTz2Rn` z7u9;cXek*t_7!8)-XkH65MzdZC!8u*4U~hE(N#h@62#bQbd7oq zxbhnM$@-9l(hOAM>IY*hjKy~M`Qpst%P$vqTLak2p0Yhl?R2b-i4n-Ey88e&=F*PU zoTS}}bX8UjRHPnst}K6PVtVl_FYj&L3BxIGuP#htK0hnx^tAqe?;;ySi4Y(J2mwNX z5Fi8y0YZQfAOr{jLVyr>P!U*|UntBMepEOtX8*6*r)HYd|NXRg;9FC_GtnsgsI<25 zmxr1M|Hb^jm@m&inQnm!9pKJ?rq&;hrD7zT@8GQpL@VUVY!VGjUwNi4Qqoz_Pu((Z zO>xYsZ4%HMUd>gEA)cGDe7Jo3`jaz@D=Wp1Z}hQ`Wy!{7L#PPxW)zDG?=)k$jlu^C`u`GZ5-lAF!{LXu$ zflt<;INuB{NHbi-@%<&99CYK~Sr6Q6EzQ zQBD~~9Y7Edt&Pro2)9_hxiKPh0x1WzR`c?XKy-DOY9g^wr41zLARhs`HkebOQetGl z^>RJDoS4o?jciv-2xV)gtg^xC(9UtS#C9OKjPMS$V|n-Z6Ell1y;QvY$gqB;^y=tn zeHy1nBVEn@5ntZ*9#1$m)0|#p*E%8QP4)fBOioqNI8Lv_dPa&tE)iCn%1>ttVFz4{ z&e~btJ^BTeL9+XeH0v8IIL+#9hnf6X%w#?(yKC<8%yGOu`&bgZALMPlDYiSk5;#yl zpBTlTDjmlmn5c-60FOq0dH2|(GmEdj+P}E435F^n@py@r_AWd=ipTa1_lL`ikEr#& z+h0}=z!|i8}%q)KCOT~}Q zXL8>mJ=oOF^+(4ytW#}V+qPz1zkKbyS+76y4f3 zi?j0yQJATv5YV-Ht+g|%Fz#m23=44g*t0W>rdizG9%%n1(NOt_spfYtl}EL{TVrhF zrQ6ErhS7}98u@?*E`K!rjPlQ)tPXP7>MpKI6}u#VK0lH>Svrohdb`cpI)oRxx8sJ) z#08Pek#|>RhHysPSXk+%m=3frDaFtQ>9*OVdpBRiw(Mef3kwWn1E*~fODAev1UZplpJuD0~9JeMtwUWF`)eDq{VrE2F* z`Oc!hU$X@IA_?;B6Xedw>yX!CX}0ZKPKXPiFu#ueD$U-)PmEGUXv7 z-x$dZ3`1@V%!g!v+trPu3L789J8?B`!qYc)SUBnd)v(5q6P`tWi#6U94D$>qzz}oG zwEmxtYtVN>fDj-A2mwNX5Fi8y0YZQfAOr{jLg3*EsxUW~%>Vygq4e(_x&~5%2mwNX z5Fi8y0YZQfAOr{jLVyq;1PB2_;6U*}VQ#iP|NoOh=}!m}LVyq;1PB2_fDj-A2mwNX z5Fi8y0YZQfcnA=93^V?t&n|4p13UCOyRj+Hh*>E8d||3|wouwB{Xyv;;R}5T0YZQf zAOr{jLVyq;1PB2_fDj-A2mwN11Og|f9z`zw(U*&J3+gjFJG;=pd3+VI%IdsaW!K|t z>a1m6E~=V~?V69snspK4AaFG==U3muBVGipg<)D8MRTfXX;M*nK3#{4CXO0a&#T0Z zCVz24)j8IwGuP$mvf`{9f;GNS`c`4Kv{@+q+tR-({Z{GMO21V48>MP#vve`}--rg& zWkP@uAOr{jLVyq;1PB2_fDj-A2mwOibBzF|)1_juIJZfaq+S2{U6xiFLZIWtlEAA6N2rl%GT^hzfVOfF2Neojq528ZX`Y)noa znwT$ieoagsEaKN(!r}z&|NpsGS86;VKnM^5ga9Ex2oM5<03kpK5CVk2=L7+o|9?&d zP0b_(2mwNX5Fi8y0YZQfAOr{jLVytXTq8j9|IfAfsqusWAwUQa0)zk|KnM^5ga9Ex z2oM6F69kg;|Ngj8`s2?@gQ=N>03kpK5CVh%AwUQa0)zk|KnM^5guug#z+&+rb^v&6 zVRm6*X5q_M_4j`IPm{(b^Zy?eNWxPy)WxGo%@w{KR~wHO)7Rd1yw;* zuZv1;M>MV$aVcS>F2Z;x7S(2QQQdWH<=i< zRc(moDa2};t!dP&#MKYhdHe{zwr23B*4z?xT)@|at!BI}t`#o+rSb3o-i_0WRI4HC zEy?!RSzUba`ZdwKR*xIaSHjx1);fagJGfqLuJpqe2wi9)W47R)l1BvLgkh+8R^WP? zX?nhGc)BBkz;Zbkp<{-YCxjMo<~ockZW{6h^XC6sR=jRG}g>~eSOw-9rcEEI$ z(%w(gX&nceD^NyWu8JFFi5tGQ^{Cvem9Imw${Q@G2>nj1ObKU3(2qni<0KxZxX~&y4{7&*HXbn_;9giw9xgy0-6_X)BX>0ZdWUzZ&!AmNG77r(8s=RcR(yIcl|L@h5Q`@IBH9m8g>|Z;6`;TXFSTMX0Jx z33=CE2bq*sak^FcpvUM_xRT`yHltWn_|-`i$5N5v1M-jbi?`GxLuFJ+xpJ-DntdFn zDyUlWPpSbIYmKZ&r2LG6TGCeVToT*0>v!bF6j^1{vb}xx?Z3&{en}#t9Lj24z`06S z+V+=9PKmH2Vhtsp3)&~AjwEj6yO;86W9dky8_7#{z>UZ*OIhz{+5>}$u|4q4?`S!D zV9jQl@LZGow#7}u3~XJuxX+xx@B%NeLPzLsWJQ9zksU?07dW9~@W2gyq3gbp_6^cR z2hDD%2bNlqI7g)=w_5V9)wpo{SKd$Qz9N;~$BrQI?ce=v5ZFgAI)aX31o~R)>T6Df z4wtg^H7D5teJx2%xu4zM9gP^l#8`d(xu49duiP`VNDCv5QK1#OmWF;UFs%^(3#`Bl zEK3*~y0TC>Q55*DqZzjD>#m1!eW(Yy^i?|KuD%}mjs5Funm`_XP17kzMxd{UJNlXz zp@qYl`kI&QfW997vC!8aeE-#){xz^YPYZ?Zu}}|f-O@v5Y9`krTf-Frb@jO;EFnz8 zU_oHp-1b9mcwQKKrfFGu{Hq*0boBMxQ$N_hzV;Ew;a~gcNJca{8k?W_VxhEh_|t`- zJou{zOY@~UHv8Yq{O`C(A3}f-AOr{jLf~_cz^?z|%;NHL@%AYd-)ILc&c>A{(&+ks zX{(pkRyNkk8!M;Jt(BMh3zy0#VqRXqu(9^W+NJWvOY7%XF1=fRYwg{Wcu;k zy0y-%on5(dZlkQBP7?LVE{ha5OXZEVuWgh&HyuB|y!#&=n^}DM<>Ku|n)i)*eC=9~ z`OeSfOzu0S>s>eHgIui zX7T9J;_W%b!-fj_C4XP-Z%%S?Tr0L&wZ$rzYxU+*`NEa+YnRqnyIAVN_kHlXGSGzM z$8quWaD8og_vPnj7G1Zv+ftmhbzx=o+rR;RYS231cnUd*t0W>rdizG9%%n1(WtfRp-45qd#OCC_1zj{8!vS= zgO|kA*LjAve>DA!GUrcL2f1uj`Z$z7pC8GcEFH&Lz1`*vMFDd=qP`tB8d!V-k&Zj- z_#Ek|cDgDvgfmK6SUFp(Z?`IJL@E40YrC3Fx_9$MR&R)5LH6^vAwGmH56Bu->OcVD z-(j=un7LX9YBuX}&}t63Ex4WAYfsNC>U!~GyH60!CJVQ=+rmRfR#1xt+2wMH^2$!7#;{Pc zR6c`#abx}bTGyjW=fAw`JvFm<>QwReu{4gC)-Imgd=rb8Z4|q|pUuRuTRaZIu*Ne% zsI{6qt>!325%qelo>{oO`yU)tD$vjRJFF68wNn~LoB7n0XENDORT$TRXtdE(M|eYp zDb`~$7DZ@at8WzTNtj( zb*!`ox)XYuFc>#%Gh)6bAd-lOcHnYT7#4)%y8+f?bxknSw;dd@qDMZcv| z#oFV9mV8J{`u>=!46~k%ss5qo4-|mP$H+0k`R*hb17vM9N#*hSDQ)y9bu4H6{ik!@ zzq;rBDfpBV>9+;h-u5Uh@dNDzad{n+m~B?SDHAI?d3Q2JsF_UKQqT{%Q+aoPo=Ym{ ztmsi|iwZy!t4RKm+G+Etn4*+UxGYHQNazq00y zm?K;6?Hx7TP6*)YV6%q=)UO0GKcX{Khp-!p%cLup8yS%C_qdL2>iIG0rgS_Ok zr(>sCy-i>GZzvP6Q5Tc|WfX8^`qW6SFJhk7;aX;2#8o*_RCACRKa+HT9aOs8XQk9v znnN#TQ%d}n>Ur*AJ{i(!WuB;PNduucjI{tsLwT~R?+vG!DeY~>O{Ad7#tuni*c$I7 z1xx9&a-LHE?jq}KtT8A%?|;M)-$Q9T{fdMLPFs=W^?qk1DW`@|QgM_YRG22&Xvz(x zR6EI7lh5qSR?B|onDn-ks@$2U8}Z;wqa0DUpUIAeji{U=+_RezbyJNQ*8AihnWT1C zIez`pPkdJnk%JA0JTv!7KB*-!!mwd&d#8C*2}Z5|PfdKSu<*wG&mEY@7y1wa_W*%U zUY%~|LA}EQ?i!A6o0c8$-u0z7R_`9W|mbO-=jWvhH^BWytp zVAL40H~CB=LZ_1=P)}Z3Q?rQNt=@y%p}|keYNd~8vlc&r?{5#Xz9y>|?|I~z4Shz6r z8&iLdFZ3Y<_CVm1mHD=_`=%Qj8WSeN`!YTgEHr(AZULJxxQ_6If%j?r&~oAa<*PO} z(|HgWcxNmOBB5z4u;%77on2nC1I|u%77za1{$IX%GUrUmHci8`!T_%=`HmCf-6qU} z>@dR1*1yU?&pRCMvbT}_+vvZj`oR{o?4$F?_!PnvUw_nbw!#4yWl1>hSkUvxyG>ttaxLHZz;4!+FUL=&)Qde(-hpm;Z;nF;?66&>hNGp&gC; zh@0xd^nFM7bSpwyAhy~uUFfyz2O4)hhGaF@VvebsR=`n~J7Tq(3NA@+n==JXlDJcf z;{q07OM0>U5vy8F*5N=swd|iGR+Th)4{pb~5;^d1zw$e}JCp@&g8fIWGGsH7NGgtymI#`D*Q1Iwok5TM$*VK%QN=rXeZcfg zrbo_8cEBUcc=m(uk$=GRdgR=Tg;J63a@DIh3_TsYO7Wxy@3n(jN^{c$aOYiXZiPLp3bgFVAuk%NlW8g{g- zH&48{da``{U3^ZRKY!}Xnd8g-BeqnM=$L1v6GM}%M%kQh5~$k7O&Nt6&PbRgAN?)g zhoJhCi}N&2veM5}c9`|=$0oYcmHAWqYu^rYMEg%ZpLILjv=eu5YPT znlXDb{_=mF*MsQ-;mgo*xF1AtSB8f9nq`|V7Y25BrcjbTNfHfJV_uP#7mV16QWG^bgurD3E^&jHt zx0aG?OFywBr=q88A1<9-LejL5?;^1#F85pxO5(<93uCQq=@mLPBD{GnPA=iK%4=2m zW-acih^V>rtyFX|r=RZOG%#Zu|ML!4>$B(N5<%6u^<`&_^!KuQ#!Nf|c2pcSPE>hA za@()f+Rn|h_1gA6d~N$RgZnPjpNS48Zt@q;TMMF{U1zyyYT45bfI{(I5m;Hc<{pezkyl)k>Ye= z4i(jwTTi_IE5{3RfqJU=pBE1P+T0hX{s>>_^FSi-srN(@5dYqm=gQ1_FT|_aL1^JH zAiN~6VG&)&Nh_XiaE_HLhkL%Sxj53z)EqsqEf1;Rf(M!>qQ{@e42b6?I}i|;FKB-D znGZ6KI4HAk+$!?e(L zah#8V_lo?`kML3eb_BrLrFa7sZqUU!r5X<`&LhY796P-_KagKEe*GrP#ALw#9`1C- zMH5?`4{vETv^+1ZqJKxZJK4iZE(*qyM! zGrUGNjB&CK(^)y{TZ`F$le2Sp)Y6S~mV2F?n1}}_Cm#Aj;zquEB(F9;_JvG0l9%j& z8<9Ttv+sc)lU$6;-1!wJXAf*fzU)jr(_;~HO;g}pFQGX&!8iCv~K{j31(_FQT-adym#kavs{t9r4~%@xK~rpmKHGbJmAMV zy;@!X9(^oZ9rNNU7sY#tWJ(bpre5v!f0$Qub01co%9+T~t0mg` z`Kk6C{HG$keu^Fkdz={97R7R+z`~FdC!Cs?=V>}#@e51?({j%=xO^YpHnG{A&o%5v zuX~{r9epa(<>n@N?x)9 zwsQ1-+sYlC+x(>DV$5h+`-9hV+JGi_=p#}U`HY*E+{G!9p+Vd5EtC178zS4?)GW;o zP}9%_CeWshefJRF!t|Q;jh63)4N$xadzT|0Pj2`P;TJURJ7=EkjO_CYYw^iU)5%MA zz;u+-K1?I~^6%xgG9z-iZOZ*ubU(y7KmzamV^>Nqu#HFvjy}clBMUFVSt8WrR;<2Z zYCIGXwoD70J}b*HvQK(~dpxpFx)F+3iBKLmpdw>ANA~^kvz#OQL_8O??;Kl9+{l0O zlUhz~d}1-%jpQUd;6@f7h#pw_;Bh^0q4g6vd*Iv)ZKG%XYASn=!ltQ(C`DF|Kl>uF zW7lv7Qu5UZ_QxfDoxJNS$F{uB}F3!;%Qbne=k z#|%3%vH7%T;Qe(=$02`CU>Vq;T40MR)A6wEYhpuN6Z@*$w)^aJnJzRh*#Q^&9C*LK z6aM3P!e8Oa;h+8=Kb6zlx`yXsYb7tjcHu6T=yfv^9+nQV6O-u*>}xNuZy5F@6sGNK zRup25(Zxn$I<{uQd%di0eV#^Qg;32@l4U;$NAIwzKJS%OkY4|DymHtZi9OpHiRIPC zXP(V;BYDXVxDnY4+*dbpN3jqvF?J;O`Y*ka(>EYPKw!O)%k3|9EDq|}I>8b+xs?TQ z=7!}**s4TFCIR-ac6Hytb|yaJxJG0N7eB^j70^iRUigF0VkD*{H*O1)PB`@*kHkLp zo=LpK_qOut<{ z-7mRin~tRSnVF7kTWd@jk+j1D#qSmtzA?W$^U2iT#8>)!76^P=JeK&dpZO;8#F=fm zwu4ht^vK}?scKk%_8sBs$ZYj2OAGt}=d3UmI2Oa{^}2(Eav$g1+ww5`rDK^sEHBvs zA9f5VJ@}J^@Bib=Iel2*IF1HUIC7=ble>Ruc&7qqkm1NNWEC1AQu-}23h$~e_<)p2J(^}uz@Au^WfXS#Uqz;+5nD?aIpVfhj&J+I&~W0128{%p(DH4M`8a90U4!NHc*Kh&79gl zNE@W@k@gQ#?&n(jV00a1A6}gka_U+{!jnxVotqxa13hXMiFPo zCifhnG0)eL;D;AWaJ-3ap~Lkx_<9^w%WPA}@u(3dg}Pxf1LvNY?!EK`M*)F$r|59GGpD$b}M!Q?7x?cScoY?Zi?8| z{du7B+3UMn*&LG?`TC#aT`0`G6q|}wNKNYLrurovU#lVND=KTKkt52muh5x#_8#;9 ziJ5;`Sg`SjK7;@vKnM^5ga9G%fFkg{!b`~j@)tiu3B>;I}2aqi}I85>z zJKLf+%B$rFTich_JVC!Fd!%cB5#*dwsdve66s{05jYJDQtP%?yFAfIS(!vZigUjP` zZ0-a$l1*&9(H=%Pt2Oj3hWS<0cebSrDGjFR-c9DO%F&XsIr>4Ni;9dN-f%63Uzy?a9i>Keu>0om&ai-?*()dTav9?Le z`big~Ttv@{skq;3O*!6q>r5_{cnZRzr`Q=5CVh%AwUQa0)zk|KnM^5ga9Ex2oM5< zz>h5g3lj(AQ2?Ej01g%p$U^{ actions.contextClick(el)); + } + + async function assertRowMenuOpensAndCloses() { + const firstRow = await gu.getRow(1); + // make sure that toggle is there + assert.isTrue(await firstRow.find(".test-row-menu-trigger").isPresent()); + // but is hidden + assert.isFalse(await firstRow.find(".test-row-menu-trigger").isDisplayed()); + // hover the row + await firstRow.mouseMove(); + // make sure toggle is visible + assert.isTrue(await firstRow.find(".test-row-menu-trigger").isDisplayed()); + // make sure that clicking on it opens up the menu + await firstRow.find(".test-row-menu-trigger").click(); + assert.isTrue(await driver.findWait('.grist-floating-menu', 1000).isDisplayed()); + // close the menu + await driver.sendKeys(Key.ESCAPE); + // make sure the menu is closed + assert.lengthOf(await driver.findAll('.grist-floating-menu'), 0); + } + + async function assertRowMenuOpensWithRightClick() { + const firstRow = await gu.getRow(1); + // make sure right click opens up the menu + const toggle = await firstRow.find(".test-row-menu-trigger"); + await rightClick(toggle); + assert.isTrue(await driver.findWait('.grist-floating-menu', 1000).isDisplayed()); + // close the menu by clicking the toggle + await toggle.click(); + // make sure the menu is closed + assert.lengthOf(await driver.findAll('.grist-floating-menu'), 0); + } + + before(async () => { + const session = await gu.session().login(); + await session.tempDoc(cleanup, "CardView.grist"); + }); + + it('should show row toggle', async function() { + await assertRowMenuOpensAndCloses(); + await assertRowMenuOpensWithRightClick(); + }); + + it('should hide row toggle when mouse moves away', async function() { + const [firstRow, secondRow] = [await gu.getRow(1), await gu.getRow(2)]; + await secondRow.mouseMove(); + assert.isTrue(await firstRow.find(".test-row-menu-trigger").isPresent()); + assert.isFalse(await firstRow.find(".test-row-menu-trigger").isDisplayed()); + }); + + it('should support right click anywhere on the row', async function() { + // rigth click a cell in a row + await rightClick(await gu.getCell(0, 1)); + + // check that the context menu shows + assert.isTrue(await driver.findWait('.grist-floating-menu', 1000).isDisplayed()); + + // send ESC to close the menu + await driver.sendKeys(Key.ESCAPE); + + // check that the context menu is gone + assert.isFalse(await driver.find('.grist-floating-menu').isPresent()); + }); + + it('should work even when no columns are visible', async function() { + // Previously, a bug would cause an error to be thrown instead. + await gu.openColumnMenu('A', 'Hide column'); + await gu.openColumnMenu('B', 'Hide column'); + await assertRowMenuOpensAndCloses(); + await assertRowMenuOpensWithRightClick(); + }); +}); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 7856b132..530ce88a 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -383,6 +383,13 @@ export function getActiveCell(): WebElementPromise { } +/** + * Returns a visible GridView row from the active section. + */ +export function getRow(rowNum: number): WebElementPromise { + return driver.findContent('.active_section .gridview_data_row_num', String(rowNum)); +} + /** * Get the numeric value from the row header of the first selected row. This would correspond to * the row with the cursor when a single rows is selected. diff --git a/test/nbrowser/saveViewSection.ts b/test/nbrowser/saveViewSection.ts new file mode 100644 index 00000000..57a8c3a6 --- /dev/null +++ b/test/nbrowser/saveViewSection.ts @@ -0,0 +1,162 @@ +import { assert, driver, Key } from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import { server, setupTestSuite } from 'test/nbrowser/testUtils'; + +describe("saveViewSection", function() { + this.timeout(20000); + setupTestSuite(); + + it("should work correctly when turning a table to 'summary'", async () => { + // create a new document + const docId = await gu.createNewDoc('Chimpy', 'nasa', 'Horizon', 'test-updateViewSection'); + + // Login and open document + await server.simulateLogin('Chimpy', 'chimpy@getgrist.com', 'nasa'); + await driver.get(`${server.getHost()}/o/nasa/doc/${docId}`); + + // add new section + await gu.addNewSection(/Table/, /Table1/); + + // change name and edit data of the 1st section (first found - both have the same name) + await gu.renameSection('TABLE1', 'Foo'); + + // open right panel + await gu.toggleSidePanel('right'); + await driver.find('.test-config-data').click(); + + // check there is no groupedBy + assert.equal(await driver.find('.test-pwc-groupedBy').isDisplayed(), false); + + // click edit table data + await driver.find('.test-pwc-editDataSelection').doClick(); + + // summarize table by 'A' and save + await driver.findContent('.test-wselect-table', /Table1/).find('.test-wselect-pivot').doClick(); + await driver.findContent('.test-wselect-column', /A/).doClick(); + await driver.find('.test-wselect-addBtn').doClick(); + + // wait for server + await gu.waitForServer(); + + // check that new table is summarized + assert.equal(await driver.findWait('.test-pwc-table', 2000).getText(), 'Table1'); + assert.deepEqual(await driver.findAll('.test-pwc-groupedBy-col', (e) => e.getText()), ['A']); + + // check sections name did not change + assert.deepEqual(await gu.getSectionTitles(), ['Foo', 'TABLE1']); + + // check 1st section is active + assert(await driver.find('.viewsection_content').matches('.active_section')); + }); + + it('should work correctly when changing table', async () => { + // click edit table data + await driver.find('.test-pwc-editDataSelection').doClick(); + + // create a new table + await driver.findContent('.test-wselect-table', /New Table/).doClick(); + await driver.find('.test-wselect-addBtn').doClick(); + + // wait for server + await gu.waitForServer(); + + // check that first section shows table2 with no grouped by cols + assert.equal(await driver.findWait('.test-pwc-table', 2000).getText(), 'Table2'); + assert.equal(await driver.find('.test-pwc-groupedBy').isDisplayed(), false); + + // check sections name did not change + assert.deepEqual(await gu.getSectionTitles(), ['Foo', 'TABLE1']); + + // check 1st section is active + assert(await driver.find('.viewsection_content').matches('.active_section')); + + // revert to what it was + await gu.undo(); + }); + + it("should work correctly when changing type", async () => { + + async function switchTypeAndAssert(t: string) { + // open page widget picker + await driver.find('.test-pwc-editDataSelection').doClick(); + + // select type t and save + await driver.findContent('.test-wselect-type', gu.exactMatch(t)).doClick(); + await driver.find('.test-wselect-addBtn').doClick(); + await gu.waitForServer(); + + // check section's type + await driver.find('.test-pwc-editDataSelection').doClick(); + assert.equal(await driver.find('.test-wselect-type[class*=-selected]').getText(), t); + + // close page widget picker + await driver.sendKeys(Key.ESCAPE); + await gu.checkForErrors(); + } + + // TODO: check what's shown by asserting data for each type + await switchTypeAndAssert('Card'); + await switchTypeAndAssert('Table'); + await switchTypeAndAssert('Chart'); + + }); + + it("should work correctly when changing grouped by column", async () => { + + // open page widget picker + await driver.find('.test-pwc-editDataSelection').doClick(); + + // Select column B + await driver.findContent('.test-wselect-column', /B/).doClick(); + await driver.find('.test-wselect-addBtn').doClick(); + await gu.waitForServer(); + + // check grouped by is now A, B + assert.deepEqual(await driver.findAll('.test-pwc-groupedBy-col', (e) => e.getText()), ['A', 'B']); + + await gu.undo(); + }); + + it("should not hide any columns when changing to a summary table", async () => { + // Previously, a bug when changing data selection would sometimes cause columns to be hidden. + // This test replicates a scenario that was used to reproduce the bug, and checks that it no + // longer occurs. + + async function assertActiveSectionColumns(...expected: string[]) { + const activeSection = await driver.find('.active_section'); + const actual = (await activeSection.findAll('.column_name', el => el.getText())) + .filter(name => name !== '+'); + assert.deepEqual(actual, expected); + } + + // Create a Places table with a single Place column. + await gu.addNewTable('Places'); + await gu.renameColumn({col: 0}, 'Place'); + await gu.sendKeys(Key.ARROW_RIGHT); + await gu.sendKeys(Key.chord(Key.ALT, '-')); + await gu.waitForServer(); + await gu.sendKeys(Key.chord(Key.ALT, '-')); + await gu.waitForServer(); + + // Create an Orders table, and rename the last column to Test. + await gu.addNewTable('Orders'); + await gu.renameColumn({col: 2}, 'Test'); + + // Duplicate the Places page. + await gu.openPageMenu('Places'); + await driver.find('.test-docpage-duplicate').click(); + await driver.find('.test-modal-confirm').click(); + await driver.findContentWait('.test-docpage-label', /copy/, 1000); + await gu.waitForServer(); + + // Change the duplicated page's data to summarize Orders, grouping by column Test. + await driver.find('.test-pwc-editDataSelection').doClick(); + await driver.findContent('.test-wselect-table', /Orders/).find('.test-wselect-pivot').doClick(); + await driver.findContent('.test-wselect-column', /Test/).doClick(); + await driver.find('.test-wselect-addBtn').doClick(); + await gu.waitForServer(); + + // Check all columns are visible. + await assertActiveSectionColumns('Test', 'count'); + }); +});