From 76448a75de30b250f6030ff70d394e20929f0d99 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 17 Apr 2015 07:00:43 -0700 Subject: [PATCH] Make token UI stronger and more consistent Summary: Ref T4100. Overall: - Use token background color to communicate token type (blue = object, yellow = function, grey = disabled/closed, red = invalid). - Use token icon color to make color choices consistent (specifically, use project icon colors in project tokens). - For functions, use token icon to communicate function result type (e.g., viewer() has a user icon; members(...) has a group icon), since we don't need the icon to indicate "this is a function" anymore. Test Plan: {F374615} {F374616} {F374617} Reviewers: chad, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4100 Differential Revision: https://secure.phabricator.com/D12446 --- src/__phutil_library_map__.php | 2 + .../typeahead/PhabricatorViewerDatasource.php | 1 + .../phid/PhabricatorObjectHandle.php | 8 +++ .../PhabricatorProjectDatasource.php | 2 +- .../PhabricatorProjectMembersDatasource.php | 1 + .../PhabricatorTypeaheadDatasource.php | 8 +-- .../storage/PhabricatorTypeaheadResult.php | 14 +++++ .../view/PhabricatorTypeaheadTokenView.php | 50 ++++++++++++++-- .../examples/PHUITypeaheadExample.php | 57 +++++++++++++++++++ .../control/AphrontFormTokenizerControl.php | 20 ++++--- webroot/rsrc/css/aphront/tokenizer.css | 45 +++++++++++++++ .../lib/control/tokenizer/Tokenizer.js | 18 ++++-- webroot/rsrc/js/core/Prefab.js | 13 +++-- 13 files changed, 210 insertions(+), 29 deletions(-) create mode 100644 src/applications/uiexample/examples/PHUITypeaheadExample.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7c92abd395..3e980d3602 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1206,6 +1206,7 @@ phutil_register_library_map(array( 'PHUITimelineEventView' => 'view/phui/PHUITimelineEventView.php', 'PHUITimelineExample' => 'applications/uiexample/examples/PHUITimelineExample.php', 'PHUITimelineView' => 'view/phui/PHUITimelineView.php', + 'PHUITypeaheadExample' => 'applications/uiexample/examples/PHUITypeaheadExample.php', 'PHUIWorkboardView' => 'view/phui/PHUIWorkboardView.php', 'PHUIWorkpanelView' => 'view/phui/PHUIWorkpanelView.php', 'PackageCreateMail' => 'applications/owners/mail/PackageCreateMail.php', @@ -4489,6 +4490,7 @@ phutil_register_library_map(array( 'PHUITimelineEventView' => 'AphrontView', 'PHUITimelineExample' => 'PhabricatorUIExample', 'PHUITimelineView' => 'AphrontView', + 'PHUITypeaheadExample' => 'PhabricatorUIExample', 'PHUIWorkboardView' => 'AphrontTagView', 'PHUIWorkpanelView' => 'AphrontTagView', 'PackageCreateMail' => 'PackageMail', diff --git a/src/applications/people/typeahead/PhabricatorViewerDatasource.php b/src/applications/people/typeahead/PhabricatorViewerDatasource.php index a57d5ab54d..92bd48bab0 100644 --- a/src/applications/people/typeahead/PhabricatorViewerDatasource.php +++ b/src/applications/people/typeahead/PhabricatorViewerDatasource.php @@ -50,6 +50,7 @@ final class PhabricatorViewerDatasource return $this->newFunctionResult() ->setName(pht('Current Viewer')) ->setPHID('viewer()') + ->setIcon('fa-user') ->setUnique(true); } diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 8b596f68f6..f36c21516a 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -56,9 +56,17 @@ final class PhabricatorObjectHandle if ($this->tagColor) { return $this->tagColor; } + return 'blue'; } + public function getIconColor() { + if ($this->tagColor) { + return $this->tagColor; + } + return null; + } + public function getTypeIcon() { if ($this->getPHIDType()) { return $this->getPHIDType()->getTypeIcon(); diff --git a/src/applications/project/typeahead/PhabricatorProjectDatasource.php b/src/applications/project/typeahead/PhabricatorProjectDatasource.php index 9b92c66620..22a2fb68f6 100644 --- a/src/applications/project/typeahead/PhabricatorProjectDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectDatasource.php @@ -63,7 +63,7 @@ final class PhabricatorProjectDatasource ->setDisplayType('Project') ->setURI('/tag/'.$proj->getPrimarySlug().'/') ->setPHID($proj->getPHID()) - ->setIcon($proj->getIcon().' bluegrey') + ->setIcon($proj->getIcon().' '.$proj->getColor()) ->setPriorityType('proj') ->setClosed($closed); diff --git a/src/applications/project/typeahead/PhabricatorProjectMembersDatasource.php b/src/applications/project/typeahead/PhabricatorProjectMembersDatasource.php index c19426e25d..59694a8109 100644 --- a/src/applications/project/typeahead/PhabricatorProjectMembersDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectMembersDatasource.php @@ -116,6 +116,7 @@ final class PhabricatorProjectMembersDatasource ->setDisplayName(pht('Members: %s', $project->getName())) ->setURI('/tag/'.$project->getPrimarySlug().'/') ->setPHID('members('.$project->getPHID().')') + ->setIcon('fa-users') ->setClosed($closed); } diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php index 29304196ea..533e5aaa88 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php @@ -187,16 +187,16 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { } protected function newFunctionResult() { - // TODO: Find a more consistent design. return id(new PhabricatorTypeaheadResult()) - ->setIcon('fa-magic indigo'); + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setIcon('fa-asterisk'); } public function newInvalidToken($name) { return id(new PhabricatorTypeaheadTokenView()) - ->setKey(PhabricatorTypeaheadTokenView::KEY_INVALID) ->setValue($name) - ->setIcon('fa-exclamation-circle red'); + ->setIcon('fa-exclamation-circle') + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_INVALID); } /* -( Token Functions )---------------------------------------------------- */ diff --git a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php index 500fcd0909..f364b142e8 100644 --- a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php +++ b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php @@ -13,6 +13,7 @@ final class PhabricatorTypeaheadResult { private $imageSprite; private $icon; private $closed; + private $tokenType; private $unique; public function setIcon($icon) { @@ -91,6 +92,18 @@ final class PhabricatorTypeaheadResult { return $this; } + public function setTokenType($type) { + $this->tokenType = $type; + return $this; + } + + public function getTokenType() { + if ($this->closed && !$this->tokenType) { + return PhabricatorTypeaheadTokenView::TYPE_DISABLED; + } + return $this->tokenType; + } + public function getSortKey() { // Put unique results (special parameter functions) ahead of other // results. @@ -116,6 +129,7 @@ final class PhabricatorTypeaheadResult { $this->getIcon(), $this->closed, $this->imageSprite ? (string)$this->imageSprite : null, + $this->tokenType, $this->unique ? 1 : null, ); while (end($data) === null) { diff --git a/src/applications/typeahead/view/PhabricatorTypeaheadTokenView.php b/src/applications/typeahead/view/PhabricatorTypeaheadTokenView.php index a412fb508d..0322fe544a 100644 --- a/src/applications/typeahead/view/PhabricatorTypeaheadTokenView.php +++ b/src/applications/typeahead/view/PhabricatorTypeaheadTokenView.php @@ -3,12 +3,16 @@ final class PhabricatorTypeaheadTokenView extends AphrontTagView { + const TYPE_OBJECT = 'object'; + const TYPE_DISABLED = 'disabled'; + const TYPE_FUNCTION = 'function'; + const TYPE_INVALID = 'invalid'; + private $key; private $icon; private $inputName; private $value; - - const KEY_INVALID = ''; + private $tokenType = self::TYPE_OBJECT; public static function newFromTypeaheadResult( PhabricatorTypeaheadResult $result) { @@ -16,16 +20,24 @@ final class PhabricatorTypeaheadTokenView return id(new PhabricatorTypeaheadTokenView()) ->setKey($result->getPHID()) ->setIcon($result->getIcon()) - ->setValue($result->getDisplayName()); + ->setValue($result->getDisplayName()) + ->setTokenType($result->getTokenType()); } public static function newFromHandle( PhabricatorObjectHandle $handle) { - return id(new PhabricatorTypeaheadTokenView()) + $token = id(new PhabricatorTypeaheadTokenView()) ->setKey($handle->getPHID()) ->setValue($handle->getFullName()) - ->setIcon($handle->getIcon()); + ->setIcon(rtrim($handle->getIcon().' '.$handle->getIconColor())); + + if ($handle->isDisabled() || + $handle->getStatus() == PhabricatorObjectHandleStatus::STATUS_CLOSED) { + $token->setTokenType(self::TYPE_DISABLED); + } + + return $token; } public function setKey($key) { @@ -37,6 +49,15 @@ final class PhabricatorTypeaheadTokenView return $this->key; } + public function setTokenType($token_type) { + $this->tokenType = $token_type; + return $this; + } + + public function getTokenType() { + return $this->tokenType; + } + public function setInputName($input_name) { $this->inputName = $input_name; return $this; @@ -69,8 +90,25 @@ final class PhabricatorTypeaheadTokenView } protected function getTagAttributes() { + $classes = array(); + $classes[] = 'jx-tokenizer-token'; + switch ($this->getTokenType()) { + case self::TYPE_FUNCTION: + $classes[] = 'jx-tokenizer-token-function'; + break; + case self::TYPE_INVALID: + $classes[] = 'jx-tokenizer-token-invalid'; + break; + case self::TYPE_DISABLED: + $classes[] = 'jx-tokenizer-token-disabled'; + break; + case self::TYPE_OBJECT: + default: + break; + } + return array( - 'class' => 'jx-tokenizer-token', + 'class' => $classes, ); } diff --git a/src/applications/uiexample/examples/PHUITypeaheadExample.php b/src/applications/uiexample/examples/PHUITypeaheadExample.php new file mode 100644 index 0000000000..1c039c380d --- /dev/null +++ b/src/applications/uiexample/examples/PHUITypeaheadExample.php @@ -0,0 +1,57 @@ +setValue(pht('Normal Object')) + ->setIcon('fa-user'); + + $token_list[] = id(new PhabricatorTypeaheadTokenView()) + ->setValue(pht('Disabled Object')) + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_DISABLED) + ->setIcon('fa-user'); + + $token_list[] = id(new PhabricatorTypeaheadTokenView()) + ->setValue(pht('Custom Object')) + ->setIcon('fa-tag green'); + + $token_list[] = id(new PhabricatorTypeaheadTokenView()) + ->setValue(pht('Function Token')) + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) + ->setIcon('fa-users'); + + $token_list[] = id(new PhabricatorTypeaheadTokenView()) + ->setValue(pht('Invalid Token')) + ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_INVALID) + ->setIcon('fa-exclamation-circle'); + + + $token_list = phutil_tag( + 'div', + array( + 'class' => 'grouped', + 'style' => 'padding: 8px', + ), + $token_list); + + $output = array(); + + $output[] = id(new PHUIObjectBoxView()) + ->setHeaderText('Tokens') + ->appendChild($token_list); + + return $output; + } +} diff --git a/src/view/form/control/AphrontFormTokenizerControl.php b/src/view/form/control/AphrontFormTokenizerControl.php index 6561b44d29..079349d12c 100644 --- a/src/view/form/control/AphrontFormTokenizerControl.php +++ b/src/view/form/control/AphrontFormTokenizerControl.php @@ -51,7 +51,9 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { } $datasource = $this->datasource; - $datasource->setViewer($this->getUser()); + if ($datasource) { + $datasource->setViewer($this->getUser()); + } $placeholder = null; if (!strlen($this->placeholder)) { @@ -84,7 +86,8 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { $token = $datasource->newInvalidToken($name); } - if ($token->getKey() == PhabricatorTypeaheadTokenView::KEY_INVALID) { + $type = $token->getTokenType(); + if ($type == PhabricatorTypeaheadTokenView::TYPE_INVALID) { $token->setKey($value); } } @@ -117,12 +120,13 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { if (!$this->disableBehavior) { Javelin::initBehavior('aphront-basic-tokenizer', array( - 'id' => $id, - 'src' => $datasource_uri, - 'value' => mpull($tokens, 'getValue', 'getKey'), - 'icons' => mpull($tokens, 'getIcon', 'getKey'), - 'limit' => $this->limit, - 'username' => $username, + 'id' => $id, + 'src' => $datasource_uri, + 'value' => mpull($tokens, 'getValue', 'getKey'), + 'icons' => mpull($tokens, 'getIcon', 'getKey'), + 'types' => mpull($tokens, 'getTokenType', 'getKey'), + 'limit' => $this->limit, + 'username' => $username, 'placeholder' => $placeholder, 'browseURI' => $browse_uri, )); diff --git a/webroot/rsrc/css/aphront/tokenizer.css b/webroot/rsrc/css/aphront/tokenizer.css index 2056c88c31..81ba64bb6c 100644 --- a/webroot/rsrc/css/aphront/tokenizer.css +++ b/webroot/rsrc/css/aphront/tokenizer.css @@ -83,6 +83,51 @@ a.jx-tokenizer-token:hover { color: {$bluetext}; } +a.jx-tokenizer-token-function { + border-color: {$sh-lightyellowborder}; + background: {$sh-yellowbackground}; + color: {$sh-yellowtext}; +} + +a.jx-tokenizer-token-function:hover { + border-color: {$sh-yellowborder}; + background: {$lightyellow}; +} + +.jx-tokenizer-token-function .phui-icon-view { + color: {$sh-yellowicon}; +} + +a.jx-tokenizer-token-disabled { + border-color: {$sh-lightgreyborder}; + background: {$sh-greybackground}; + color: {$sh-greytext}; +} + +a.jx-tokenizer-token-disabled:hover { + border-color: {$sh-greyborder}; + background: {$greybackground}; +} + +.jx-tokenizer-token-disabled .phui-icon-view { + color: {$sh-greyicon}; +} + +a.jx-tokenizer-token-invalid { + border-color: {$sh-lightredborder}; + background: {$sh-redbackground}; + color: {$sh-redtext}; +} + +a.jx-tokenizer-token-invalid:hover { + border-color: {$sh-redborder}; + background: {$lightred}; +} + +.jx-tokenizer-token-invalid .phui-icon-view { + color: {$sh-redicon}; +} + .tokenizer-result { position: relative; padding: 5px 8px 5px 28px; diff --git a/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js b/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js index 93e540ece3..8de7913368 100644 --- a/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js +++ b/webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js @@ -348,16 +348,22 @@ JX.install('Tokenizer', { }, '\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', { + var attrs = { className: 'jx-tokenizer-token', sigil: 'token', meta: {key: key} - }, [display_token, input, remove]); + }; + var container = JX.$N('a', attrs); + + var render_callback = this.getRenderTokenCallback(); + if (render_callback) { + display_token = render_callback(value, key, container); + } + + JX.DOM.setContent(container, [display_token, input, remove]); + + return container; }, getTokens : function() { diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index 9120f95242..085a2a4e1a 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -172,23 +172,27 @@ JX.install('Prefab', { var tokenizer = new JX.Tokenizer(root); tokenizer.setTypeahead(typeahead); - tokenizer.setRenderTokenCallback(function(value, key) { + tokenizer.setRenderTokenCallback(function(value, key, container) { var result = datasource.getResult(key); var icon; + var type; if (result) { icon = result.icon; value = result.displayName; + type = result.tokenType; } else { icon = config.icons[key]; + type = config.types[key]; } if (icon) { icon = JX.Prefab._renderIcon(icon); } - // TODO: Maybe we should render these closed tags in grey? Figure out - // how we're going to use color. + if (type) { + JX.DOM.alterClass(container, 'jx-tokenizer-token-' + type, true); + } return [icon, value]; }); @@ -288,7 +292,8 @@ JX.install('Prefab', { closed: closed, type: fields[5], sprite: fields[10], - unique: fields[11] || false + tokenType: fields[11], + unique: fields[12] || false }; },