From 7ce8a1f437ae7fd17f59cb12f080d5a9a4cff326 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 14 Jun 2014 21:12:19 -0700 Subject: [PATCH] Turn thumbs into a history grid thing Summary: This could probably use some refinement (and, like, explanatory text, and stronger cues about what rows and columns mean) but feels fairly good to me, at least on test data. I didn't do any scrolling for now since we have to do full height on mobile anyway I think. I did swap it so the newer ones are on top. Left/right navigate you among current images only, but you can click any thumb to review history. Removed history view since it's no longer useful. Some things that would probably help: - Some kind of header explaining what this is ("Mock History" or something). - Stronger visual cue that columns are related by being the same image. - Clearer cues about obsolete/deleted images (e.g., on the stage itself?) - Maybe general tweaks. - Maybe a placeholder (like a grey "X") for images which have been deleted. (I'm planning to add comment counts too, which I think will be pretty useful, but that felt good to put in another diff.) Test Plan: See screenshots. Reviewers: chad Reviewed By: chad Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D9543 --- src/__phutil_library_map__.php | 4 +- .../PhabricatorFileTransformController.php | 3 + .../files/storage/PhabricatorFile.php | 6 + .../PhabricatorApplicationPholio.php | 1 - .../PholioImageHistoryController.php | 90 ------------ .../controller/PholioMockViewController.php | 51 +------ .../pholio/view/PholioMockImagesView.php | 85 +++-------- .../pholio/view/PholioMockThumbGridView.php | 135 ++++++++++++++++++ .../rsrc/css/application/pholio/pholio.css | 49 +++---- .../pholio/behavior-pholio-mock-view.js | 37 +++-- 10 files changed, 205 insertions(+), 256 deletions(-) delete mode 100644 src/applications/pholio/controller/PholioImageHistoryController.php create mode 100644 src/applications/pholio/view/PholioMockThumbGridView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f79605a29f..629b852cb0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2410,7 +2410,6 @@ phutil_register_library_map(array( 'PholioController' => 'applications/pholio/controller/PholioController.php', 'PholioDAO' => 'applications/pholio/storage/PholioDAO.php', 'PholioImage' => 'applications/pholio/storage/PholioImage.php', - 'PholioImageHistoryController' => 'applications/pholio/controller/PholioImageHistoryController.php', 'PholioImageQuery' => 'applications/pholio/query/PholioImageQuery.php', 'PholioImageUploadController' => 'applications/pholio/controller/PholioImageUploadController.php', 'PholioInlineController' => 'applications/pholio/controller/PholioInlineController.php', @@ -2426,6 +2425,7 @@ phutil_register_library_map(array( 'PholioMockMailReceiver' => 'applications/pholio/mail/PholioMockMailReceiver.php', 'PholioMockQuery' => 'applications/pholio/query/PholioMockQuery.php', 'PholioMockSearchEngine' => 'applications/pholio/query/PholioMockSearchEngine.php', + 'PholioMockThumbGridView' => 'applications/pholio/view/PholioMockThumbGridView.php', 'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php', 'PholioPHIDTypeImage' => 'applications/pholio/phid/PholioPHIDTypeImage.php', 'PholioPHIDTypeMock' => 'applications/pholio/phid/PholioPHIDTypeMock.php', @@ -5312,7 +5312,6 @@ phutil_register_library_map(array( 1 => 'PhabricatorMarkupInterface', 2 => 'PhabricatorPolicyInterface', ), - 'PholioImageHistoryController' => 'PholioController', 'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PholioImageUploadController' => 'PholioController', 'PholioInlineController' => 'PholioController', @@ -5338,6 +5337,7 @@ phutil_register_library_map(array( 'PholioMockMailReceiver' => 'PhabricatorObjectMailReceiver', 'PholioMockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PholioMockSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PholioMockThumbGridView' => 'AphrontView', 'PholioMockViewController' => 'PholioController', 'PholioPHIDTypeImage' => 'PhabricatorPHIDType', 'PholioPHIDTypeMock' => 'PhabricatorPHIDType', diff --git a/src/applications/files/controller/PhabricatorFileTransformController.php b/src/applications/files/controller/PhabricatorFileTransformController.php index cdbb6a31c2..6e1002c514 100644 --- a/src/applications/files/controller/PhabricatorFileTransformController.php +++ b/src/applications/files/controller/PhabricatorFileTransformController.php @@ -65,6 +65,9 @@ final class PhabricatorFileTransformController case 'thumb-220x165': $xformed_file = $this->executeThumbTransform($file, 220, 165); break; + case 'preview-100': + $xformed_file = $this->executePreviewTransform($file, 100); + break; case 'preview-140': $xformed_file = $this->executePreviewTransform($file, 140); break; diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 8de85ee389..8aa4b94c26 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -538,6 +538,12 @@ final class PhabricatorFile extends PhabricatorFileDAO return PhabricatorEnv::getCDNURI($path); } + public function getPreview100URI() { + $path = '/file/xform/preview-100/'.$this->getPHID().'/' + .$this->getSecretKey().'/'; + return PhabricatorEnv::getCDNURI($path); + } + public function getPreview140URI() { $path = '/file/xform/preview-140/'.$this->getPHID().'/' .$this->getSecretKey().'/'; diff --git a/src/applications/pholio/application/PhabricatorApplicationPholio.php b/src/applications/pholio/application/PhabricatorApplicationPholio.php index d2c40512e6..9c7bdbdd96 100644 --- a/src/applications/pholio/application/PhabricatorApplicationPholio.php +++ b/src/applications/pholio/application/PhabricatorApplicationPholio.php @@ -53,7 +53,6 @@ final class PhabricatorApplicationPholio extends PhabricatorApplication { ), 'image/' => array( 'upload/' => 'PholioImageUploadController', - 'history/(?P\d+)/' => 'PholioImageHistoryController', ), ), ); diff --git a/src/applications/pholio/controller/PholioImageHistoryController.php b/src/applications/pholio/controller/PholioImageHistoryController.php deleted file mode 100644 index 131c323304..0000000000 --- a/src/applications/pholio/controller/PholioImageHistoryController.php +++ /dev/null @@ -1,90 +0,0 @@ -imageID = $data['id']; - } - - public function processRequest() { - $request = $this->getRequest(); - $user = $request->getUser(); - - $image = id(new PholioImageQuery()) - ->setViewer($user) - ->withIDs(array($this->imageID)) - ->executeOne(); - - if (!$image) { - return new Aphront404Response(); - } - - // 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) - ->withIDs(array($image->getMockID())) - ->executeOne(); - - $phids = array($mock->getAuthorPHID()); - $this->loadHandles($phids); - - $engine = id(new PhabricatorMarkupEngine()) - ->setViewer($user); - $engine->addObject($mock, PholioMock::MARKUP_FIELD_DESCRIPTION); - $engine->process(); - - - $images = $mock->getImageHistorySet($this->imageID); - $mock->attachImages($images); - $latest_image = last($images); - - $title = pht( - 'Image history for "%s" from the mock "%s."', - $latest_image->getName(), - $mock->getName()); - - $header = id(new PHUIHeaderView()) - ->setHeader($title); - - require_celerity_resource('pholio-css'); - require_celerity_resource('pholio-inline-comments-css'); - - $comment_form_id = null; - $output = id(new PholioMockImagesView()) - ->setRequestURI($request->getRequestURI()) - ->setCommentFormID($comment_form_id) - ->setUser($user) - ->setMock($mock) - ->setImageID($this->imageID) - ->setViewMode('history'); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs - ->addTextCrumb('M'.$mock->getID(), '/M'.$mock->getID()) - ->addTextCrumb('Image History', $request->getRequestURI()); - - $content = array( - $crumbs, - $header, - $output->render(), - ); - - return $this->buildApplicationPage( - $content, - array( - 'title' => 'M'.$mock->getID().' '.$title, - 'device' => true, - 'pageObjects' => array($mock->getPHID()), - )); - } - -} diff --git a/src/applications/pholio/controller/PholioMockViewController.php b/src/applications/pholio/controller/PholioMockViewController.php index ad2af6c984..20c869178b 100644 --- a/src/applications/pholio/controller/PholioMockViewController.php +++ b/src/applications/pholio/controller/PholioMockViewController.php @@ -1,8 +1,5 @@ getImageStatus($mock, $this->imageID); - $comment_form_id = celerity_generate_unique_node_id(); $output = id(new PholioMockImagesView()) ->setRequestURI($request->getRequestURI()) @@ -115,11 +110,16 @@ final class PholioMockViewController extends PholioController { ->setHeader($header) ->addPropertyList($properties); + $thumb_grid = id(new PholioMockThumbGridView()) + ->setUser($user) + ->setMock($mock); + $content = array( $crumbs, - $image_status, $object_box, - $output->render(), + $output, + phutil_tag('br'), + $thumb_grid, $xaction_view, $add_comment, ); @@ -133,43 +133,6 @@ final class PholioMockViewController extends PholioController { )); } - private function getImageStatus(PholioMock $mock, $image_id) { - $status = null; - $images = $mock->getImages(); - foreach ($images as $image) { - if ($image->getID() == $image_id) { - return $status; - } - } - - $images = $mock->getAllImages(); - $images = mpull($images, null, 'getID'); - $image = idx($images, $image_id); - - if ($image) { - $history = $mock->getImageHistorySet($image_id); - $latest_image = last($history); - $href = $this->getApplicationURI( - 'image/history/'.$latest_image->getID().'/'); - $status = id(new AphrontErrorView()) - ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) - ->setTitle(pht('The requested image is obsolete.')) - ->appendChild(phutil_tag( - 'p', - array(), - array( - pht('You are viewing this mock with the latest image set.'), - ' ', - phutil_tag( - 'a', - array('href' => $href), - pht( - 'Click here to see the history of the now obsolete image.'))))); - } - - return $status; - } - private function buildActionView(PholioMock $mock) { $user = $this->getRequest()->getUser(); diff --git a/src/applications/pholio/view/PholioMockImagesView.php b/src/applications/pholio/view/PholioMockImagesView.php index ea86b8121d..ba980e1580 100644 --- a/src/applications/pholio/view/PholioMockImagesView.php +++ b/src/applications/pholio/view/PholioMockImagesView.php @@ -9,25 +9,12 @@ 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; return $this; } + public function getCommentFormID() { return $this->commentFormID; } @@ -36,6 +23,7 @@ final class PholioMockImagesView extends AphrontView { $this->requestURI = $request_uri; return $this; } + public function getRequestURI() { return $this->requestURI; } @@ -74,7 +62,7 @@ final class PholioMockImagesView extends AphrontView { $selected_id = head_key($ids); } - foreach ($mock->getImages() as $image) { + foreach ($mock->getAllImages() as $image) { $file = $image->getFile(); $metadata = $file->getMetadata(); $x = idx($metadata, PhabricatorFile::METADATA_IMAGE_WIDTH); @@ -90,10 +78,15 @@ final class PholioMockImagesView extends AphrontView { 'height' => $y, 'title' => $image->getName(), 'desc' => $image->getDescription(), - 'isObsolete' => (bool) $image->getIsObsolete(), + 'isObsolete' => (bool)$image->getIsObsolete(), ); } + $navsequence = array(); + foreach ($mock->getImages() as $image) { + $navsequence[] = $image->getID(); + } + $login_uri = id(new PhutilURI('/login/')) ->setQueryParam('next', (string) $this->getRequestURI()); $config = array( @@ -105,7 +98,7 @@ final class PholioMockImagesView extends AphrontView { 'selectedID' => $selected_id, 'loggedIn' => $this->getUser()->isLoggedIn(), 'logInLink' => (string) $login_uri, - 'viewMode' => $this->getViewMode() + 'navsequence' => $navsequence, ); Javelin::initBehavior('pholio-mock-view', $config); @@ -149,65 +142,19 @@ final class PholioMockImagesView extends AphrontView { ), ''); - $carousel_holder = ''; - if (count($mock->getImages()) > 0) { - $thumbnails = array(); - foreach ($mock->getImages() as $image) { - $thumbfile = $image->getFile(); - - $dimensions = PhabricatorImageTransformer::getPreviewDimensions( - $thumbfile, - 140); - - $tag = phutil_tag( - 'img', - array( - 'width' => $dimensions['sdx'], - 'height' => $dimensions['sdy'], - 'src' => $thumbfile->getPreview140URI(), - 'class' => 'pholio-mock-carousel-thumbnail', - 'style' => 'top: '.floor((140 - $dimensions['sdy'] ) / 2).'px', - )); - - $thumbnails[] = javelin_tag( - 'a', - array( - 'sigil' => 'mock-thumbnail', - 'class' => 'pholio-mock-carousel-thumb-item', - 'href' => $this->getImagePageURI($image, $mock), - 'meta' => array( - 'imageID' => $image->getID(), - ), - ), - $tag); - } - - $carousel_holder = phutil_tag( - 'div', - array( - 'id' => 'pholio-mock-carousel', - 'class' => 'pholio-mock-carousel', - ), - $thumbnails); - } - $mockview[] = phutil_tag( 'div', - array( - 'class' => 'pholio-mock-image-container', - 'id' => 'pholio-mock-image-container' - ), - array($mock_wrapper, $inline_comments_holder, $carousel_holder)); + array( + 'class' => 'pholio-mock-image-container', + 'id' => 'pholio-mock-image-container' + ), + array($mock_wrapper, $inline_comments_holder)); 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().'/'; - } + $uri = '/M'.$mock->getID().'/'.$image->getID().'/'; return $uri; } } diff --git a/src/applications/pholio/view/PholioMockThumbGridView.php b/src/applications/pholio/view/PholioMockThumbGridView.php new file mode 100644 index 0000000000..6419ff305d --- /dev/null +++ b/src/applications/pholio/view/PholioMockThumbGridView.php @@ -0,0 +1,135 @@ +mock = $mock; + return $this; + } + + public function render() { + $mock = $this->mock; + + $all_images = $mock->getAllImages(); + $all_images = mpull($all_images, null, 'getPHID'); + + $history = mpull($all_images, 'getReplacesImagePHID', 'getPHID'); + + $replaced = array(); + foreach ($history as $phid => $replaces_phid) { + if ($replaces_phid) { + $replaced[$replaces_phid] = true; + } + } + + // Figure out the columns. Start with all the active images. + $images = mpull($mock->getImages(), null, 'getPHID'); + + // Now, find deleted images: obsolete images which were not replaced. + foreach ($mock->getAllImages() as $image) { + if (!$image->getIsObsolete()) { + // Image is current. + continue; + } + + if (isset($replaced[$image->getPHID()])) { + // Image was replaced. + continue; + } + + // This is an obsolete image which was not replaced, so it must be + // a deleted image. + $images[$image->getPHID()] = $image; + } + + $cols = array(); + $depth = 0; + foreach ($images as $image) { + $phid = $image->getPHID(); + + $col = array(); + + // If this is a deleted image, null out the final column. + if ($image->getIsObsolete()) { + $col[] = null; + } + + $col[] = $phid; + while ($phid && isset($history[$phid])) { + $col[] = $history[$phid]; + $phid = $history[$phid]; + } + + $cols[] = $col; + $depth = max($depth, count($col)); + } + + $grid = array(); + for ($ii = 0; $ii < $depth; $ii++) { + $row = array(); + foreach ($cols as $col) { + if (empty($col[$ii])) { + $row[] = phutil_tag('td', array(), null); + } else { + $thumb = $this->renderThumbnail($all_images[$col[$ii]]); + $row[] = phutil_tag('td', array(), $thumb); + } + } + $grid[] = phutil_tag('tr', array(), $row); + } + + $grid = phutil_tag( + 'table', + array( + 'id' => 'pholio-mock-thumb-grid', + 'class' => 'pholio-mock-thumb-grid', + ), + $grid); + + return phutil_tag( + 'div', + array( + 'class' => 'pholio-mock-thumb-grid-container', + ), + $grid); + } + + + private function renderThumbnail(PholioImage $image) { + $thumbfile = $image->getFile(); + + $dimensions = PhabricatorImageTransformer::getPreviewDimensions( + $thumbfile, + 100); + + $tag = phutil_tag( + 'img', + array( + 'width' => $dimensions['sdx'], + 'height' => $dimensions['sdy'], + 'src' => $thumbfile->getPreview100URI(), + 'class' => 'pholio-mock-thumb-grid-image', + 'style' => 'top: '.floor((100 - $dimensions['sdy'] ) / 2).'px', + )); + + $classes = array('pholio-mock-thumb-grid-item'); + if ($image->getIsObsolete()) { + $classes[] = 'pholio-mock-thumb-grid-item-obsolete'; + } + + return javelin_tag( + 'a', + array( + 'sigil' => 'mock-thumbnail', + 'class' => implode(' ', $classes), + 'href' => '#', + 'meta' => array( + 'imageID' => $image->getID(), + ), + ), + $tag); + } + +} diff --git a/webroot/rsrc/css/application/pholio/pholio.css b/webroot/rsrc/css/application/pholio/pholio.css index 0cb30ae45c..322ce5e678 100644 --- a/webroot/rsrc/css/application/pholio/pholio.css +++ b/webroot/rsrc/css/application/pholio/pholio.css @@ -10,50 +10,41 @@ background: url('/rsrc/image/texture/pholio-background.gif'); } -.pholio-mock-carousel { +.pholio-mock-thumb-grid-container { background-color: #282828; - text-align: center; + padding: 12px; + overflow-x: auto; + overflow-y: hidden; } -.pholio-mock-carousel-thumb-item { +.pholio-mock-thumb-grid { + margin: 0 auto; +} + +.pholio-mock-thumb-grid-item { display: inline-block; cursor: pointer; - width: 140px; - height: 140px; + width: 100px; + height: 100px; padding: 5px; margin: 3px; background: #181818; vertical-align: middle; - border: 1px solid #383838; + border: 1px solid {$greyborder}; position: relative; } -.device-desktop .pholio-mock-carousel-thumb-item:hover, -.pholio-mock-carousel-thumb-current { +.device-desktop .pholio-mock-thumb-grid-item:hover, +.pholio-mock-thumb-grid-current { background: #383838; - border-color: #686868; + border-color: {$sky}; } -.device .pholio-mock-carousel-thumb-item { - width: 5px; - height: 5px; - padding: 0px; - border-radius: 5px; - margin: 5px 2px; - background: #383838; - border-color: #686868; +.pholio-mock-thumb-grid-item-obsolete { + opacity: 0.5; } -.device .pholio-mock-carousel-thumb-current { - background: #dfdfdf; - border-color: #ffffff; -} - -.device .pholio-mock-carousel-thumb-item img { - display: none; -} - -.pholio-mock-carousel-thumbnail { +.pholio-mock-thumb-grid-image { margin: auto; position: relative; } @@ -78,10 +69,6 @@ opacity: 0.50; } -.pholio-image-info { - border-bottom: 1px solid #101010; - margin-bottom: 5px; -} .pholio-image-info-item { padding: 0 8px; 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 c74451be7d..916a33071b 100644 --- a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js +++ b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js @@ -110,6 +110,15 @@ JX.behavior('pholio-mock-view', function(config) { return null; } + function get_image_navindex(id) { + for (var ii = 0; ii < config.navsequence.length; ii++) { + if (config.navsequence[ii] == id) { + return ii; + } + } + return null; + } + function get_image(id) { var idx = get_image_index(id); if (idx === null) { @@ -133,9 +142,12 @@ JX.behavior('pholio-mock-view', function(config) { if (!active_image) { return; } - var idx = get_image_index(active_image.id); - idx = (idx + delta + config.images.length) % config.images.length; - select_image(config.images[idx].id); + var idx = get_image_navindex(active_image.id); + if (idx === null) { + return; + } + idx = (idx + delta + config.navsequence.length) % config.navsequence.length; + select_image(config.navsequence[idx]); } function redraw_image() { @@ -197,7 +209,7 @@ JX.behavior('pholio-mock-view', function(config) { img.src = active_image.fullURI; var thumbs = JX.DOM.scry( - JX.$('pholio-mock-carousel'), + JX.$('pholio-mock-thumb-grid'), 'a', 'mock-thumbnail'); @@ -206,7 +218,7 @@ JX.behavior('pholio-mock-view', function(config) { JX.DOM.alterClass( thumbs[k], - 'pholio-mock-carousel-thumb-current', + 'pholio-mock-thumb-grid-current', (active_image.id == thumb_meta.imageID)); } @@ -217,7 +229,7 @@ JX.behavior('pholio-mock-view', function(config) { } JX.Stratcom.listen( - ['mousedown', 'click'], + 'click', 'mock-thumbnail', function(e) { if (!e.isNormalMouseEvent()) { @@ -238,10 +250,6 @@ JX.behavior('pholio-mock-view', function(config) { return; } - if (config.viewMode == 'history') { - return; - } - if (JX.Stratcom.pass()) { return; } @@ -602,15 +610,6 @@ JX.behavior('pholio-mock-view', function(config) { 'View Full Image'); info.push(full_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++) { info[ii] = JX.$N('div', {className: 'pholio-image-info-item'}, info[ii]); }