From 1663dc32c90c4baaefa37a5c1888d32270555216 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Thu, 1 Aug 2013 13:59:37 -0700 Subject: [PATCH] Pholio - finish off history view Summary: ...bsasically add a "view mode" and play with that throughout the stack. Differences are... - normal mode has comments; history mode does not - normal mode has inline comments; history mode does not - page uris are correct with respect to either mode ...and that's about it. I played around (wasted too much time) trying to make this cuter. I think just jamming this mode in here is the easiest / cleanest thing at the end. Feel free to tell me otherwise! This largely gets even better via T3612. However, this fixes T3572. Test Plan: played around with a mock with some history. noted correct uris on images. noted no errors in js console. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T3572 Differential Revision: https://secure.phabricator.com/D6638 --- .../PholioImageHistoryController.php | 8 ++--- .../pholio/view/PholioMockImagesView.php | 36 ++++++++++++++++--- .../pholio/behavior-pholio-mock-view.js | 30 ++++++++++------ 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/applications/pholio/controller/PholioImageHistoryController.php b/src/applications/pholio/controller/PholioImageHistoryController.php index 186a930057..7b6d17480f 100644 --- a/src/applications/pholio/controller/PholioImageHistoryController.php +++ b/src/applications/pholio/controller/PholioImageHistoryController.php @@ -27,7 +27,7 @@ final class PholioImageHistoryController extends PholioController { // note while we have a mock object, its missing images we need to show // the history of what's happened here. // fetch the real deal - // + $mock = id(new PholioMockQuery()) ->setViewer($user) ->needImages(true) @@ -44,7 +44,6 @@ final class PholioImageHistoryController extends PholioController { $images = $mock->getImageHistorySet($this->imageID); - // TODO - this is a hack until we specialize the view object $mock->attachImages($images); $latest_image = last($images); @@ -59,13 +58,14 @@ final class PholioImageHistoryController extends PholioController { require_celerity_resource('pholio-css'); require_celerity_resource('pholio-inline-comments-css'); - $comment_form_id = celerity_generate_unique_node_id(); + $comment_form_id = null; $output = id(new PholioMockImagesView()) ->setRequestURI($request->getRequestURI()) ->setCommentFormID($comment_form_id) ->setUser($user) ->setMock($mock) - ->setImageID($this->imageID); + ->setImageID($this->imageID) + ->setViewMode('history'); $crumbs = $this->buildApplicationCrumbs(); $crumbs diff --git a/src/applications/pholio/view/PholioMockImagesView.php b/src/applications/pholio/view/PholioMockImagesView.php index d8d8dd0563..0622d0a2a6 100644 --- a/src/applications/pholio/view/PholioMockImagesView.php +++ b/src/applications/pholio/view/PholioMockImagesView.php @@ -9,6 +9,20 @@ final class PholioMockImagesView extends AphrontView { private $imageID; private $requestURI; private $commentFormID; + private $viewMode = 'normal'; + + /** + * Supports normal (/MX, /MX/Y/) and history (/pholio/image/history/Y/) + * modes. The former has handy dandy commenting functionality and the + * latter does not. + */ + public function setViewMode($view_mode) { + $this->viewMode = $view_mode; + return $this; + } + public function getViewMode() { + return $this->viewMode; + } public function setCommentFormID($comment_form_id) { $this->commentFormID = $comment_form_id; @@ -66,15 +80,17 @@ final class PholioMockImagesView extends AphrontView { $x = idx($metadata, PhabricatorFile::METADATA_IMAGE_WIDTH); $y = idx($metadata, PhabricatorFile::METADATA_IMAGE_HEIGHT); + $history_uri = '/pholio/image/history/'.$image->getID().'/'; $images[] = array( 'id' => $image->getID(), - 'fullURI' => $image->getFile()->getBestURI(), - 'pageURI' => '/M'.$mock->getID().'/'.$image->getID().'/', - 'historyURI' => '/pholio/image/history/'.$image->getID().'/', + 'fullURI' => $file->getBestURI(), + 'pageURI' => $this->getImagePageURI($image, $mock), + 'historyURI' => $history_uri, 'width' => $x, 'height' => $y, 'title' => $image->getName(), 'desc' => $image->getDescription(), + 'isObsolete' => (bool) $image->getIsObsolete(), ); } @@ -88,7 +104,8 @@ final class PholioMockImagesView extends AphrontView { 'images' => $images, 'selectedID' => $selected_id, 'loggedIn' => $this->getUser()->isLoggedIn(), - 'logInLink' => (string) $login_uri + 'logInLink' => (string) $login_uri, + 'viewMode' => $this->getViewMode() ); Javelin::initBehavior('pholio-mock-view', $config); @@ -146,7 +163,7 @@ final class PholioMockImagesView extends AphrontView { array( 'sigil' => 'mock-thumbnail', 'class' => 'pholio-mock-carousel-thumb-item', - 'href' => '/M'.$mock->getID().'/'.$image->getID().'/', + 'href' => $this->getImagePageURI($image, $mock), 'meta' => array( 'imageID' => $image->getID(), ), @@ -173,4 +190,13 @@ final class PholioMockImagesView extends AphrontView { return $mockview; } + + private function getImagePageURI(PholioImage $image, PholioMock $mock) { + if ($this->getViewMode() == 'normal') { + $uri = '/M'.$mock->getID().'/'.$image->getID().'/'; + } else { + $uri = '/pholio/image/history/'.$image->getID().'/'; + } + return $uri; + } } diff --git a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js index 964a86d21b..9fa332caa1 100644 --- a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js +++ b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js @@ -264,6 +264,10 @@ JX.behavior('pholio-mock-view', function(config) { return; } + if (config.viewMode == 'history') { + return; + } + if (drag_begin) { return; } @@ -631,7 +635,7 @@ JX.behavior('pholio-mock-view', function(config) { } load_inline_comments(); - if (config.loggedIn) { + if (config.loggedIn && config.commentFormID) { JX.DOM.invoke(JX.$(config.commentFormID), 'shouldRefresh'); } @@ -683,12 +687,14 @@ JX.behavior('pholio-mock-view', function(config) { image.desc); info.push(desc); - var embed = JX.$N( - 'div', - {className: 'pholio-image-embedding'}, - JX.$H('Embed this image:
{M' + config.mockID + + if (!image.isObsolete) { + var embed = JX.$N( + 'div', + {className: 'pholio-image-embedding'}, + JX.$H('Embed this image:
{M' + config.mockID + ', image=' + image.id + '}')); - info.push(embed); + info.push(embed); + } // Render image dimensions and visible size. If we have this infomation // from the server we can display some of it immediately; otherwise, we need @@ -728,11 +734,13 @@ JX.behavior('pholio-mock-view', function(config) { 'View Full Image'); info.push(full_link); - var history_link = JX.$N( - 'a', - { href: image.historyURI }, - 'View Image History'); - info.push(history_link); + if (config.viewMode != 'history') { + var history_link = JX.$N( + 'a', + { href: image.historyURI }, + 'View Image History'); + info.push(history_link); + } for (var ii = 0; ii < info.length; ii++) {