From 9a10413dbc884214cd88a7b1d087d6ecb43e4a9d Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 8 Nov 2016 11:31:06 -0800 Subject: [PATCH] Improve typeahead behavior with mixed-case matches Summary: Ref T8510. We had two issues with mixed-case result sorting, like typing `@joe` to match user `Joe`. - The fallback sort was not normalized properly, so "J" could sort after "j". Instead, normalize values for sorting. - The `prefix_hits` and older `priority_hits` mechanisms were competing destructively. The `prefix_hits` mechanism completely replaces the `priority_hits` mechanism. Instead, use only the `prefix_hits` mechanism. Test Plan: - Copied results for "joe" from WMF. - Hard-coded the controller to return them. - Searched for `@joe`. - After patches, first hit is user "Joe". Reviewers: chad Reviewed By: chad Maniphest Tasks: T8510 Differential Revision: https://secure.phabricator.com/D16826 --- resources/celerity/map.php | 46 +++++++++---------- .../typeahead/source/TypeaheadSource.js | 6 +-- webroot/rsrc/js/core/Prefab.js | 19 +------- 3 files changed, 28 insertions(+), 43 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 24a1aba29e..47590fb389 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'conpherence.pkg.css' => 'cea72e09', 'conpherence.pkg.js' => '6249a1cf', 'core.pkg.css' => '4fc9469e', - 'core.pkg.js' => '035325a7', + 'core.pkg.js' => '1a77dddf', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'a4ba74b5', 'differential.pkg.js' => '634399e9', @@ -264,7 +264,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' => 'b25d5444', + 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js' => '0fcf201c', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadStaticSource.js' => '6c0e62fa', 'rsrc/favicons/apple-touch-icon-114x114.png' => '12a24178', 'rsrc/favicons/apple-touch-icon-120x120.png' => '0d1543c7', @@ -487,7 +487,7 @@ return array( 'rsrc/js/core/KeyboardShortcutManager.js' => '4a021c10', 'rsrc/js/core/MultirowRowManager.js' => 'b5d57730', 'rsrc/js/core/Notification.js' => 'ccf1cbf8', - 'rsrc/js/core/Prefab.js' => 'cfd23f37', + 'rsrc/js/core/Prefab.js' => '8d40ae75', 'rsrc/js/core/ShapedRequest.js' => '7cbe244b', 'rsrc/js/core/TextAreaUtils.js' => '320810c8', 'rsrc/js/core/Title.js' => '485aaa6c', @@ -758,7 +758,7 @@ return array( 'javelin-typeahead-normalizer' => 'e6e25838', 'javelin-typeahead-ondemand-source' => '013ffff9', 'javelin-typeahead-preloaded-source' => '54f314a0', - 'javelin-typeahead-source' => 'b25d5444', + 'javelin-typeahead-source' => '0fcf201c', 'javelin-typeahead-static-source' => '6c0e62fa', 'javelin-uri' => 'c989ade3', 'javelin-util' => '93cc50d6', @@ -810,7 +810,7 @@ return array( 'phabricator-notification-menu-css' => '1e055865', 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => 'cfd23f37', + 'phabricator-prefab' => '8d40ae75', 'phabricator-remarkup-css' => 'cd912f2c', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', @@ -1013,6 +1013,12 @@ return array( 'javelin-install', 'javelin-util', ), + '0fcf201c' => array( + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-typeahead-normalizer', + ), '116cf19b' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1652,6 +1658,18 @@ return array( 'javelin-stratcom', 'javelin-install', ), + '8d40ae75' => 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', + ), '8ff5e24c' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1850,12 +1868,6 @@ return array( 'javelin-request', 'phabricator-shaped-request', ), - 'b25d5444' => array( - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-typeahead-normalizer', - ), 'b2b4fbaf' => array( 'javelin-behavior', 'javelin-dom', @@ -1975,18 +1987,6 @@ return array( 'javelin-util', 'phabricator-notification-css', ), - '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', 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 d3dcbdf2b4..91eec515f5 100644 --- a/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js +++ b/webroot/rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadSource.js @@ -303,9 +303,9 @@ JX.install('TypeaheadSource', { } 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 key_u = u.sort || u.name; + var key_v = v.sort || v.name; + return key_u.localeCompare(key_v); }; var filter_handler = this.getFilterHandler() || function(value, list) { diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index 7dc2cc35fc..2c3da10102 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -198,12 +198,6 @@ JX.install('Prefab', { 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; - } - } - if (!item.priority) { continue; } @@ -211,12 +205,6 @@ JX.install('Prefab', { if (config.username && item.priority == config.username) { self_hits[item.id] = true; } - - for (var hh = 0; hh < tokens.length; hh++) { - if (item.priority.substr(0, tokens[hh].length) == tokens[hh]) { - priority_hits[item.id] = true; - } - } } list.sort(function(u, v) { @@ -242,10 +230,6 @@ JX.install('Prefab', { } } - if (priority_hits[u.id] != priority_hits[v.id]) { - return priority_hits[v.id] ? 1 : -1; - } - if (prefix_hits[u.id] != prefix_hits[v.id]) { return prefix_hits[v.id] ? 1 : -1; } @@ -347,7 +331,8 @@ JX.install('Prefab', { color: fields[11], tokenType: fields[12], unique: fields[13] || false, - autocomplete: fields[14] + autocomplete: fields[14], + sort: JX.TypeaheadNormalizer.normalize(fields[0]) }; },