1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Improve tokenizer sorting rules

Summary:
Currently, we sort all results alphabetically. This isn't ideal. Instead, sort them like this:

  - If the viewing user appears in the list, always sort them first. This is common in a lot of contexts and some "Ben Evans" guy is sorting first on secure.phabricator.com and causing me no end of aggravation.
  - If the tokens match a "priority" component (e.g., username), sort that before results which do not have a "priority" match.
  - Within a group (self, priority, everything else) sort tokens alphabetically.

NOTE: I need to go add setUser() to all the tokenizers to make the "self" rule work, but that's trivial so I figured I'd get this out first.

Test Plan:
https://secure.phabricator.com/file/data/4s2a72l5hhyyqqkq4bnd/PHID-FILE-x2r6ubk7s7dz54kxmtwx/Screen_Shot_2012-03-07_at_9.18.03_AM.png

Previously, "aaaaaepriestley" (first alphabetic match) would sort before "epriestley" (the viewing user). Now, "epriestley" sorts first because that is the viewer.

https://secure.phabricator.com/file/data/rmnxgnafz42f23fsjwui/PHID-FILE-yrnn55jl3ysbntldq3af/Screen_Shot_2012-03-07_at_9.18.09_AM.png

Previously, "aaaagopher" (first alphabetic match) would sort before "banana" (the "priority" match). Now, "banana" sorts first because it priority matches on username.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T946

Differential Revision: https://secure.phabricator.com/D1807
This commit is contained in:
epriestley 2012-03-07 13:17:44 -08:00
parent 76fd9a2d28
commit 492d047a49
7 changed files with 97 additions and 27 deletions

2
externals/javelin vendored

@ -1 +1 @@
Subproject commit e91928961926c1a81247cd9d7bdcdee251f18cfa Subproject commit 14e180d5d316d496886730cb6a41d1b36de16ec1

View file

