From 3ec07c4987692588169b82c9ed50acf0fa1611f1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Dec 2015 10:14:10 -0800 Subject: [PATCH] Show hovercards for most links in object property views Summary: Ref T8980. This isn't 100% coverage but should be pretty much all of the common ones. These feel a touch iffy to me at first glance so I didn't go crazy trying to hunt all of them down. I have some other plans for them so maybe they'll feel better by the end of it. Test Plan: Hovered over author, reviewers, blocked tasks, projects, etc. Reviewers: chad Reviewed By: chad Maniphest Tasks: T8980 Differential Revision: https://secure.phabricator.com/D14877 --- .../customfield/DifferentialAuthorField.php | 2 +- .../view/DifferentialReviewersView.php | 2 +- .../ManiphestTaskDetailController.php | 23 ++++++++----- .../phid/PhabricatorObjectHandle.php | 31 +++++++++++++---- .../phid/view/PHUIHandleListView.php | 9 +++-- .../phid/view/PHUIHandleTagListView.php | 9 +++++ src/applications/phid/view/PHUIHandleView.php | 33 +++++++++++++++---- .../PhabricatorProjectUIEventListener.php | 3 +- .../view/SubscriptionListStringBuilder.php | 2 +- .../PhabricatorStandardCustomFieldPHIDs.php | 2 +- 10 files changed, 89 insertions(+), 27 deletions(-) diff --git a/src/applications/differential/customfield/DifferentialAuthorField.php b/src/applications/differential/customfield/DifferentialAuthorField.php index 6e14c8bbae..bac57755ab 100644 --- a/src/applications/differential/customfield/DifferentialAuthorField.php +++ b/src/applications/differential/customfield/DifferentialAuthorField.php @@ -32,7 +32,7 @@ final class DifferentialAuthorField } public function renderPropertyViewValue(array $handles) { - return $handles[$this->getObject()->getAuthorPHID()]->renderLink(); + return $handles[$this->getObject()->getAuthorPHID()]->renderHovercardLink(); } } diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php index 5dd7c09d73..ac4e482278 100644 --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -120,7 +120,7 @@ final class DifferentialReviewersView extends AphrontView { } - $item->setTarget($handle->renderLink()); + $item->setTarget($handle->renderHovercardLink()); $view->addItem($item); } diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index e5e5083664..b2b49b43d4 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -208,19 +208,26 @@ final class ManiphestTaskDetailController extends ManiphestController { ->setObject($task) ->setActionList($actions); - $view->addProperty( - pht('Assigned To'), - $task->getOwnerPHID() - ? $handles->renderHandle($task->getOwnerPHID()) - : phutil_tag('em', array(), pht('None'))); + $owner_phid = $task->getOwnerPHID(); + if ($owner_phid) { + $assigned_to = $handles + ->renderHandle($owner_phid) + ->setShowHovercard(true); + } else { + $assigned_to = phutil_tag('em', array(), pht('None')); + } + + $view->addProperty(pht('Assigned To'), $assigned_to); $view->addProperty( pht('Priority'), ManiphestTaskPriority::getTaskPriorityName($task->getPriority())); - $view->addProperty( - pht('Author'), - $handles->renderHandle($task->getAuthorPHID())); + $author = $handles + ->renderHandle($task->getAuthorPHID()) + ->setShowHovercard(true); + + $view->addProperty(pht('Author'), $author); $source = $task->getOriginalEmailSource(); if ($source) { diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 283ad0fb63..4eb6178338 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -248,6 +248,23 @@ final class PhabricatorObjectHandle public function renderLink($name = null) { + return $this->renderLinkWithAttributes($name, array()); + } + + public function renderHovercardLink($name = null) { + Javelin::initBehavior('phabricator-hovercards'); + + $attributes = array( + 'sigil' => 'hovercard', + 'meta' => array( + 'hoverPHID' => $this->getPHID(), + ), + ); + + return $this->renderLinkWithAttributes($name, $attributes); + } + + private function renderLinkWithAttributes($name, array $attributes) { if ($name === null) { $name = $this->getLinkName(); } @@ -275,13 +292,15 @@ final class PhabricatorObjectHandle ->setIconFont('fa-lock lightgreytext'); } - return phutil_tag( + $attributes = $attributes + array( + 'href' => $uri, + 'class' => implode(' ', $classes), + 'title' => $title, + ); + + return javelin_tag( $uri ? 'a' : 'span', - array( - 'href' => $uri, - 'class' => implode(' ', $classes), - 'title' => $title, - ), + $attributes, array($icon, $name)); } diff --git a/src/applications/phid/view/PHUIHandleListView.php b/src/applications/phid/view/PHUIHandleListView.php index 7ec0ce7606..cc9f3d00bd 100644 --- a/src/applications/phid/view/PHUIHandleListView.php +++ b/src/applications/phid/view/PHUIHandleListView.php @@ -34,9 +34,14 @@ final class PHUIHandleListView } protected function getTagContent() { + $list = $this->handleList; $items = array(); - foreach ($this->handleList as $handle) { - $items[] = $handle->renderLink(); + foreach ($list as $handle) { + $view = $list->renderHandle($handle->getPHID()); + + $view->setShowHovercard(true); + + $items[] = $view; } if ($this->getAsInline()) { diff --git a/src/applications/phid/view/PHUIHandleTagListView.php b/src/applications/phid/view/PHUIHandleTagListView.php index f5ebe33d65..73cbef2350 100644 --- a/src/applications/phid/view/PHUIHandleTagListView.php +++ b/src/applications/phid/view/PHUIHandleTagListView.php @@ -7,6 +7,7 @@ final class PHUIHandleTagListView extends AphrontTagView { private $limit; private $noDataString; private $slim; + private $showHovercards; public function setHandles(array $handles) { $this->handles = $handles; @@ -33,6 +34,11 @@ final class PHUIHandleTagListView extends AphrontTagView { return $this; } + public function setShowHovercards($show_hovercards) { + $this->showHovercards = $show_hovercards; + return $this; + } + protected function getTagName() { return 'ul'; } @@ -62,6 +68,9 @@ final class PHUIHandleTagListView extends AphrontTagView { $list = array(); foreach ($handles as $handle) { $tag = $handle->renderTag(); + if ($this->showHovercards) { + $tag->setPHID($handle->getPHID()); + } if ($this->slim) { $tag->setSlimShady(true); } diff --git a/src/applications/phid/view/PHUIHandleView.php b/src/applications/phid/view/PHUIHandleView.php index a4b766a991..f4bce39662 100644 --- a/src/applications/phid/view/PHUIHandleView.php +++ b/src/applications/phid/view/PHUIHandleView.php @@ -15,6 +15,7 @@ final class PHUIHandleView private $handlePHID; private $asTag; private $useShortName; + private $showHovercard; public function setHandleList(PhabricatorHandleList $list) { $this->handleList = $list; @@ -36,17 +37,37 @@ final class PHUIHandleView return $this; } + public function setShowHovercard($hovercard) { + $this->showHovercard = $hovercard; + return $this; + } + public function render() { $handle = $this->handleList[$this->handlePHID]; + if ($this->asTag) { - return $handle->renderTag(); - } else { - if ($this->useShortName) { - return $handle->renderLink($handle->getName()); - } else { - return $handle->renderLink(); + $tag = $handle->renderTag(); + + if ($this->showHovercard) { + $tag->setPHID($handle->getPHID()); } + + return $tag; } + + if ($this->useShortName) { + $name = $handle->getName(); + } else { + $name = null; + } + + if ($this->showHovercard) { + $link = $handle->renderHovercardLink($name); + } else { + $link = $handle->renderLink($name); + } + + return $link; } } diff --git a/src/applications/project/events/PhabricatorProjectUIEventListener.php b/src/applications/project/events/PhabricatorProjectUIEventListener.php index 326a9b9878..55a4a2c4a0 100644 --- a/src/applications/project/events/PhabricatorProjectUIEventListener.php +++ b/src/applications/project/events/PhabricatorProjectUIEventListener.php @@ -101,7 +101,8 @@ final class PhabricatorProjectUIEventListener if ($handles) { $list = id(new PHUIHandleTagListView()) ->setHandles($handles) - ->setAnnotations($annotations); + ->setAnnotations($annotations) + ->setShowHovercards(true); } else { $list = phutil_tag('em', array(), pht('None')); } diff --git a/src/applications/subscriptions/view/SubscriptionListStringBuilder.php b/src/applications/subscriptions/view/SubscriptionListStringBuilder.php index e8761e554e..7ccc729241 100644 --- a/src/applications/subscriptions/view/SubscriptionListStringBuilder.php +++ b/src/applications/subscriptions/view/SubscriptionListStringBuilder.php @@ -57,7 +57,7 @@ final class SubscriptionListStringBuilder extends Phobject { // link. Instead, render "a, b, c, d" in this case, and then when we get one // more render "a, b, c, and 2 others". if ($subscribers_count <= ($show_count + 1)) { - return phutil_implode_html(', ', mpull($handles, 'renderLink')); + return phutil_implode_html(', ', mpull($handles, 'renderHovercardLink')); } $show = array_slice($handles, 0, $show_count); diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php index 855cf2b4a0..b9b1bf6505 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php @@ -83,7 +83,7 @@ abstract class PhabricatorStandardCustomFieldPHIDs return null; } - $handles = mpull($handles, 'renderLink'); + $handles = mpull($handles, 'renderHovercardLink'); $handles = phutil_implode_html(', ', $handles); return $handles; }