From e18b161464ee786e7a32e26faeb13c87638713eb Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 14 Feb 2014 15:16:01 -0800 Subject: [PATCH] Don't show closed typeahead results while open results exist Summary: Ref T4420. When a result list contains both open and closed results, hide the closed results. I think this has a good chance of almost always working, and feeling very intuitive. It has a small chance of being a weird mess. It feels reasonable to me so far The one bad case I can come up with here is that if you have results which shadow each other, like "Apples" (a closed project) and "Apples and Bananas" (an open project), it is impossible to get "Apples" in the result list, because "Apples and Bananas" will always shadow it. Let's wait for someone to hit this before we figure out how to deal with it. Test Plan: Typed through open stuff to hit closed stuff. Reviewers: chad, btrahan Reviewed By: chad CC: aran Maniphest Tasks: T4420 Differential Revision: https://secure.phabricator.com/D8238 --- resources/celerity/map.php | 52 +++++++++---------- .../PhabricatorProjectSearchIndexer.php | 1 - .../typeahead/source/TypeaheadSource.js | 32 +++++++++--- webroot/rsrc/js/core/Prefab.js | 33 ++++++++++++ 4 files changed, 85 insertions(+), 33 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 11b51714da..a0a18d8b3e 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,13 +8,13 @@ return array( 'names' => array( 'core.pkg.css' => '1ccefdc6', - 'core.pkg.js' => 'ee746639', + 'core.pkg.js' => '92f2c0a7', 'darkconsole.pkg.js' => 'ca8671ce', 'differential.pkg.css' => '6aef439e', 'differential.pkg.js' => '322ea941', 'diffusion.pkg.css' => '3783278d', 'diffusion.pkg.js' => '7b51e80a', - 'javelin.pkg.js' => 'da52b0df', + 'javelin.pkg.js' => 'c7ef4e11', 'maniphest.pkg.css' => 'f1887d71', 'maniphest.pkg.js' => '1e8f11af', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', @@ -213,7 +213,7 @@ return array( 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js' => 'dbd9cd11', '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' => '74fe50ac', + 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => '5e18d309', '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', @@ -432,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' => '83ee580c', + 'rsrc/js/core/Prefab.js' => '88ca7175', 'rsrc/js/core/ShapedRequest.js' => 'dfa181a4', 'rsrc/js/core/TextAreaUtils.js' => 'b3ec3cfc', 'rsrc/js/core/ToolTip.js' => '0a81ea29', @@ -648,7 +648,7 @@ return array( 'javelin-typeahead-normalizer' => '5f850b5c', 'javelin-typeahead-ondemand-source' => '7383383f', 'javelin-typeahead-preloaded-source' => 'e9b95df3', - 'javelin-typeahead-source' => '74fe50ac', + 'javelin-typeahead-source' => '5e18d309', 'javelin-typeahead-static-source' => 'c2b8bf64', 'javelin-uri' => 'd9a9b862', 'javelin-util' => '7501647b', @@ -702,7 +702,7 @@ return array( 'phabricator-object-list-view-css' => '1a1ea560', 'phabricator-object-selector-css' => '029a133d', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => '83ee580c', + 'phabricator-prefab' => '88ca7175', 'phabricator-profile-css' => '3a7e04ca', 'phabricator-project-tag-css' => '095c9404', 'phabricator-remarkup-css' => 'ca7f2265', @@ -1163,6 +1163,13 @@ return array( 3 => 'javelin-stratcom', 4 => 'javelin-vector', ), + '5e18d309' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-dom', + 3 => 'javelin-typeahead-normalizer', + ), '5f004630' => array( 0 => 'javelin-behavior', @@ -1234,13 +1241,6 @@ return array( 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', @@ -1308,19 +1308,6 @@ 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', @@ -1368,6 +1355,19 @@ return array( 6 => 'javelin-history', 7 => 'javelin-vector', ), + '88ca7175' => + 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', + ), '8a3ed18b' => array( 0 => 'javelin-magical-init', diff --git a/src/applications/project/search/PhabricatorProjectSearchIndexer.php b/src/applications/project/search/PhabricatorProjectSearchIndexer.php index ee29969f2d..a5c8d5dca3 100644 --- a/src/applications/project/search/PhabricatorProjectSearchIndexer.php +++ b/src/applications/project/search/PhabricatorProjectSearchIndexer.php @@ -28,7 +28,6 @@ final class PhabricatorProjectSearchIndexer PhabricatorProjectPHIDTypeProject::TYPECONST, time()); - // NOTE: This could be more full-featured, but for now we're mostly // interested in the side effects of indexing. 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 9d49596ca0..254d6b90f1 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js @@ -109,7 +109,21 @@ JX.install('TypeaheadSource', { * * @param function */ - sortHandler : null + sortHandler : null, + + /** + * Optional function which is used to filter results before display. Inputs + * are the input string and a list of matches. The function should + * return a list of matches to display. This is the minimum useful + * implementation: + * + * function(value, list) { + * return list; + * } + * + * @param function + */ + filterHandler : null }, @@ -275,7 +289,7 @@ JX.install('TypeaheadSource', { } } - this.sortHits(value, hits); + this.filterAndSortHits(value, hits); var nodes = this.renderNodes(value, hits); this.invoke('resultsready', nodes); @@ -284,24 +298,30 @@ JX.install('TypeaheadSource', { } }, - sortHits : function(value, hits) { + filterAndSortHits : function(value, hits) { var objs = []; var ii; for (ii = 0; ii < hits.length; ii++) { objs.push(this._raw[hits[ii]]); } - var default_comparator = function(u, v) { + var default_comparator = function(u, v) { var key_u = u.sort || u.name; var key_v = v.sort || v.name; return key_u.localeCompare(key_v); }; - var handler = this.getSortHandler() || function(value, list, cmp) { + var filter_handler = this.getFilterHandler() || function(value, list) { + return list; + }; + + objs = filter_handler(value, objs); + + var sort_handler = this.getSortHandler() || function(value, list, cmp) { list.sort(cmp); }; - handler(value, objs, default_comparator); + sort_handler(value, objs, default_comparator); hits.splice(0, hits.length); for (ii = 0; ii < objs.length; ii++) { diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index c35dc6c635..89fc8efbe5 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -146,6 +146,39 @@ JX.install('Prefab', { }; datasource.setSortHandler(JX.bind(datasource, sort_handler)); + + // Don't show any closed objects until the query is specific enough that + // it only selects closed objects. Specifically, if the result list had + // any open objects, remove all the closed objects from the list. + var filter_handler = function(value, list) { + // Look for any open result. + var has_open = false; + var ii; + for (ii = 0; ii < list.length; ii++) { + if (!list[ii].closed) { + has_open = true; + break; + } + } + + if (!has_open) { + // Everything is closed, so just use it as-is. + return list; + } + + // Otherwise, only display the open results. + var results = []; + for (ii = 0; ii < list.length; ii++) { + if (!list[ii].closed) { + results.push(list[ii]); + } + } + + return results; + }; + + datasource.setFilterHandler(filter_handler); + datasource.setTransformer( function(object) { var closed = object[9];