From b348aaefb97aa28ab85deef572e7acfdbbf4e1c0 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Tue, 6 Aug 2013 09:20:04 -0700 Subject: [PATCH] Add Hovercards / restyle feed one line stories. Summary: This adds hovercards to most stories and removes the profile photo from one line stories. I don't know about my implementation, which has difficulties with application transactions (because it shows status). Which leads me to a bigger question, which is can we render all people through a common function like AphrontTagView so we can easily class and/or hovercard it anywhere. Test Plan: Reviewed my feed, various stories. Reviewers: epriestley, btrahan Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D6684 --- src/__celerity_resource_map__.php | 4 +- .../feed/story/PhabricatorFeedStory.php | 10 ++- .../story/PhabricatorFeedStoryPhriction.php | 10 +-- .../story/PhabricatorFeedStoryProject.php | 66 ++++++++++++------- .../feed/story/PhabricatorFeedStoryStatus.php | 2 +- .../phid/PhabricatorObjectHandle.php | 12 ++-- src/view/phui/PHUIFeedStoryView.php | 3 +- webroot/rsrc/css/phui/phui-feed-story.css | 6 ++ 8 files changed, 77 insertions(+), 36 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 5528c06f04..03909c0259 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -3791,7 +3791,7 @@ celerity_register_resource_map(array( ), 'phui-feed-story-css' => array( - 'uri' => '/res/5d7ab26c/rsrc/css/phui/phui-feed-story.css', + 'uri' => '/res/68a0ce41/rsrc/css/phui/phui-feed-story.css', 'type' => 'css', 'requires' => array( @@ -3827,7 +3827,7 @@ celerity_register_resource_map(array( ), 'phui-remarkup-preview-css' => array( - 'uri' => '/res/6c886e63/rsrc/css/phui/phui-remarkup-preview.css', + 'uri' => '/res/4535e062/rsrc/css/phui/phui-remarkup-preview.css', 'type' => 'css', 'requires' => array( diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index c9d5c35791..646799103b 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -232,11 +232,19 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { // NOTE: We render our own link here to customize the styling and add // the '_top' target for framed feeds. - return phutil_tag( + $class = null; + if ($handle->getType() == PhabricatorPeoplePHIDTypeUser::TYPECONST) { + $class = 'phui-link-person'; + } + + return javelin_tag( 'a', array( 'href' => $handle->getURI(), 'target' => $this->framed ? '_top' : null, + 'sigil' => 'hovercard', + 'meta' => array('hoverPHID' => $phid), + 'class' => $class, ), $handle->getLinkName()); } diff --git a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php index e4c18b3e7a..e9bd0b141e 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php +++ b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php @@ -19,6 +19,7 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { $data = $this->getStoryData(); $author_phid = $data->getAuthorPHID(); + $author_link = $this->linkTo($author_phid); $document_phid = $data->getValue('phid'); $view = $this->newStoryView(); @@ -38,7 +39,7 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { $from_handle = $this->getHandle($from_phid); $view->setTitle(pht( '%s moved the document %s from %s to %s.', - $this->linkTo($author_phid), + $author_link, $document_handle->renderLink(), phutil_tag( 'a', @@ -58,7 +59,7 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { default: $view->setTitle(pht( '%s %s the document %s.', - $this->linkTo($author_phid), + $author_link, $verb, $this->linkTo($document_phid))); break; @@ -76,7 +77,8 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { } public function renderText() { - $author_name = $this->getHandle($this->getAuthorPHID())->getLinkName(); + $author_phid = $this->getHandle($this->getAuthorPHID()); + $author_link = $this->linkTo($author_phid); $document_handle = $this->getHandle($this->getPrimaryObjectPHID()); $document_title = $document_handle->getLinkName(); @@ -85,7 +87,7 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { $action = $this->getValue('action'); $verb = PhrictionActionConstants::getActionPastTenseVerb($action); - $text = "{$author_name} {$verb} the document". + $text = "{$author_link} {$verb} the document". " {$document_title} {$document_uri}"; return $text; diff --git a/src/applications/feed/story/PhabricatorFeedStoryProject.php b/src/applications/feed/story/PhabricatorFeedStoryProject.php index e18ad7bdd5..e45b350ebc 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryProject.php +++ b/src/applications/feed/story/PhabricatorFeedStoryProject.php @@ -33,18 +33,21 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory { $proj_phid = $data->getValue('projectPHID'); $author_phid = $data->getAuthorPHID(); + $author_link = $this->linkTo($author_phid); switch ($type) { case PhabricatorProjectTransactionType::TYPE_NAME: if (strlen($old)) { - $action = hsprintf( - 'renamed project %s from %s to %s.', + $action = pht( + '% renamed project %s from %s to %s.', + $author_link, $this->linkTo($proj_phid), $this->renderString($old), $this->renderString($new)); } else { - $action = hsprintf( - 'created project %s (as %s).', + $action = pht( + '% created project %s (as %s).', + $author_link, $this->linkTo($proj_phid), $this->renderString($new)); } @@ -52,8 +55,9 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory { case PhabricatorProjectTransactionType::TYPE_STATUS: $old_name = PhabricatorProjectStatus::getNameForStatus($old); $new_name = PhabricatorProjectStatus::getNameForStatus($new); - $action = hsprintf( - 'changed project %s status from %s to %s.', + $action = pht( + '%s changed project %s status from %s to %s.', + $author_link, $this->linkTo($proj_phid), $this->renderString($old_name), $this->renderString($new_name)); @@ -64,48 +68,64 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory { if ((count($add) == 1) && (count($rem) == 0) && (head($add) == $author_phid)) { - $action = hsprintf('joined project %s.', $this->linkTo($proj_phid)); + $action = pht( + '% joined project %s.', + $author_link, + $this->linkTo($proj_phid)); } else if ((count($add) == 0) && (count($rem) == 1) && (head($rem) == $author_phid)) { - $action = hsprintf('left project %s.', $this->linkTo($proj_phid)); + $action = pht( + '%s left project %s.', + $author_link, + $this->linkTo($proj_phid)); } else if (empty($rem)) { - $action = hsprintf( - 'added members to project %s: %s.', + $action = pht( + '%s added members to project %s: %s.', + $author_link, $this->linkTo($proj_phid), $this->renderHandleList($add)); } else if (empty($add)) { - $action = hsprintf( - 'removed members from project %s: %s.', + $action = pht( + '%s removed members from project %s: %s.', + $author_link, $this->linkTo($proj_phid), $this->renderHandleList($rem)); } else { - $action = hsprintf( - 'changed members of project %s, added: %s; removed: %s.', + $action = pht( + '%s changed members of project %s, added: %s; removed: %s.', + $author_link, $this->linkTo($proj_phid), $this->renderHandleList($add), $this->renderHandleList($rem)); } break; case PhabricatorProjectTransactionType::TYPE_CAN_VIEW: - $action = hsprintf( - 'changed the visibility for %s.', + $action = pht( + '%s changed the visibility for %s.', + $author_link, $this->linkTo($proj_phid)); break; case PhabricatorProjectTransactionType::TYPE_CAN_EDIT: - $action = hsprintf( - 'changed the edit policy for %s.', + $action = pht( + '%s changed the edit policy for %s.', + $author_link, $this->linkTo($proj_phid)); break; case PhabricatorProjectTransactionType::TYPE_CAN_JOIN: - $action = hsprintf( - 'changed the join policy for %s.', + $action = pht( + '%s changed the join policy for %s.', + $author_link, $this->linkTo($proj_phid)); break; default: - $action = hsprintf('updated project %s.', $this->linkTo($proj_phid)); + $action = pht( + '%s updated project %s.', + $author_link, + $this->linkTo($proj_phid)); break; } - $view->setTitle(hsprintf('%s %s', $this->linkTo($author_phid), $action)); + + $view->setTitle($action); $view->setImage($this->getHandle($author_phid)->getImageURI()); return $view; @@ -121,7 +141,7 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory { $proj_uri = PhabricatorEnv::getURI($proj_handle->getURI()); $author_phid = $this->getAuthorPHID(); - $author_name = $this->getHandle($author_phid)->getLinkName(); + $author_name = $this->linkTo($author_phid); switch ($type) { case PhabricatorProjectTransactionType::TYPE_NAME: diff --git a/src/applications/feed/story/PhabricatorFeedStoryStatus.php b/src/applications/feed/story/PhabricatorFeedStoryStatus.php index 425847f918..34d4b6937d 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryStatus.php +++ b/src/applications/feed/story/PhabricatorFeedStoryStatus.php @@ -28,7 +28,7 @@ final class PhabricatorFeedStoryStatus extends PhabricatorFeedStory { $author_name = $author_handle->getLinkName(); $author_uri = PhabricatorEnv::getURI($author_handle->getURI()); - $text = pht('% supdated their status %s', $author_name, $author_uri); + $text = pht('% updated their status %s', $author_name, $author_uri); return $text; } diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 6832c213c6..d9bfcda361 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -172,24 +172,28 @@ final class PhabricatorObjectHandle if ($name === null) { $name = $this->getLinkName(); } - $class = null; + $classes = array(); $title = $this->title; if ($this->status != PhabricatorObjectHandleStatus::STATUS_OPEN) { - $class .= ' handle-status-'.$this->status; + $classes[] = 'handle-status-'.$this->status; $title = $title ? $title : $this->status; } if ($this->disabled) { - $class .= ' handle-disabled'; + $classes[] = 'handle-disabled'; $title = 'disabled'; // Overwrite status. } + if ($this->getType() == PhabricatorPeoplePHIDTypeUser::TYPECONST) { + $classes[] = 'phui-link-person'; + } + return phutil_tag( 'a', array( 'href' => $this->getURI(), - 'class' => $class, + 'class' => implode(' ', $classes), 'title' => $title, ), $name); diff --git a/src/view/phui/PHUIFeedStoryView.php b/src/view/phui/PHUIFeedStoryView.php index e15979dc22..ad7f8e812a 100644 --- a/src/view/phui/PHUIFeedStoryView.php +++ b/src/view/phui/PHUIFeedStoryView.php @@ -123,6 +123,7 @@ final class PHUIFeedStoryView extends AphrontView { public function render() { require_celerity_resource('phui-feed-story-css'); + Javelin::initBehavior('phabricator-hovercards'); $oneline = $this->isEmptyContent($this->renderChildren()); $body = null; @@ -204,7 +205,7 @@ final class PHUIFeedStoryView extends AphrontView { 'class' => 'phui-feed-story-head', ), array( - $actor, + (!$oneline ? $actor : null), nonempty($this->title, pht('Untitled Story')), $icons, $ol_foot diff --git a/webroot/rsrc/css/phui/phui-feed-story.css b/webroot/rsrc/css/phui/phui-feed-story.css index 3352594a7d..e0f7ff9abb 100644 --- a/webroot/rsrc/css/phui/phui-feed-story.css +++ b/webroot/rsrc/css/phui/phui-feed-story.css @@ -19,7 +19,12 @@ .phui-feed-story-head { padding: 8px; overflow: hidden; + color: #555; +} + +.phui-feed-story-head .phui-link-person { color: #333; + font-weight: bold; } .phui-feed-story-body { @@ -99,5 +104,6 @@ color: #777; margin-top: 2px; line-height: 14px; + font-weight: normal; }