From 3c19f363bf4790a8e492add4854a448d5c702bde Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 14 Feb 2014 10:23:56 -0800 Subject: [PATCH] Provide result type information in tokenizers Summary: Ref T1279. The new dual-mode user/project tokenizers are a bit disorienting. Provide content type hints. Very open to any suggestions here, most of this patch is just getting the right data in the right places. We can change things up pretty easily. - I like the little icons in the tokens themselves, I think they look good and are useful. - I'm less sold on the '(Project)' thing I did in the dropdown. We can easily make this richer if you have thoughts on it -- we could put icons in the left column maybe? Or right-justify the types? - I made it always sort users above projects. Test Plan: See screenshot. Reviewers: chad, btrahan Reviewed By: btrahan CC: chad, aran, carl Maniphest Tasks: T4420, T1279 Differential Revision: https://secure.phabricator.com/D7250 --- resources/celerity/map.php | 66 +++++++++---------- .../phid/PhabricatorPeoplePHIDTypeUser.php | 4 ++ .../phid/PhabricatorObjectHandle.php | 7 ++ .../phid/type/PhabricatorPHIDType.php | 4 ++ .../PhabricatorProjectPHIDTypeProject.php | 4 ++ ...torTypeaheadCommonDatasourceController.php | 8 ++- .../storage/PhabricatorTypeaheadResult.php | 7 ++ .../control/AphrontTokenizerTemplateView.php | 19 +++++- .../control/AphrontFormTokenizerControl.php | 4 +- webroot/rsrc/css/aphront/tokenizer.css | 10 +++ .../lib/control/tokenizer/Tokenizer.js | 11 +++- webroot/rsrc/js/core/Prefab.js | 32 ++++++++- 12 files changed, 133 insertions(+), 43 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 33ccea4d8c..edfe669c7b 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,14 +7,14 @@ return array( 'names' => array( - 'core.pkg.css' => '607946ba', - 'core.pkg.js' => 'c7854cc5', + 'core.pkg.css' => 'd0936e05', + 'core.pkg.js' => '51eb4e66', 'darkconsole.pkg.js' => 'ca8671ce', 'differential.pkg.css' => '6aef439e', 'differential.pkg.js' => '322ea941', 'diffusion.pkg.css' => '3783278d', 'diffusion.pkg.js' => '7b51e80a', - 'javelin.pkg.js' => '6d430a66', + 'javelin.pkg.js' => '896bb02e', 'maniphest.pkg.css' => 'f1887d71', 'maniphest.pkg.js' => '1e8f11af', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', @@ -33,7 +33,7 @@ return array( 'rsrc/css/aphront/phabricator-nav-view.css' => 'd0d4a509', 'rsrc/css/aphront/request-failure-view.css' => 'da14df31', 'rsrc/css/aphront/table-view.css' => '92a719ca', - 'rsrc/css/aphront/tokenizer.css' => 'd888465e', + 'rsrc/css/aphront/tokenizer.css' => '8e7ae263', 'rsrc/css/aphront/tooltip.css' => '9c90229d', 'rsrc/css/aphront/transaction.css' => 'ce491938', 'rsrc/css/aphront/two-column.css' => '16ab3ad2', @@ -207,7 +207,7 @@ return array( 'rsrc/externals/javelin/lib/__tests__/URI.js' => 'ece3ddb3', 'rsrc/externals/javelin/lib/__tests__/behavior.js' => 'c1d75ee6', 'rsrc/externals/javelin/lib/behavior.js' => '8a3ed18b', - 'rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js' => 'e9e18227', + 'rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js' => 'e7c21fb3', 'rsrc/externals/javelin/lib/control/typeahead/Typeahead.js' => 'c22f4c01', 'rsrc/externals/javelin/lib/control/typeahead/normalizer/TypeaheadNormalizer.js' => '5f850b5c', 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js' => 'dbd9cd11', @@ -431,7 +431,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' => '9eaf0bfa', + 'rsrc/js/core/Prefab.js' => '6c78666a', 'rsrc/js/core/ShapedRequest.js' => 'dfa181a4', 'rsrc/js/core/TextAreaUtils.js' => 'b3ec3cfc', 'rsrc/js/core/ToolTip.js' => '0a81ea29', @@ -489,7 +489,7 @@ return array( 'aphront-panel-view-css' => '5846dfa2', 'aphront-request-failure-view-css' => 'da14df31', 'aphront-table-view-css' => '92a719ca', - 'aphront-tokenizer-control-css' => 'd888465e', + 'aphront-tokenizer-control-css' => '8e7ae263', 'aphront-tooltip-css' => '9c90229d', 'aphront-two-column-view-css' => '16ab3ad2', 'aphront-typeahead-control-css' => '00c9a200', @@ -641,7 +641,7 @@ return array( 'javelin-request' => '23f9bb8d', 'javelin-resource' => '356de121', 'javelin-stratcom' => 'c293f7b9', - 'javelin-tokenizer' => 'e9e18227', + 'javelin-tokenizer' => 'e7c21fb3', 'javelin-typeahead' => 'c22f4c01', 'javelin-typeahead-composite-source' => 'dbd9cd11', 'javelin-typeahead-normalizer' => '5f850b5c', @@ -701,7 +701,7 @@ return array( 'phabricator-object-list-view-css' => '1a1ea560', 'phabricator-object-selector-css' => '029a133d', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => '9eaf0bfa', + 'phabricator-prefab' => '6c78666a', 'phabricator-profile-css' => '3a7e04ca', 'phabricator-project-tag-css' => '095c9404', 'phabricator-remarkup-css' => 'ca7f2265', @@ -1205,6 +1205,19 @@ return array( 0 => 'javelin-install', 1 => 'javelin-util', ), + '6c78666a' => + 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', + ), '6ec125a0' => array( 0 => 'javelin-behavior', @@ -1348,6 +1361,10 @@ return array( 2 => 'javelin-stratcom', 3 => 'javelin-uri', ), + '8e7ae263' => + array( + 0 => 'aphront-typeahead-control-css', + ), '8f24abfc' => array( 0 => 'javelin-behavior', @@ -1414,19 +1431,6 @@ return array( 1 => 'javelin-vector', 2 => 'javelin-dom', ), - '9eaf0bfa' => - 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', - ), '9eb2cedb' => array( 0 => 'javelin-behavior', @@ -1736,10 +1740,6 @@ return array( 3 => 'javelin-dom', 4 => 'phabricator-keyboard-shortcut', ), - 'd888465e' => - array( - 0 => 'aphront-typeahead-control-css', - ), 'd8ef8659' => array( 0 => 'javelin-behavior', @@ -1815,6 +1815,13 @@ return array( 2 => 'javelin-dom', 3 => 'javelin-util', ), + 'e7c21fb3' => + array( + 0 => 'javelin-dom', + 1 => 'javelin-util', + 2 => 'javelin-stratcom', + 3 => 'javelin-install', + ), 'e9b95df3' => array( 0 => 'javelin-install', @@ -1822,13 +1829,6 @@ return array( 2 => 'javelin-request', 3 => 'javelin-typeahead-source', ), - 'e9e18227' => - array( - 0 => 'javelin-dom', - 1 => 'javelin-util', - 2 => 'javelin-stratcom', - 3 => 'javelin-install', - ), 'e9fdb5e5' => array( 0 => 'javelin-behavior', diff --git a/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php b/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php index a6df8adb15..1bc144c6d0 100644 --- a/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php +++ b/src/applications/people/phid/PhabricatorPeoplePHIDTypeUser.php @@ -12,6 +12,10 @@ final class PhabricatorPeoplePHIDTypeUser extends PhabricatorPHIDType { return pht('Phabricator User'); } + public function getTypeIcon() { + return 'policy-all'; + } + public function newObject() { return new PhabricatorUser(); } diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 85f355e8c4..3829878f10 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -17,6 +17,13 @@ final class PhabricatorObjectHandle private $objectName; private $policyFiltered; + public function getTypeIcon() { + if ($this->getPHIDType()) { + return $this->getPHIDType()->getTypeIcon(); + } + return null; + } + public function setPolicyFiltered($policy_filered) { $this->policyFiltered = $policy_filered; return $this; diff --git a/src/applications/phid/type/PhabricatorPHIDType.php b/src/applications/phid/type/PhabricatorPHIDType.php index 6ee4c4e8ec..4e901e2358 100644 --- a/src/applications/phid/type/PhabricatorPHIDType.php +++ b/src/applications/phid/type/PhabricatorPHIDType.php @@ -9,6 +9,10 @@ abstract class PhabricatorPHIDType { return null; } + public function getTypeIcon() { + return null; + } + /** * Build a @{class:PhabricatorPolicyAwareQuery} to load objects of this type * by PHID. diff --git a/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php b/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php index 312ead52ea..c4de156143 100644 --- a/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php +++ b/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php @@ -12,6 +12,10 @@ final class PhabricatorProjectPHIDTypeProject extends PhabricatorPHIDType { return pht('Project'); } + public function getTypeIcon() { + return 'policy-project'; + } + public function newObject() { return new PhabricatorProject(); } diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php index fa54094a27..d19b2d0775 100644 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php +++ b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php @@ -208,7 +208,9 @@ final class PhabricatorTypeaheadCommonDatasourceController ->setName($user->getFullName()) ->setURI('/p/'.$user->getUsername()) ->setPHID($user->getPHID()) - ->setPriorityString($user->getUsername()); + ->setPriorityString($user->getUsername()) + ->setIcon('policy-all') + ->setPriorityType('user'); if ($need_rich_data) { $display_type = 'User'; @@ -217,7 +219,6 @@ final class PhabricatorTypeaheadCommonDatasourceController } $result->setDisplayType($display_type); $result->setImageURI($handles[$user->getPHID()]->getImageURI()); - $result->setPriorityType('user'); } $results[] = $result; } @@ -292,7 +293,8 @@ final class PhabricatorTypeaheadCommonDatasourceController ->setName($proj->getName()) ->setDisplayType("Project") ->setURI('/project/view/'.$proj->getID().'/') - ->setPHID($proj->getPHID()); + ->setPHID($proj->getPHID()) + ->setIcon('policy-project'); $proj_result->setImageURI($proj->getProfileImageURI()); diff --git a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php index 8627e5bdda..f6b2a8ce48 100644 --- a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php +++ b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php @@ -10,6 +10,12 @@ final class PhabricatorTypeaheadResult { private $displayType; private $imageURI; private $priorityType; + private $icon; + + public function setIcon($icon) { + $this->icon = $icon; + return $this; + } public function setName($name) { $this->name = $name; @@ -61,6 +67,7 @@ final class PhabricatorTypeaheadResult { $this->displayType, $this->imageURI ? (string)$this->imageURI : null, $this->priorityType, + $this->icon, ); while (end($data) === null) { array_pop($data); diff --git a/src/view/control/AphrontTokenizerTemplateView.php b/src/view/control/AphrontTokenizerTemplateView.php index 4e4e442593..345f827d2e 100644 --- a/src/view/control/AphrontTokenizerTemplateView.php +++ b/src/view/control/AphrontTokenizerTemplateView.php @@ -12,6 +12,7 @@ final class AphrontTokenizerTemplateView extends AphrontView { } public function setValue(array $value) { + assert_instances_of($value, 'PhabricatorObjectHandle'); $this->value = $value; return $this; } @@ -38,7 +39,10 @@ final class AphrontTokenizerTemplateView extends AphrontView { $tokens = array(); foreach ($values as $key => $value) { - $tokens[] = $this->renderToken($key, $value); + $tokens[] = $this->renderToken( + $value->getPHID(), + $value->getFullName(), + $value->getTypeIcon()); } $input = javelin_tag( @@ -66,11 +70,22 @@ final class AphrontTokenizerTemplateView extends AphrontView { $content); } - private function renderToken($key, $value) { + private function renderToken($key, $value, $icon) { $input_name = $this->getName(); if ($input_name) { $input_name .= '[]'; } + + if ($icon) { + $value = array( + phutil_tag( + 'span', + array( + 'class' => 'phui-icon-view sprite-status status-'.$icon, + )), + $value); + } + return phutil_tag( 'a', array( diff --git a/src/view/form/control/AphrontFormTokenizerControl.php b/src/view/form/control/AphrontFormTokenizerControl.php index c43fd68405..83847ce46c 100644 --- a/src/view/form/control/AphrontFormTokenizerControl.php +++ b/src/view/form/control/AphrontFormTokenizerControl.php @@ -36,7 +36,6 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { $values = nonempty($this->getValue(), array()); assert_instances_of($values, 'PhabricatorObjectHandle'); - $values = mpull($values, 'getFullName', 'getPHID'); if ($this->getID()) { $id = $this->getID(); @@ -62,7 +61,8 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { Javelin::initBehavior('aphront-basic-tokenizer', array( 'id' => $id, 'src' => $this->datasource, - 'value' => $values, + 'value' => mpull($values, 'getFullName', 'getPHID'), + 'icons' => mpull($values, 'getTypeIcon', 'getPHID'), 'limit' => $this->limit, 'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'), 'username' => $username, diff --git a/webroot/rsrc/css/aphront/tokenizer.css b/webroot/rsrc/css/aphront/tokenizer.css index a62e55e77b..6403da91fd 100644 --- a/webroot/rsrc/css/aphront/tokenizer.css +++ b/webroot/rsrc/css/aphront/tokenizer.css @@ -74,3 +74,13 @@ a.jx-tokenizer-token:hover { text-decoration: none; background: #bbcef1; } + +.jx-tokenizer-token .phui-icon-view { + display: inline-block; + margin: 2px 4px -2px 0; +} + +.tokenizer-result-type { + margin-left: 4px; + color: {$lightgreytext}; +} diff --git a/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js b/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js index 3c5736c134..4465f7f91b 100644 --- a/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js +++ b/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js @@ -46,7 +46,8 @@ JX.install('Tokenizer', { 'change'], properties : { - limit : null + limit : null, + renderTokenCallback : null }, members : { @@ -329,11 +330,17 @@ JX.install('Tokenizer', { sigil: 'remove' }, '\u00d7'); // U+00D7 multiplication sign + var display_token = value; + var render_callback = this.getRenderTokenCallback(); + if (render_callback) { + display_token = render_callback(value, key); + } + return JX.$N('a', { className: 'jx-tokenizer-token', sigil: 'token', meta: {key: key} - }, [value, input, remove]); + }, [display_token, input, remove]); }, getTokens : function() { diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index 56bd70ac7f..121ca6cce9 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -99,6 +99,16 @@ JX.install('Prefab', { return priority_hits[v.id] ? 1 : -1; } + // Sort users ahead of other result types. + if (u.priorityType != v.priorityType) { + if (u.priorityType == 'user') { + return -1; + } + if (v.priorityType == 'user') { + return 1; + } + } + return cmp(u, v); }); }; @@ -111,7 +121,9 @@ JX.install('Prefab', { display: object[0], uri: object[1], id: object[2], - priority: object[3] + priority: object[3], + priorityType: object[7], + icon: object[8] }; }); @@ -122,6 +134,24 @@ JX.install('Prefab', { var tokenizer = new JX.Tokenizer(root); tokenizer.setTypeahead(typeahead); + tokenizer.setRenderTokenCallback(function(value, key) { + var icon = datasource.getResult(key); + if (icon) { + icon = icon.icon; + } else { + icon = config.icons[key]; + } + + if (!icon) { + return value; + } + + icon = JX.$N( + 'span', + {className: 'phui-icon-view sprite-status status-' + icon}); + + return [icon, value]; + }); if (config.placeholder) { tokenizer.setPlaceholder(config.placeholder); -- 2.51.2