From ad0562e15e38739c703294d7722b65cd78c1f96f Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 9 Jun 2016 13:13:11 -0700 Subject: [PATCH] Improve some typeahead matching behaviors Summary: Ref T8510. Sort prefix matches above non-prefix matches, so that "Ape Discovery" does not match "discovery" better than "Discovery". Sort functions last. Rename function internal strings so they don't get over-promoted the prefix-match rules. Add kind of a hack to get "Project X" sorting above all the "Project X (Milestone 1)" results. Test Plan: Created "Ape Discovery", "Baboon Discovery", "Chimpanzee Discovery", etc. Main project now sorts above milestones: {F1681773} Prefix matches now sort above other matches: {F1681774} Function results (rarely used) are now less prominent: {F1681775} Better function results here: {F1681776} More function results: {F1681777} Reviewers: chad Reviewed By: chad Maniphest Tasks: T8510 Differential Revision: https://secure.phabricator.com/D16094 --- resources/celerity/map.php | 46 +++++++++---------- .../PhabricatorProjectDatasource.php | 5 ++ ...abricatorProjectLogicalOrNotDatasource.php | 4 +- ...habricatorProjectLogicalUserDatasource.php | 2 +- .../typeahead/source/TypeaheadSource.js | 2 +- webroot/rsrc/js/core/Prefab.js | 20 ++++++++ 6 files changed, 52 insertions(+), 27 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index c3c65fb46a..3110e85dd2 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => '6913fe66', - 'core.pkg.js' => '3f2c120d', + 'core.pkg.js' => '10275c16', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'f3fb8324', 'differential.pkg.js' => '4b7d8f19', @@ -258,7 +258,7 @@ return array( 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js' => '503e17fd', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadOnDemandSource.js' => '013ffff9', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadPreloadedSource.js' => '54f314a0', - 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => '1bc11c4a', + 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => 'b25d5444', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadStaticSource.js' => '6c0e62fa', 'rsrc/favicons/apple-touch-icon-120x120.png' => '43742962', 'rsrc/favicons/apple-touch-icon-152x152.png' => '669eaec3', @@ -465,7 +465,7 @@ return array( 'rsrc/js/core/KeyboardShortcutManager.js' => 'c1700f6f', 'rsrc/js/core/MultirowRowManager.js' => 'b5d57730', 'rsrc/js/core/Notification.js' => 'ccf1cbf8', - 'rsrc/js/core/Prefab.js' => 'e67df814', + 'rsrc/js/core/Prefab.js' => 'cfd23f37', 'rsrc/js/core/ShapedRequest.js' => '7cbe244b', 'rsrc/js/core/TextAreaUtils.js' => '320810c8', 'rsrc/js/core/Title.js' => 'df5e11d2', @@ -734,7 +734,7 @@ return array( 'javelin-typeahead-normalizer' => 'e6e25838', 'javelin-typeahead-ondemand-source' => '013ffff9', 'javelin-typeahead-preloaded-source' => '54f314a0', - 'javelin-typeahead-source' => '1bc11c4a', + 'javelin-typeahead-source' => 'b25d5444', 'javelin-typeahead-static-source' => '6c0e62fa', 'javelin-uri' => 'c989ade3', 'javelin-util' => '93cc50d6', @@ -785,7 +785,7 @@ return array( 'phabricator-notification-menu-css' => 'f31c0bde', 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => 'e67df814', + 'phabricator-prefab' => 'cfd23f37', 'phabricator-remarkup-css' => '523d34bb', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', @@ -1045,12 +1045,6 @@ return array( 'javelin-util', 'phabricator-keyboard-shortcut-manager', ), - '1bc11c4a' => array( - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-typeahead-normalizer', - ), '1bd28176' => array( 'javelin-install', 'javelin-dom', @@ -1779,6 +1773,12 @@ return array( 'javelin-request', 'phabricator-shaped-request', ), + 'b25d5444' => array( + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-typeahead-normalizer', + ), 'b2b4fbaf' => array( 'javelin-behavior', 'javelin-dom', @@ -1932,6 +1932,18 @@ return array( 'javelin-workflow', 'phabricator-drag-and-drop-file-upload', ), + 'cfd23f37' => array( + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-typeahead', + 'javelin-tokenizer', + 'javelin-typeahead-preloaded-source', + 'javelin-typeahead-ondemand-source', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-util', + ), 'd0c516d5' => array( 'javelin-behavior', 'javelin-dom', @@ -2077,18 +2089,6 @@ return array( 'javelin-workflow', 'javelin-magical-init', ), - 'e67df814' => array( - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-typeahead', - 'javelin-tokenizer', - 'javelin-typeahead-preloaded-source', - 'javelin-typeahead-ondemand-source', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-util', - ), 'e6e25838' => array( 'javelin-install', ), diff --git a/src/applications/project/typeahead/PhabricatorProjectDatasource.php b/src/applications/project/typeahead/PhabricatorProjectDatasource.php index b51c55ee0a..e0b277c2ad 100644 --- a/src/applications/project/typeahead/PhabricatorProjectDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectDatasource.php @@ -84,6 +84,11 @@ final class PhabricatorProjectDatasource $all_strings = array(); $all_strings[] = $proj->getDisplayName(); + + // Add an extra space after the name so that the original project + // sorts ahead of milestones. This is kind of a hack but ehh? + $all_strings[] = null; + foreach ($proj->getSlugs() as $project_slug) { $all_strings[] = $project_slug->getSlug(); } diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php index ad392a677a..b9984efb05 100644 --- a/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php @@ -92,14 +92,14 @@ final class PhabricatorProjectLogicalOrNotDatasource $return[] = id(clone $result) ->setPHID('any('.$result->getPHID().')') ->setDisplayName(pht('In Any: %s', $result->getDisplayName())) - ->setName($result->getName().' any'); + ->setName('any '.$result->getName()); } if ($return_not) { $return[] = id(clone $result) ->setPHID('not('.$result->getPHID().')') ->setDisplayName(pht('Not In: %s', $result->getDisplayName())) - ->setName($result->getName().' not'); + ->setName('not '.$result->getName()); } } diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php index 3b02195c25..bc19c12aa3 100644 --- a/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php @@ -46,7 +46,7 @@ final class PhabricatorProjectLogicalUserDatasource ->setIcon('fa-asterisk') ->setPHID('projects('.$result->getPHID().')') ->setDisplayName(pht("User's Projects: %s", $result->getDisplayName())) - ->setName($result->getName().' projects'); + ->setName('projects '.$result->getName()); } return $results; 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 74dc1c2453..d3dcbdf2b4 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js @@ -357,7 +357,7 @@ JX.install('TypeaheadSource', { if (!str.length) { return []; } - return str.split(/\s/g); + return str.split(/\s+/g); }, _defaultTransformer : function(object) { return { diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index 5948958c8f..7dc2cc35fc 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -184,11 +184,20 @@ JX.install('Prefab', { var priority_hits = {}; var self_hits = {}; + // We'll put matches where the user's input is a prefix of the name + // above mathches where that isn't true. + var prefix_hits = {}; + var tokens = this.tokenize(value); + var normal = this.normalize(value); for (var ii = 0; ii < list.length; ii++) { var item = list[ii]; + if (this.normalize(item.name).indexOf(normal) === 0) { + prefix_hits[item.id] = true; + } + for (var jj = 0; jj < tokens.length; jj++) { if (item.name.indexOf(tokens[jj]) === 0) { priority_hits[item.id] = true; @@ -237,6 +246,10 @@ JX.install('Prefab', { return priority_hits[v.id] ? 1 : -1; } + if (prefix_hits[u.id] != prefix_hits[v.id]) { + return prefix_hits[v.id] ? 1 : -1; + } + // Sort users ahead of other result types. if (u.priorityType != v.priorityType) { if (u.priorityType == 'user') { @@ -247,6 +260,13 @@ JX.install('Prefab', { } } + // Sort functions after other result types. + var uf = (u.tokenType == 'function'); + var vf = (v.tokenType == 'function'); + if (uf != vf) { + return uf ? 1 : -1; + } + return cmp(u, v); }); },