From 0efce646c95f346b376da560d0f2d5cbfe4b8f57 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 14 Feb 2014 10:24:58 -0800 Subject: [PATCH] Improve tokenizer loading behaviors Summary: Ref T4420. Fixes T3309. Two major UX issues here: - When the user extends a query ("alin" -> "alinc"), we currently hide all the results, then show them again when the new results arrive. This makes the typeahead feel a bit flickery. Instead, show matching results, then add more results when everything arrives. - When loading more results from ondemand sources, we currently do not give you any indication that things are loading. Instead: - Show a loading GIF (this might need #design help, @chad). - Slightly lighten the control border. - I didn't want to do anything like actually add "loading" text because it would cause UI flicker in the 'extend a query' case and some other cases, but otherwise this design is totally made up. Test Plan: Typed into tokenizers and extended queries, got a better-feeling UI. Reviewers: chad, btrahan Reviewed By: btrahan CC: chad, aran Maniphest Tasks: T3309, T4420 Differential Revision: https://secure.phabricator.com/D8233 --- resources/celerity/map.php | 113 +++++++++--------- webroot/rsrc/css/aphront/typeahead.css | 18 +++ .../lib/control/tokenizer/Tokenizer.js | 9 ++ .../lib/control/typeahead/Typeahead.js | 48 ++++++-- .../source/TypeaheadOnDemandSource.js | 13 ++ .../typeahead/source/TypeaheadSource.js | 6 +- .../rsrc/image/loading/tokenizer_loading.gif | Bin 0 -> 2545 bytes webroot/rsrc/js/core/Prefab.js | 6 + 8 files changed, 145 insertions(+), 68 deletions(-) create mode 100644 webroot/rsrc/image/loading/tokenizer_loading.gif diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d01a8c542a..11b51714da 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,14 +7,14 @@ return array( 'names' => array( - 'core.pkg.css' => '044c2f0c', - 'core.pkg.js' => '0dc59a05', + 'core.pkg.css' => '1ccefdc6', + 'core.pkg.js' => 'ee746639', 'darkconsole.pkg.js' => 'ca8671ce', 'differential.pkg.css' => '6aef439e', 'differential.pkg.js' => '322ea941', 'diffusion.pkg.css' => '3783278d', 'diffusion.pkg.js' => '7b51e80a', - 'javelin.pkg.js' => '896bb02e', + 'javelin.pkg.js' => 'da52b0df', 'maniphest.pkg.css' => 'f1887d71', 'maniphest.pkg.js' => '1e8f11af', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', @@ -37,7 +37,7 @@ return array( 'rsrc/css/aphront/tooltip.css' => '9c90229d', 'rsrc/css/aphront/transaction.css' => 'ce491938', 'rsrc/css/aphront/two-column.css' => '16ab3ad2', - 'rsrc/css/aphront/typeahead.css' => '104a6525', + 'rsrc/css/aphront/typeahead.css' => 'd24b4228', 'rsrc/css/application/auth/auth.css' => '1e655982', 'rsrc/css/application/base/main-menu-view.css' => 'd36e0c11', 'rsrc/css/application/base/notification-menu.css' => 'fc9a363c', @@ -207,13 +207,13 @@ return array( 'rsrc/externals/javelin/lib/__tests__/URI.js' => 'ece3ddb3', 'rsrc/externals/javelin/lib/__tests__/behavior.js' => 'c1d75ee6', 'rsrc/externals/javelin/lib/behavior.js' => '8a3ed18b', - 'rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js' => 'e7c21fb3', - 'rsrc/externals/javelin/lib/control/typeahead/Typeahead.js' => 'c22f4c01', + 'rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js' => '1c1a6cdf', + 'rsrc/externals/javelin/lib/control/typeahead/Typeahead.js' => 'd99e27f7', 'rsrc/externals/javelin/lib/control/typeahead/normalizer/TypeaheadNormalizer.js' => '5f850b5c', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js' => 'dbd9cd11', - 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js' => '1f595fb0', + 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js' => '7383383f', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadPreloadedSource.js' => 'e9b95df3', - 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => 'f4412299', + 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => '74fe50ac', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadStaticSource.js' => 'c2b8bf64', 'rsrc/externals/raphael/g.raphael.js' => '40dde778', 'rsrc/externals/raphael/g.raphael.line.js' => '40da039e', @@ -287,6 +287,7 @@ return array( 'rsrc/image/loading/loading_48.gif' => '6a4994c7', 'rsrc/image/loading/loading_d48.gif' => 'cdcbe900', 'rsrc/image/loading/loading_w24.gif' => '7662fa2b', + 'rsrc/image/loading/tokenizer_loading.gif' => '93622511', 'rsrc/image/main_texture.png' => '29a2c5ad', 'rsrc/image/menu_texture.png' => '5a17580d', 'rsrc/image/people/harding.png' => '45aa614e', @@ -431,7 +432,7 @@ return array( 'rsrc/js/core/KeyboardShortcutManager.js' => 'ad7a69ca', 'rsrc/js/core/MultirowRowManager.js' => 'e7076916', 'rsrc/js/core/Notification.js' => '95944043', - 'rsrc/js/core/Prefab.js' => '979f864d', + 'rsrc/js/core/Prefab.js' => '83ee580c', 'rsrc/js/core/ShapedRequest.js' => 'dfa181a4', 'rsrc/js/core/TextAreaUtils.js' => 'b3ec3cfc', 'rsrc/js/core/ToolTip.js' => '0a81ea29', @@ -492,7 +493,7 @@ return array( 'aphront-tokenizer-control-css' => '08ea6326', 'aphront-tooltip-css' => '9c90229d', 'aphront-two-column-view-css' => '16ab3ad2', - 'aphront-typeahead-control-css' => '104a6525', + 'aphront-typeahead-control-css' => 'd24b4228', 'auth-css' => '1e655982', 'config-options-css' => '7fedf08b', 'conpherence-menu-css' => '561348ac', @@ -641,13 +642,13 @@ return array( 'javelin-request' => '23f9bb8d', 'javelin-resource' => '356de121', 'javelin-stratcom' => 'c293f7b9', - 'javelin-tokenizer' => 'e7c21fb3', - 'javelin-typeahead' => 'c22f4c01', + 'javelin-tokenizer' => '1c1a6cdf', + 'javelin-typeahead' => 'd99e27f7', 'javelin-typeahead-composite-source' => 'dbd9cd11', 'javelin-typeahead-normalizer' => '5f850b5c', - 'javelin-typeahead-ondemand-source' => '1f595fb0', + 'javelin-typeahead-ondemand-source' => '7383383f', 'javelin-typeahead-preloaded-source' => 'e9b95df3', - 'javelin-typeahead-source' => 'f4412299', + 'javelin-typeahead-source' => '74fe50ac', 'javelin-typeahead-static-source' => 'c2b8bf64', 'javelin-uri' => 'd9a9b862', 'javelin-util' => '7501647b', @@ -701,7 +702,7 @@ return array( 'phabricator-object-list-view-css' => '1a1ea560', 'phabricator-object-selector-css' => '029a133d', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => '979f864d', + 'phabricator-prefab' => '83ee580c', 'phabricator-profile-css' => '3a7e04ca', 'phabricator-project-tag-css' => '095c9404', 'phabricator-remarkup-css' => 'ca7f2265', @@ -909,6 +910,13 @@ return array( 4 => 'javelin-workflow', 5 => 'phabricator-draggable-list', ), + '1c1a6cdf' => + array( + 0 => 'javelin-dom', + 1 => 'javelin-util', + 2 => 'javelin-stratcom', + 3 => 'javelin-install', + ), '1e1c8a59' => array( 0 => 'javelin-behavior', @@ -928,13 +936,6 @@ return array( 5 => 'phabricator-drag-and-drop-file-upload', 6 => 'phabricator-draggable-list', ), - '1f595fb0' => - array( - 0 => 'javelin-install', - 1 => 'javelin-util', - 2 => 'javelin-request', - 3 => 'javelin-typeahead-source', - ), '2290aeef' => array( 0 => 'javelin-install', @@ -1226,6 +1227,20 @@ return array( 0 => 'javelin-behavior', 1 => 'javelin-dom', ), + '7383383f' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-request', + 3 => 'javelin-typeahead-source', + ), + '74fe50ac' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-dom', + 3 => 'javelin-typeahead-normalizer', + ), '75e50c72' => array( 0 => 'javelin-behavior', @@ -1293,6 +1308,19 @@ return array( 1 => 'javelin-dom', 2 => 'javelin-reactor-dom', ), + '83ee580c' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-dom', + 3 => 'javelin-typeahead', + 4 => 'javelin-tokenizer', + 5 => 'javelin-typeahead-preloaded-source', + 6 => 'javelin-typeahead-ondemand-source', + 7 => 'javelin-dom', + 8 => 'javelin-stratcom', + 9 => 'javelin-util', + ), '8454ce4f' => array( 0 => 'javelin-behavior', @@ -1398,19 +1426,6 @@ return array( 2 => 'javelin-view-visitor', 3 => 'javelin-util', ), - '979f864d' => - array( - 0 => 'javelin-install', - 1 => 'javelin-util', - 2 => 'javelin-dom', - 3 => 'javelin-typeahead', - 4 => 'javelin-tokenizer', - 5 => 'javelin-typeahead-preloaded-source', - 6 => 'javelin-typeahead-ondemand-source', - 7 => 'javelin-dom', - 8 => 'javelin-stratcom', - 9 => 'javelin-util', - ), '9b9197be' => array( 0 => 'javelin-behavior', @@ -1604,13 +1619,6 @@ return array( 5 => 'javelin-workflow', 6 => 'javelin-vector', ), - 'c22f4c01' => - array( - 0 => 'javelin-install', - 1 => 'javelin-dom', - 2 => 'javelin-vector', - 3 => 'javelin-util', - ), 'c293f7b9' => array( 0 => 'javelin-install', @@ -1752,6 +1760,13 @@ return array( 7 => 'phabricator-dropdown-menu', 8 => 'phabricator-menu-item', ), + 'd99e27f7' => + array( + 0 => 'javelin-install', + 1 => 'javelin-dom', + 2 => 'javelin-vector', + 3 => 'javelin-util', + ), 'd9a9b862' => array( 0 => 'javelin-install', @@ -1815,13 +1830,6 @@ return array( 2 => 'javelin-dom', 3 => 'javelin-util', ), - 'e7c21fb3' => - array( - 0 => 'javelin-dom', - 1 => 'javelin-util', - 2 => 'javelin-stratcom', - 3 => 'javelin-install', - ), 'e9b95df3' => array( 0 => 'javelin-install', @@ -1874,13 +1882,6 @@ return array( 2 => 'javelin-vector', 3 => 'javelin-dom', ), - 'f4412299' => - array( - 0 => 'javelin-install', - 1 => 'javelin-util', - 2 => 'javelin-dom', - 3 => 'javelin-typeahead-normalizer', - ), 'f6b56f7a' => array( 0 => 'javelin-behavior', diff --git a/webroot/rsrc/css/aphront/typeahead.css b/webroot/rsrc/css/aphront/typeahead.css index fcf983a72d..cdeeb3d6eb 100644 --- a/webroot/rsrc/css/aphront/typeahead.css +++ b/webroot/rsrc/css/aphront/typeahead.css @@ -47,3 +47,21 @@ table.jx-typeahead input { input.jx-typeahead-placeholder { color: {$lightgreytext}; } + +.jx-typeahead-waiting-indicator { + position: absolute; + right: 8px; + top: 2px; + width: 24px; + height: 24px; + background-image: url(/rsrc/image/loading/tokenizer_loading.gif); + display: none; +} + +.jx-typeahead-waiting .jx-typeahead-waiting-indicator { + display: inline-block; +} + +div.jx-tokenizer-container-focused.jx-typeahead-waiting { + border-color: {$lightblueborder}; +} diff --git a/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js b/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js index 4465f7f91b..7ebe2f6bcd 100644 --- a/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js +++ b/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js @@ -36,6 +36,7 @@ JX.install('Tokenizer', { construct : function(containerNode) { this._containerNode = containerNode; + this._uiNodes = []; }, events : [ @@ -63,6 +64,7 @@ JX.install('Tokenizer', { _seq : 0, _lastvalue : null, _placeholder : null, + _uinodes : null, start : function() { if (__DEV__) { @@ -124,6 +126,7 @@ JX.install('Tokenizer', { this._root = root; root.appendChild(focus); + JX.DOM.appendContent(root, this._uiNodes); var typeahead = this._typeahead; typeahead.setInputNode(this._focus); @@ -232,6 +235,12 @@ JX.install('Tokenizer', { return this; }, + addUINode : function(node) { + this._uiNodes.push(node); + this._typeahead.addUINode(node); + return this; + }, + _redraw : function(force) { // If there are tokens in the tokenizer, never show a placeholder. diff --git a/webroot/rsrc/externals/javelin/lib/control/typeahead/Typeahead.js b/webroot/rsrc/externals/javelin/lib/control/typeahead/Typeahead.js index 210e0b077c..edadb10dbd 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/Typeahead.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/Typeahead.js @@ -124,6 +124,7 @@ JX.install('Typeahead', { _datasource : null, _waitingListener : null, _readyListener : null, + _completeListener : null, /** * Activate your properly configured typeahead. It won't do anything until @@ -160,15 +161,20 @@ JX.install('Typeahead', { this._datasource.unbindFromTypeahead(); this._waitingListener.remove(); this._readyListener.remove(); + this._completeListener.remove(); } this._waitingListener = datasource.listen( 'waiting', - JX.bind(this, this.waitForResults) - ); + JX.bind(this, this.waitForResults)); + this._readyListener = datasource.listen( 'resultsready', - JX.bind(this, this.showResults) - ); + JX.bind(this, this.showResults)); + + this._completeListener = datasource.listen( + 'complete', + JX.bind(this, this.doneWaitingForResults)); + datasource.bindToTypeahead(this); this._datasource = datasource; }, @@ -190,6 +196,19 @@ JX.install('Typeahead', { return this; }, + /** + * Add an arbitrary node to the UI. Phabricator uses this to add a + * "waiting" graphic. + * + * @param node An arbitrary display node for the UI. + * @return this + * @task config + */ + addUINode : function(node) { + JX.DOM.appendContent(this._hardpoint, node); + return this; + }, + /** * Hide the typeahead's dropdown suggestion menu. @@ -247,18 +266,27 @@ JX.install('Typeahead', { this._value = this._control.value; this.invoke('change', this._value); }, + /** - * Show a "waiting for results" UI in place of the typeahead's dropdown - * suggestion menu. NOTE: currently there's no such UI, lolol. + * Show a "waiting for results" UI. We may be showing a partial result set + * at this time, if the user is extending a query we already have results + * for. * * @task control * @return void */ waitForResults : function() { - // TODO: Build some sort of fancy spinner or "..." type UI here to - // visually indicate that we're waiting on the server. - // Wait on the datasource 'complete' event for hiding the spinner. - this.hide(); + JX.DOM.alterClass(this._hardpoint, 'jx-typeahead-waiting', true); + }, + + /** + * Hide the "waiting for results" UI. + * + * @task control + * @return void + */ + doneWaitingForResults : function() { + JX.DOM.alterClass(this._hardpoint, 'jx-typeahead-waiting', false); }, /** diff --git a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js index 43ef772ef3..c799c82e6d 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js @@ -48,6 +48,19 @@ JX.install('TypeaheadOnDemandSource', { if (this.haveData[value]) { this.matchResults(value); } else { + // If we have data for any prefix of the query, send those results + // back immediately. This allows "alinc" -> "alinco" to show partial + // results without the UI flickering. We'll still show the loading + // state, and then can show better results once we get everything + // back. + for (var ii = value.length - 1; ii > 0; ii--) { + var substr = value.substring(0, ii); + if (this.haveData[substr]) { + this.matchResults(value, false); + break; + } + } + this.waitForResults(); setTimeout( JX.bind(this, this.sendRequest, this.lastChange, value, raw_value), diff --git a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js index 6a3ff7a6e5..9d49596ca0 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js @@ -211,7 +211,7 @@ JX.install('TypeaheadSource', { }, - matchResults : function(value) { + matchResults : function(value, partial) { // This table keeps track of the number of tokens each potential match // has actually matched. When we're done, the real matches are those @@ -279,7 +279,9 @@ JX.install('TypeaheadSource', { var nodes = this.renderNodes(value, hits); this.invoke('resultsready', nodes); - this.invoke('complete'); + if (!partial) { + this.invoke('complete'); + } }, sortHits : function(value, hits) { diff --git a/webroot/rsrc/image/loading/tokenizer_loading.gif b/webroot/rsrc/image/loading/tokenizer_loading.gif new file mode 100644 index 0000000000000000000000000000000000000000..7ac990cf04f2323f9723518bfd72ce102222cefa GIT binary patch literal 2545 zcma*pYg7~I83*u}xzA)~n25ScX&WZP5>nU=1T3OL6cUK&3gY5=2L-vPKosR#4J1Gy z2{*Y2f`*F<925Z+DqcWPab1;V*Q&?%tge1gx39sBO@dC?%nI@=>b1`dA^f;rd<;kzBEP~@QDg`5nuHDmwWi`ccB(8fJu!D@W@6GOX&Kuyf#(~9#Kot4om)5$3D8qAS)~A$Ca-6yH;bP-YR%)e z*uVwm#a35*X`LHGj32c;jsl4QZfgsvW!V?{2-SqnJnoXXF*nkrHs1S%=LGrPst~{V zbIXcXr&l++uUTGfQbH(6qL7Kn#+Xt|g{`v4+z9V)In}XN;&Aif7dZ&S7naR;h=?Fn^{lc|@~&lZlv?W9&si{Zho3Zq zBNC0W8pT9f=}<$GQHasxVL1urLe*G$-i1I28VmOw4D1-nlxMUF7p{*V9W9#? zG3-PbWllts&>7IxQA0d2*lgcSw`;G+rpt*+JWGhuM10z1YIElzOCrY2>B0NtO_MPS zMFf29+_^(8vP$B!PKEl9XU62S@@i=sRcJ^lp7MFFlGzbm>4Ok7O@u`=7$RC~4%Sy4 zYHq1ICcqWPd4_@-lucMS20@nSCdAI1T?+2+dcgowe`%@+K-I${;uYHgEWPq*J&<;{Drqc!cL#1&wL zi~k_=rv}{|hWqv;9yTrI81Aln{6ubW*40;m3xk6_R;~^o)wq1blF3R~tzpvi6+Rf{ zO@-+>SE~;p9EM~ zU%BS@TGN0@Gf>_;Jmxbp+J13zX24k|z@bQPb{EF0CmfYbxJ~O%k&CRSN)_G#!raT! zB2RD$R7b>ld3eqnaC6j5`6O>B8pXLa+91{US;KR#c0HAi>FV66qJsEf-$^exLCvJ- z%W|Bbzf?`*EQ`~OSQ*Od+B(aTqp3|iTw(MPFU$~VJ?;aGI!+E-Q!o?glyj^!ZbYbP z<81oFtestXwVDMoCi>Qzy^KTP|UDdNG+sSR`)Vuyu=yjHg}M zH5*jBS^y(;6%bRP17M-JlrwwBuLqQr?LxNQ2Ap(AZMr6W(R`uRYsO5SbV3s5B{#F3 z?bTf&Y=oM+5~VulI}4c$ywg?lF2$O-J>>{pyh0HWwX(|vc5$0dm&0?WCd+4+EVI!Y zSIvL^QCh;@6C7H)WSzV1jZquFVu}7kr(#T4NoZNQwWz+Hsot|!kK&TG%XbyRqHrU> zrDUs0vWz(H*cQ)oToQ|KQbzl`y7D9&Rj64em4>mS`oNM&In6ragv)*h3eOeJDClzv zrUIRU#P6attq&SbC2NImfWuB$|KFHY&nHXNgaXGshn8el`2t4$ssAr zg-enc+Mjah;dVLBDhMKtMEzQ6_8=ffITG6I%4%_Z1t~EgEIs%Y50R-}{lXPtaRnGM z%m{fOr=JJ$_yGCStL)*PzUStktKI5vCo@J|sw0KJ8P70OktWe{=ZBoUg49f1TqwLl zj*zTG2hq?vo)*iz!b|MDT0pU`bU3<|A(po40|gRSfg_q4KxI5qwo& z&vU_ttugmMuaC9=bfLnK70l9k%ub9E@1oeeAm5%70H@0l36AOzh_dm7SnIW36a>X1 zM~QH{g@HC5TaYNAZGeh}>7I}Ph};P1A^_1^orKh*jUSKaVq_nG>tL@gP?*|emk;O8 z$Qb+&V?7I{%@YTZ<%k{ecMc$xmSy4qQV}Kg5