@ -357,7 +357,7 @@ celerity_register_resource_map(array(
), ),
'javelin-behavior-aphront-basic-tokenizer' => 'javelin-behavior-aphront-basic-tokenizer' =>
array( array(
'uri' => '/res/9be30797/rsrc/js/application/core/behavior-tokenizer.js', 'uri' => '/res/69a085d3/rsrc/js/application/core/behavior-tokenizer.js',
'type' => 'js', 'type' => 'js',
'requires' => 'requires' =>
array( array(
@ -368,6 +368,7 @@ celerity_register_resource_map(array(
4 => 'javelin-typeahead-ondemand-source', 4 => 'javelin-typeahead-ondemand-source',
5 => 'javelin-dom', 5 => 'javelin-dom',
6 => 'javelin-stratcom', 6 => 'javelin-stratcom',
7 => 'javelin-util',
), ),
'disk' => '/rsrc/js/application/core/behavior-tokenizer.js', 'disk' => '/rsrc/js/application/core/behavior-tokenizer.js',
), ),
@ -2007,6 +2008,22 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/2e291441/differential.pkg.js', 'uri' => '/res/pkg/2e291441/differential.pkg.js',
'type' => 'js', 'type' => 'js',
), ),
'3e7cc9b3' =>
array(
'name' => 'typeahead.pkg.js',
'symbols' =>
array(
0 => 'javelin-typeahead',
1 => 'javelin-typeahead-normalizer',
2 => 'javelin-typeahead-source',
3 => 'javelin-typeahead-preloaded-source',
4 => 'javelin-typeahead-ondemand-source',
5 => 'javelin-tokenizer',
6 => 'javelin-behavior-aphront-basic-tokenizer',
),
'uri' => '/res/pkg/3e7cc9b3/typeahead.pkg.js',
'type' => 'js',
),
'4fbae2af' => '4fbae2af' =>
array( array(
'name' => 'javelin.pkg.js', 'name' => 'javelin.pkg.js',
@ -2036,22 +2053,6 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/61f9d480/diffusion.pkg.css', 'uri' => '/res/pkg/61f9d480/diffusion.pkg.css',
'type' => 'css', 'type' => 'css',
), ),
'ae7ea233' =>
array(
'name' => 'typeahead.pkg.js',
'symbols' =>
array(
0 => 'javelin-typeahead',
1 => 'javelin-typeahead-normalizer',
2 => 'javelin-typeahead-source',
3 => 'javelin-typeahead-preloaded-source',
4 => 'javelin-typeahead-ondemand-source',
5 => 'javelin-tokenizer',
6 => 'javelin-behavior-aphront-basic-tokenizer',
),
'uri' => '/res/pkg/ae7ea233/typeahead.pkg.js',
'type' => 'js',
),
31583232 => 31583232 =>
array( array(
'name' => 'maniphest.pkg.css', 'name' => 'maniphest.pkg.css',
@ -2114,7 +2115,7 @@ celerity_register_resource_map(array(
'differential-table-of-contents-css' => '09c86840', 'differential-table-of-contents-css' => '09c86840',
'diffusion-commit-view-css' => '61f9d480', 'diffusion-commit-view-css' => '61f9d480',
'javelin-behavior' => '4fbae2af', 'javelin-behavior' => '4fbae2af',
'javelin-behavior-aphront-basic-tokenizer' => 'ae7ea233', 'javelin-behavior-aphront-basic-tokenizer' => '3e7cc9b3',
'javelin-behavior-aphront-drag-and-drop' => '2e291441', 'javelin-behavior-aphront-drag-and-drop' => '2e291441',
'javelin-behavior-aphront-drag-and-drop-textarea' => '2e291441', 'javelin-behavior-aphront-drag-and-drop-textarea' => '2e291441',
'javelin-behavior-aphront-form-disable-on-submit' => '95944588', 'javelin-behavior-aphront-form-disable-on-submit' => '95944588',
@ -2146,12 +2147,12 @@ celerity_register_resource_map(array(
'javelin-mask' => '95944588', 'javelin-mask' => '95944588',
'javelin-request' => '4fbae2af', 'javelin-request' => '4fbae2af',
'javelin-stratcom' => '4fbae2af', 'javelin-stratcom' => '4fbae2af',
'javelin-tokenizer' => 'ae7ea233', 'javelin-tokenizer' => '3e7cc9b3',
'javelin-typeahead' => 'ae7ea233', 'javelin-typeahead' => '3e7cc9b3',
'javelin-typeahead-normalizer' => 'ae7ea233', 'javelin-typeahead-normalizer' => '3e7cc9b3',
'javelin-typeahead-ondemand-source' => 'ae7ea233', 'javelin-typeahead-ondemand-source' => '3e7cc9b3',
'javelin-typeahead-preloaded-source' => 'ae7ea233', 'javelin-typeahead-preloaded-source' => '3e7cc9b3',
'javelin-typeahead-source' => 'ae7ea233', 'javelin-typeahead-source' => '3e7cc9b3',
'javelin-uri' => '4fbae2af', 'javelin-uri' => '4fbae2af',
'javelin-util' => '4fbae2af', 'javelin-util' => '4fbae2af',
'javelin-vector' => '4fbae2af', 'javelin-vector' => '4fbae2af',

View file

@ -1,7 +1,7 @@
<?php <?php
/* /*
* Copyright 2011 Facebook, Inc. * Copyright 2012 Facebook, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -81,6 +81,7 @@ final class DifferentialCCsFieldSpecification
return id(new AphrontFormTokenizerControl()) return id(new AphrontFormTokenizerControl())
->setLabel('CC') ->setLabel('CC')
->setName('cc') ->setName('cc')
->setUser($this->getUser())
->setDatasource('/typeahead/common/mailable/') ->setDatasource('/typeahead/common/mailable/')
->setValue($cc_map); ->setValue($cc_map);
} }

View file

@ -86,6 +86,7 @@ final class DifferentialReviewersFieldSpecification
return id(new AphrontFormTokenizerControl()) return id(new AphrontFormTokenizerControl())
->setLabel('Reviewers') ->setLabel('Reviewers')
->setName('reviewers') ->setName('reviewers')
->setUser($this->getUser())
->setDatasource('/typeahead/common/users/') ->setDatasource('/typeahead/common/users/')
->setValue($reviewer_map) ->setValue($reviewer_map)
->setError($this->error); ->setError($this->error);

View file

@ -129,6 +129,7 @@ class PhabricatorTypeaheadCommonDatasourceController
$user->getUsername().' ('.$user->getRealName().')', $user->getUsername().' ('.$user->getRealName().')',
'/p/'.$user->getUsername(), '/p/'.$user->getUsername(),
$user->getPHID(), $user->getPHID(),
$user->getUsername(),
); );
} }
} }
@ -162,6 +163,7 @@ class PhabricatorTypeaheadCommonDatasourceController
'r'.$repo->getCallsign().' ('.$repo->getName().')', 'r'.$repo->getCallsign().' ('.$repo->getName().')',
'/diffusion/'.$repo->getCallsign().'/', '/diffusion/'.$repo->getCallsign().'/',
$repo->getPHID(), $repo->getPHID(),
'r'.$repo->getCallsign(),
); );
} }
} }

View file

@ -1,7 +1,7 @@
<?php <?php
/* /*
* Copyright 2011 Facebook, Inc. * Copyright 2012 Facebook, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -21,6 +21,7 @@ class AphrontFormTokenizerControl extends AphrontFormControl {
private $datasource; private $datasource;
private $disableBehavior; private $disableBehavior;
private $limit; private $limit;
private $user;
public function setDatasource($datasource) { public function setDatasource($datasource) {
$this->datasource = $datasource; $this->datasource = $datasource;
@ -41,6 +42,11 @@ class AphrontFormTokenizerControl extends AphrontFormControl {
return $this; return $this;
} }
public function setUser($user) {
$this->user = $user;
return $this;
}
protected function renderInput() { protected function renderInput() {
$name = $this->getName(); $name = $this->getName();
$values = nonempty($this->getValue(), array()); $values = nonempty($this->getValue(), array());
@ -56,6 +62,11 @@ class AphrontFormTokenizerControl extends AphrontFormControl {
$template->setID($id); $template->setID($id);
$template->setValue($values); $template->setValue($values);
$username = null;
if ($this->user) {
$username = $this->user->getUsername();
}
if (!$this->disableBehavior) { if (!$this->disableBehavior) {
Javelin::initBehavior('aphront-basic-tokenizer', array( Javelin::initBehavior('aphront-basic-tokenizer', array(
'id' => $id, 'id' => $id,
@ -63,6 +74,7 @@ class AphrontFormTokenizerControl extends AphrontFormControl {
'value' => $values, 'value' => $values,
'limit' => $this->limit, 'limit' => $this->limit,
'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'), 'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
'username' => $username,
)); ));
} }

View file

@ -7,6 +7,7 @@
* javelin-typeahead-ondemand-source * javelin-typeahead-ondemand-source
* javelin-dom * javelin-dom
* javelin-stratcom * javelin-stratcom
* javelin-util
*/ */
JX.behavior('aphront-basic-tokenizer', function(config) { JX.behavior('aphront-basic-tokenizer', function(config) {
@ -19,6 +20,57 @@ JX.behavior('aphront-basic-tokenizer', function(config) {
datasource = new JX.TypeaheadPreloadedSource(config.src); datasource = new JX.TypeaheadPreloadedSource(config.src);
} }
// Sort results so that the viewing user always comes up first; after that,
// prefer unixname matches to realname matches.
var sort_handler = function(value, list, cmp) {
var priority_hits = {};
var self_hits = {};
var tokens = this.tokenize(value);
for (var ii = 0; ii < list.length; ii++) {
var item = list[ii];
if (!item.priority) {
continue;
}
if (config.username && item.priority == config.username) {
self_hits[item.id] = true;
}
for (var jj = 0; jj < tokens.length; jj++) {
if (item.priority.substr(0, tokens[jj].length) == tokens[jj]) {
priority_hits[item.id] = true;
}
}
}
list.sort(function(u, v) {
if (self_hits[u.id] != self_hits[v.id]) {
return self_hits[v.id] ? 1 : -1;
}
if (priority_hits[u.id] != priority_hits[v.id]) {
return priority_hits[v.id] ? 1 : -1;
}
return cmp(u, v);
});
};
datasource.setSortHandler(JX.bind(datasource, sort_handler));
datasource.setTransformer(
function(object) {
return {
name : object[0],
display : object[0],
uri : object[1],
id : object[2],
priority : object[3]
};
});
var typeahead = new JX.Typeahead( var typeahead = new JX.Typeahead(
root, root,
JX.DOM.find(root, 'input', 'tokenizer-input')); JX.DOM.find(root, 'input', 'tokenizer-input'));
@ -38,4 +90,5 @@ JX.behavior('aphront-basic-tokenizer', function(config) {
JX.Stratcom.addData(root, {'tokenizer' : tokenizer}); JX.Stratcom.addData(root, {'tokenizer' : tokenizer});
tokenizer.start(); tokenizer.start();
}); });