revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00
Code Issues Releases Wiki Activity

Sort "closed" results (like disabled users) to the bottom of typeaheads, but don't hide them completely

Browse source 
Summary:
Fixes T12538. Instead of hiding "closed" results unless only closed results match, show closed results but sort them to the bottom.

This fixes the actual issue in T12538, and I think this is probably the correct/best behavior for the global search.

It also makes all other typeaheads use this behavior. They currently have a "bug" where enabled user `abcd` makes it impossible to select disabled user `abc`. This manifests in some real cases, where enabled function `members(abc)` makes it impossible to disabled user `abc` in some function tokenizers.

If ths feels worse, we could go back to filtering in the simpler cases and introduce a rule like "show closed results if only closed results would be shown OR if query is an exact match for the disabled result", but that gets dicier because "exact match" is a fuzzy concept.

(There are a lot of other minor bad behaviors that this doesn't try to fix.)

Test Plan:
Enabled project "instabug" no longer prevents bot user "instabug" from being shown:

{F4903843}

Disabled user "mmaven" is sorted below enabled user "mmclewis", in defiance of the otherwise alphabetical order. There's no visual cue that this user is disabled because of T6906.

{F4903845}

Same as above, but this source renders "disabled" in a more obvious way:

{F4903848}

Function selecting members of active project `members(instabug)` no longer prevents selection of bot user `instabug`:

{F4903849}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12538

Differential Revision: https://secure.phabricator.com/D17695
This commit is contained in:
epriestley 2017-04-14 12:14:04 -07:00
parent aec19d2acf
commit f0fbf7a7d3
4 changed files with 76 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

72
resources/celerity/map.php
View file

@ -10,7 +10,7 @@ return array(
'conpherence.pkg.css' => '437d3b5a',
'conpherence.pkg.js' => '281b1a73',
'core.pkg.css' => 'b2ad82f4',
'core.pkg.js' => 'fbc1c380',
'core.pkg.js' => 'bf3b5cdf',
'darkconsole.pkg.js' => 'e7393ebb',
'differential.pkg.css' => '90b30783',
'differential.pkg.js' => 'ddfeb49b',
@ -472,7 +472,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' => '8d40ae75',
'rsrc/js/core/Prefab.js' => 'c5af80a2',
'rsrc/js/core/ShapedRequest.js' => '7cbe244b',
'rsrc/js/core/TextAreaUtils.js' => '320810c8',
'rsrc/js/core/Title.js' => '485aaa6c',
@ -510,7 +510,7 @@ return array(
'rsrc/js/core/behavior-reorder-applications.js' => '76b9fc3e',
'rsrc/js/core/behavior-reveal-content.js' => '60821bc7',
'rsrc/js/core/behavior-scrollbar.js' => '834a1173',
'rsrc/js/core/behavior-search-typeahead.js' => '06c32383',
'rsrc/js/core/behavior-search-typeahead.js' => '0f2a0820',
'rsrc/js/core/behavior-select-content.js' => 'bf5374ef',
'rsrc/js/core/behavior-select-on-click.js' => '4e3e79a6',
'rsrc/js/core/behavior-setup-check-https.js' => '491416b3',
@ -528,7 +528,7 @@ return array(
'rsrc/js/phui/behavior-phui-tab-group.js' => '0a0b10e9',
'rsrc/js/phuix/PHUIXActionListView.js' => 'b5c256b8',
'rsrc/js/phuix/PHUIXActionView.js' => 'b3465b9b',
'rsrc/js/phuix/PHUIXAutocomplete.js' => 'd5b2abf3',
'rsrc/js/phuix/PHUIXAutocomplete.js' => 'd713a2c5',
'rsrc/js/phuix/PHUIXDropdownMenu.js' => '8018ee50',
'rsrc/js/phuix/PHUIXFormControl.js' => '83e03671',
'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b',
@ -666,7 +666,7 @@ return array(
'javelin-behavior-phabricator-oncopy' => '2926fff2',
'javelin-behavior-phabricator-remarkup-assist' => '0ca788bd',
'javelin-behavior-phabricator-reveal-content' => '60821bc7',
'javelin-behavior-phabricator-search-typeahead' => '06c32383',
'javelin-behavior-phabricator-search-typeahead' => '0f2a0820',
'javelin-behavior-phabricator-show-older-transactions' => '94c65b72',
'javelin-behavior-phabricator-tooltips' => 'c420b0b9',
'javelin-behavior-phabricator-transaction-comment-form' => 'b23b49e6',
@ -792,7 +792,7 @@ return array(
'phabricator-notification-menu-css' => '6a697e43',
'phabricator-object-selector-css' => '85ee8ce6',
'phabricator-phtize' => 'd254d646',
'phabricator-prefab' => '8d40ae75',
'phabricator-prefab' => 'c5af80a2',
'phabricator-remarkup-css' => '17c0fb37',
'phabricator-search-results-css' => 'f87d23ad',
'phabricator-shaped-request' => '7cbe244b',
@ -885,7 +885,7 @@ return array(
'phui-workpanel-view-css' => 'a3a63478',
'phuix-action-list-view' => 'b5c256b8',
'phuix-action-view' => 'b3465b9b',
'phuix-autocomplete' => 'd5b2abf3',
'phuix-autocomplete' => 'd713a2c5',
'phuix-dropdown-menu' => '8018ee50',
'phuix-form-control-view' => '83e03671',
'phuix-icon-view' => 'bff6884b',
@ -943,17 +943,6 @@ return array(
'javelin-stratcom',
'javelin-workflow',
),
'06c32383' => array(
'javelin-behavior',
'javelin-typeahead-ondemand-source',
'javelin-typeahead',
'javelin-dom',
'javelin-uri',
'javelin-util',
'javelin-stratcom',
'phabricator-prefab',
'phuix-icon-view',
),
'0825c27a' => array(
'javelin-behavior',
'javelin-dom',
@ -999,6 +988,17 @@ return array(
'phuix-autocomplete',
'javelin-mask',
),
'0f2a0820' => array(
'javelin-behavior',
'javelin-typeahead-ondemand-source',
'javelin-typeahead',
'javelin-dom',
'javelin-uri',
'javelin-util',
'javelin-stratcom',
'phabricator-prefab',
'phuix-icon-view',
),
'0f764c35' => array(
'javelin-install',
'javelin-util',
@ -1588,18 +1588,6 @@ 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',
),
'8fadb715' => array(
'javelin-install',
'javelin-util',
@ -1955,6 +1943,18 @@ return array(
'c587b80f' => array(
'javelin-install',
),
'c5af80a2' => 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',
),
'c7ccd872' => array(
'phui-fontkit-css',
),
@ -2055,12 +2055,6 @@ return array(
'javelin-uri',
'phabricator-notification',
),
'd5b2abf3' => array(
'javelin-install',
'javelin-dom',
'phuix-icon-view',
'phabricator-prefab',
),
'd6a7e717' => array(
'multirow-row-manager',
'javelin-install',
@ -2070,6 +2064,12 @@ return array(
'javelin-json',
'phabricator-prefab',
),
'd713a2c5' => array(
'javelin-install',
'javelin-dom',
'phuix-icon-view',
'phabricator-prefab',
),
'd7a74243' => array(
'javelin-behavior',
'javelin-stratcom',

32
webroot/rsrc/js/core/Prefab.js
View file

@ -101,7 +101,6 @@ JX.install('Prefab', {
datasource.setSortHandler(
JX.bind(datasource, JX.Prefab.sortHandler, config));
datasource.setFilterHandler(JX.Prefab.filterClosedResults);
datasource.setTransformer(JX.Prefab.transformDatasourceResults);
var typeahead = new JX.Typeahead(
@ -256,37 +255,6 @@ JX.install('Prefab', {
},
/**
* Filter callback for tokenizers and typeaheads which filters out closed
* or disabled objects unless they are the only options.
*/
filterClosedResults: 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;
},
/**
* Transform results from a wire format into a usable format in a standard
* way.

95
webroot/rsrc/js/core/behavior-search-typeahead.js
View file

@ -52,72 +52,55 @@ JX.behavior('phabricator-search-typeahead', function(config) {
datasource.setTransformer(transform);
// Sort handler that orders results by type (e.g., applications, users)
// and then selects for good matches on the "priority" substrings if they
// exist (for instance, username matches are preferred over real name
// matches, and application name matches are preferred over application
// flavor text matches).
var sort_handler = function(value, list, cmp) {
var priority_hits = {};
var type_priority = {
'jump' : 1,
'apps' : 2,
'proj' : 3,
'user' : 4,
'repo' : 5,
'symb' : 6
};
var tokens = this.tokenize(value);
// First, sort all the results normally.
JX.bind(this, JX.Prefab.sortHandler, {}, value, list, cmp)();
// Now we're going to apply some special rules to order results by type,
// so applications always appear near the top, then users, etc.
var ii;
var type_order = [
'jump',
'apps',
'proj',
'user',
'repo',
'symb',
'misc'
];
var type_map = {};
for (ii = 0; ii < type_order.length; ii++) {
type_map[type_order[ii]] = true;
}
var buckets = {};
for (ii = 0; ii < list.length; ii++) {
var item = list[ii];
for (var jj = 0; jj < tokens.length; jj++) {
if (item.name.indexOf(tokens[jj]) === 0) {
priority_hits[item.id] = true;
}
var type = item.priorityType;
if (!type_map.hasOwnProperty(type)) {
type = 'misc';
}
if (!item.priority) {
continue;
if (!buckets.hasOwnProperty(type)) {
buckets[type] = [];
}
for (var hh = 0; hh < tokens.length; hh++) {
if (item.priority.substr(0, tokens[hh].length) == tokens[hh]) {
priority_hits[item.id] = true;
}
}
buckets[type].push(item);
}
list.sort(function(u, v) {
var u_type = type_priority[u.priorityType] || 999;
var v_type = type_priority[v.priorityType] || 999;
if (u_type != v_type) {
return u_type - v_type;
}
if (priority_hits[u.id] != priority_hits[v.id]) {
return priority_hits[v.id] ? 1 : -1;
}
return cmp(u, v);
});
// If we have more results than fit, limit each type of result to 3, so
// we show 3 applications, then 3 users, etc. For jump items, we show only
// one result.
var type_count = 0;
var current_type = null;
for (ii = 0; ii < list.length; ii++) {
if (list[ii].type != current_type) {
current_type = list[ii].type;
type_count = 1;
} else {
type_count++;
var jj;
var results = [];
for (ii = 0; ii < type_order.length; ii++) {
var current_type = type_order[ii];
var type_list = buckets[current_type] || [];
for (jj = 0; jj < type_list.length; jj++) {
// Skip this item if:
// - it's a jump nav item, and we already have at least one jump
@ -125,19 +108,21 @@ JX.behavior('phabricator-search-typeahead', function(config) {
// - we have more items than will fit in the typeahead, and this
// is the 4..Nth result of its type.
var skip = ((current_type == 'jump') && (type_count > 1)) ||
var skip = ((current_type == 'jump') && (jj > 1)) ||
((list.length > config.limit) && (type_count > 3));
if (skip) {
list.splice(ii, 1);
ii--;
continue;
}
results.push(type_list[jj]);
}
}
// Replace the list in place with the results.
list.splice.apply(list, [0, list.length].concat(results));
};
datasource.setSortHandler(JX.bind(datasource, sort_handler));
datasource.setFilterHandler(JX.Prefab.filterClosedResults);
datasource.setMaximumResultCount(config.limit);
var typeahead = new JX.Typeahead(JX.$(config.id), JX.$(config.input));

1
webroot/rsrc/js/phuix/PHUIXAutocomplete.js
View file

@ -153,7 +153,6 @@ JX.install('PHUIXAutocomplete', {
datasource.setTransformer(JX.bind(this, this._transformresult));
datasource.setSortHandler(
JX.bind(datasource, JX.Prefab.sortHandler, {}));
datasource.setFilterHandler(JX.Prefab.filterClosedResults);
this._datasources[code] = datasource;
}