From 87012d60f1d04a12e77377f4b390ff279f46c010 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 14 Feb 2014 10:24:11 -0800 Subject: [PATCH] Show icons and disabled/archived/closed results in typahead dynamic list Summary: Ref T4420. This is mostly a design change, but addresses two functional issues: # Many sources exclude disabled accounts, system agents, archived projects, etc. It is rare to select these, but excluding them completely is too severe, and we've made more than a handful of changes over time to replace a "users" endpoint with an "accounts" endpoint (to include disabled users) or similar. Instead, always show these results, but sort them last and use a special style to clearly mark them as closed, disabled, or otherwise unusual. - As a practical consequence, all the similar endpoints can now be merged, so "accounts" and "users" return the exact same result sets. # Increasingly, sources can return multiple object types in a single list. For example, "CC" can have a user or mailing list, and soon a project or repository. However, the result list is fairly homogenous across types and it isn't easy to quickly pick out projects vs users. To help with this, add icons showing the result type. Test Plan: {F113079} (The main search results get touched here too, I verified they didn't blow up.) Reviewers: chad, btrahan Reviewed By: chad CC: chad, aran, mbishopim3 Maniphest Tasks: T4420 Differential Revision: https://secure.phabricator.com/D8231 --- resources/celerity/map.php | 54 ++++++++--------- ...torTypeaheadCommonDatasourceController.php | 43 +++++--------- .../storage/PhabricatorTypeaheadResult.php | 7 +++ webroot/rsrc/css/aphront/tokenizer.css | 23 +++++++- webroot/rsrc/css/aphront/typeahead.css | 1 - .../css/application/base/main-menu-view.css | 4 +- webroot/rsrc/js/core/Prefab.js | 59 +++++++++++++++++-- 7 files changed, 126 insertions(+), 65 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index edfe669c7b..08616461f0 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,8 +7,8 @@ return array( 'names' => array( - 'core.pkg.css' => 'd0936e05', - 'core.pkg.js' => '51eb4e66', + 'core.pkg.css' => '044c2f0c', + 'core.pkg.js' => 'ba6e232d', 'darkconsole.pkg.js' => 'ca8671ce', 'differential.pkg.css' => '6aef439e', 'differential.pkg.js' => '322ea941', @@ -33,13 +33,13 @@ 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' => '8e7ae263', + 'rsrc/css/aphront/tokenizer.css' => '08ea6326', 'rsrc/css/aphront/tooltip.css' => '9c90229d', 'rsrc/css/aphront/transaction.css' => 'ce491938', 'rsrc/css/aphront/two-column.css' => '16ab3ad2', - 'rsrc/css/aphront/typeahead.css' => '00c9a200', + 'rsrc/css/aphront/typeahead.css' => '104a6525', 'rsrc/css/application/auth/auth.css' => '1e655982', - 'rsrc/css/application/base/main-menu-view.css' => '1de0ef6f', + 'rsrc/css/application/base/main-menu-view.css' => 'd36e0c11', 'rsrc/css/application/base/notification-menu.css' => 'fc9a363c', 'rsrc/css/application/base/phabricator-application-launch-view.css' => '55ba7571', 'rsrc/css/application/base/standard-page-view.css' => '517cdfb1', @@ -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' => '6c78666a', + 'rsrc/js/core/Prefab.js' => '1e644329', 'rsrc/js/core/ShapedRequest.js' => 'dfa181a4', 'rsrc/js/core/TextAreaUtils.js' => 'b3ec3cfc', 'rsrc/js/core/ToolTip.js' => '0a81ea29', @@ -489,10 +489,10 @@ return array( 'aphront-panel-view-css' => '5846dfa2', 'aphront-request-failure-view-css' => 'da14df31', 'aphront-table-view-css' => '92a719ca', - 'aphront-tokenizer-control-css' => '8e7ae263', + 'aphront-tokenizer-control-css' => '08ea6326', 'aphront-tooltip-css' => '9c90229d', 'aphront-two-column-view-css' => '16ab3ad2', - 'aphront-typeahead-control-css' => '00c9a200', + 'aphront-typeahead-control-css' => '104a6525', 'auth-css' => '1e655982', 'config-options-css' => '7fedf08b', 'conpherence-menu-css' => '561348ac', @@ -692,7 +692,7 @@ return array( 'phabricator-jump-nav' => 'f0c5e726', 'phabricator-keyboard-shortcut' => '1ae869f2', 'phabricator-keyboard-shortcut-manager' => 'ad7a69ca', - 'phabricator-main-menu-view' => '1de0ef6f', + 'phabricator-main-menu-view' => 'd36e0c11', 'phabricator-menu-item' => '0f386ef4', 'phabricator-nav-view-css' => 'd0d4a509', 'phabricator-notification' => '95944043', @@ -701,7 +701,7 @@ return array( 'phabricator-object-list-view-css' => '1a1ea560', 'phabricator-object-selector-css' => '029a133d', 'phabricator-phtize' => 'd254d646', - 'phabricator-prefab' => '6c78666a', + 'phabricator-prefab' => '1e644329', 'phabricator-profile-css' => '3a7e04ca', 'phabricator-project-tag-css' => '095c9404', 'phabricator-remarkup-css' => 'ca7f2265', @@ -836,6 +836,10 @@ return array( array( 0 => 'javelin-install', ), + '08ea6326' => + array( + 0 => 'aphront-typeahead-control-css', + ), '0a81ea29' => array( 0 => 'javelin-install', @@ -924,6 +928,19 @@ return array( 5 => 'phabricator-drag-and-drop-file-upload', 6 => 'phabricator-draggable-list', ), + '1e644329' => + 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', + ), '1f595fb0' => array( 0 => 'javelin-install', @@ -1205,19 +1222,6 @@ 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', @@ -1361,10 +1365,6 @@ return array( 2 => 'javelin-stratcom', 3 => 'javelin-uri', ), - '8e7ae263' => - array( - 0 => 'aphront-typeahead-control-css', - ), '8f24abfc' => array( 0 => 'javelin-behavior', diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php index d19b2d0775..97e6ebcb7a 100644 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php +++ b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php @@ -24,7 +24,6 @@ final class PhabricatorTypeaheadCommonDatasourceController $need_users = false; $need_agents = false; $need_applications = false; - $need_all_users = false; $need_lists = false; $need_projs = false; $need_repos = false; @@ -56,25 +55,20 @@ final class PhabricatorTypeaheadCommonDatasourceController $need_noproject = true; break; case 'users': - $need_users = true; - break; + case 'accounts': case 'authors': $need_users = true; - $need_agents = true; break; case 'mailable': - $need_users = true; - $need_lists = true; - break; case 'allmailable': $need_users = true; - $need_all_users = true; $need_lists = true; break; case 'projects': $need_projs = true; break; case 'usersorprojects': + case 'accountsorprojects': $need_users = true; $need_projs = true; break; @@ -84,15 +78,6 @@ final class PhabricatorTypeaheadCommonDatasourceController case 'packages': $need_packages = true; break; - case 'accounts': - $need_users = true; - $need_all_users = true; - break; - case 'accountsorprojects': - $need_users = true; - $need_all_users = true; - $need_projs = true; - break; case 'arcanistprojects': $need_arcanist_projects = true; break; @@ -195,13 +180,11 @@ final class PhabricatorTypeaheadCommonDatasourceController } foreach ($users as $user) { - if (!$need_all_users) { - if (!$need_agents && $user->getIsSystemAgent()) { - continue; - } - if ($user->getIsDisabled()) { - continue; - } + $closed = null; + if ($user->getIsDisabled()) { + $closed = pht('Disabled'); + } else if ($user->getIsSystemAgent()) { + $closed = pht('System Agent'); } $result = id(new PhabricatorTypeaheadResult()) @@ -210,7 +193,8 @@ final class PhabricatorTypeaheadCommonDatasourceController ->setPHID($user->getPHID()) ->setPriorityString($user->getUsername()) ->setIcon('policy-all') - ->setPriorityType('user'); + ->setPriorityType('user') + ->setClosed($closed); if ($need_rich_data) { $display_type = 'User'; @@ -285,16 +269,21 @@ final class PhabricatorTypeaheadCommonDatasourceController if ($need_projs) { $projs = id(new PhabricatorProjectQuery()) ->setViewer($viewer) - ->withStatus(PhabricatorProjectQuery::STATUS_OPEN) ->needImages(true) ->execute(); foreach ($projs as $proj) { + $closed = null; + if ($proj->isArchived()) { + $closed = pht('Archived'); + } + $proj_result = id(new PhabricatorTypeaheadResult()) ->setName($proj->getName()) ->setDisplayType("Project") ->setURI('/project/view/'.$proj->getID().'/') ->setPHID($proj->getPHID()) - ->setIcon('policy-project'); + ->setIcon('policy-project') + ->setClosed($closed); $proj_result->setImageURI($proj->getProfileImageURI()); diff --git a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php index f6b2a8ce48..581d26d566 100644 --- a/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php +++ b/src/applications/typeahead/storage/PhabricatorTypeaheadResult.php @@ -11,6 +11,7 @@ final class PhabricatorTypeaheadResult { private $imageURI; private $priorityType; private $icon; + private $closed; public function setIcon($icon) { $this->icon = $icon; @@ -57,6 +58,11 @@ final class PhabricatorTypeaheadResult { return $this; } + public function setClosed($closed) { + $this->closed = $closed; + return $this; + } + public function getWireFormat() { $data = array( $this->name, @@ -68,6 +74,7 @@ final class PhabricatorTypeaheadResult { $this->imageURI ? (string)$this->imageURI : null, $this->priorityType, $this->icon, + $this->closed, ); while (end($data) === null) { array_pop($data); diff --git a/webroot/rsrc/css/aphront/tokenizer.css b/webroot/rsrc/css/aphront/tokenizer.css index 6403da91fd..0ebee5b0fe 100644 --- a/webroot/rsrc/css/aphront/tokenizer.css +++ b/webroot/rsrc/css/aphront/tokenizer.css @@ -80,7 +80,24 @@ a.jx-tokenizer-token:hover { margin: 2px 4px -2px 0; } -.tokenizer-result-type { - margin-left: 4px; - color: {$lightgreytext}; +.tokenizer-result { + position: relative; + padding: 5px 8px 5px 28px; +} + +.tokenizer-result .phui-icon-view { + display: inline-block; + width: 24px; + height: 24px; + position: absolute; + top: 5px; + left: 8px; +} + +.tokenizer-result-closed { + color: {$greytext}; +} + +.tokenizer-closed { + margin-top: 2px; } diff --git a/webroot/rsrc/css/aphront/typeahead.css b/webroot/rsrc/css/aphront/typeahead.css index 30a25c0a94..fcf983a72d 100644 --- a/webroot/rsrc/css/aphront/typeahead.css +++ b/webroot/rsrc/css/aphront/typeahead.css @@ -21,7 +21,6 @@ div.jx-typeahead-results { div.jx-typeahead-results a.jx-result { color: #333; display: block; - padding: 5px 8px; font-size: 13px; border-bottom: 1px solid #e7e7e7; border-top: 1px solid #fff; diff --git a/webroot/rsrc/css/application/base/main-menu-view.css b/webroot/rsrc/css/application/base/main-menu-view.css index 36688ca883..d26b9f84b9 100644 --- a/webroot/rsrc/css/application/base/main-menu-view.css +++ b/webroot/rsrc/css/application/base/main-menu-view.css @@ -206,8 +206,8 @@ .phabricator-main-search-typeahead-result { display: block; - padding: 1px 4px 1px 38px; - background-position: 4px 4px; + padding: 6px 12px 6px 46px; + background-position: 12px 9px; background-size: 25px 25px; background-repeat: no-repeat; } diff --git a/webroot/rsrc/js/core/Prefab.js b/webroot/rsrc/js/core/Prefab.js index 121ca6cce9..19321cab72 100644 --- a/webroot/rsrc/js/core/Prefab.js +++ b/webroot/rsrc/js/core/Prefab.js @@ -95,6 +95,24 @@ JX.install('Prefab', { return self_hits[v.id] ? 1 : -1; } + // If one result is open and one is closed, show the open result + // first. The "!" tricks here are becaused closed values are display + // strings, so the value is either `null` or some truthy string. If + // we compare the values directly, we'll apply this rule to two + // objects which are both closed but for different reasons, like + // "Archived" and "Disabled". + + var u_open = !u.closed; + var v_open = !v.closed; + + if (u_open != v_open) { + if (u_open) { + return -1; + } else { + return 1; + } + } + if (priority_hits[u.id] != priority_hits[v.id]) { return priority_hits[v.id] ? 1 : -1; } @@ -113,17 +131,47 @@ JX.install('Prefab', { }); }; + var render_icon = function(icon) { + return JX.$N( + 'span', + {className: 'phui-icon-view sprite-status status-' + icon}); + }; + datasource.setSortHandler(JX.bind(datasource, sort_handler)); datasource.setTransformer( function(object) { + var closed = object[9]; + var closed_ui; + if (closed) { + closed_ui = JX.$N( + 'div', + {className: 'tokenizer-closed'}, + closed); + } + + var icon = object[8]; + var icon_ui; + if (icon) { + icon_ui = render_icon(icon); + } + + var display = JX.$N( + 'div', + {className: 'tokenizer-result'}, + [icon_ui, object[0], closed_ui]); + if (closed) { + JX.DOM.alterClass(display, 'tokenizer-result-closed', true); + } + return { name: object[0], - display: object[0], + display: display, uri: object[1], id: object[2], priority: object[3], priorityType: object[7], - icon: object[8] + icon: icon, + closed: closed }; }); @@ -146,9 +194,10 @@ JX.install('Prefab', { return value; } - icon = JX.$N( - 'span', - {className: 'phui-icon-view sprite-status status-' + icon}); + icon = render_icon(icon); + + // TODO: Maybe we should render these closed tags in grey? Figure out + // how we're going to use color. return [icon, value]; });