From 4afb6446d97ba4353ec495f65b75bb71ad7b7228 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 28 Aug 2018 14:37:19 -0700 Subject: [PATCH] Allow DocumentView to render with a curtain, and make Phriction use a curtain Summary: Depends on D19616. Ref T13077. Fixes T8172. In the last round of design updates, a lot of actions got stuffed into "Actions" menus. I never really got used to these and think they're a net usability loss, and broadly agree with the feedback in T8172. I'd generally like to move back toward a state where actions are available on the page, not hidden in a menu. For now, just put a curtain view on these pages. This could be refined later (e.g., stick this menu to the right hand side of the screen) depending on where other Phriction changes go. (Broadly, I'm also not satisfied with where we ended up on the fixed-width pages like Diffusion > Manage, Config, and Instances. In contrast, I //do// like where we ended up with Phortune in terms of overall design. I anticipate revisiting some of this stuff eventually.) Test Plan: - Looked at Phriction pages on desktop/tablet/mobile/printable -- actions are now available on the page. - Looked at other DocumentView pages (like Phame blogs) -- no changes for now. Reviewers: amckinley Maniphest Tasks: T13077, T8172 Differential Revision: https://secure.phabricator.com/D19617 --- resources/celerity/map.php | 14 ++-- .../PhrictionDocumentController.php | 46 +++++------ src/view/phui/PHUIDocumentView.php | 76 ++++++++++++++----- webroot/rsrc/css/phui/phui-document-pro.css | 29 +++++++ webroot/rsrc/css/phui/phui-document.css | 7 +- webroot/rsrc/css/phui/phui-header-view.css | 1 - 6 files changed, 116 insertions(+), 57 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 3842fbe839..609f156e88 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.js' => '15191c65', - 'core.pkg.css' => 'fc4839c8', + 'core.pkg.css' => 'badf3f16', 'core.pkg.js' => 'b5a949ca', 'differential.pkg.css' => '06dc617c', 'differential.pkg.js' => 'c1cfa143', @@ -146,15 +146,15 @@ return array( 'rsrc/css/phui/phui-comment-panel.css' => 'f50152ad', 'rsrc/css/phui/phui-crumbs-view.css' => '10728aaa', 'rsrc/css/phui/phui-curtain-view.css' => '2bdaf026', - 'rsrc/css/phui/phui-document-pro.css' => '8af7ea27', + 'rsrc/css/phui/phui-document-pro.css' => '0e41dd91', 'rsrc/css/phui/phui-document-summary.css' => '9ca48bdf', - 'rsrc/css/phui/phui-document.css' => '878c2f52', + 'rsrc/css/phui/phui-document.css' => '552493fa', 'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9', 'rsrc/css/phui/phui-fontkit.css' => '1320ed01', 'rsrc/css/phui/phui-form-view.css' => 'f808e5be', 'rsrc/css/phui/phui-form.css' => '7aaa04e3', 'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f', - 'rsrc/css/phui/phui-header-view.css' => 'edeb9252', + 'rsrc/css/phui/phui-header-view.css' => '1ba8b707', 'rsrc/css/phui/phui-hovercard.css' => 'f0592bcf', 'rsrc/css/phui/phui-icon-set-selector.css' => '87db8fee', 'rsrc/css/phui/phui-icon.css' => 'cf24ceec', @@ -813,15 +813,15 @@ return array( 'phui-crumbs-view-css' => '10728aaa', 'phui-curtain-view-css' => '2bdaf026', 'phui-document-summary-view-css' => '9ca48bdf', - 'phui-document-view-css' => '878c2f52', - 'phui-document-view-pro-css' => '8af7ea27', + 'phui-document-view-css' => '552493fa', + 'phui-document-view-pro-css' => '0e41dd91', 'phui-feed-story-css' => '44a9c8e9', 'phui-font-icon-base-css' => '870a7360', 'phui-fontkit-css' => '1320ed01', 'phui-form-css' => '7aaa04e3', 'phui-form-view-css' => 'f808e5be', 'phui-head-thing-view-css' => 'fd311e5f', - 'phui-header-view-css' => 'edeb9252', + 'phui-header-view-css' => '1ba8b707', 'phui-hovercard' => '1bd28176', 'phui-hovercard-view-css' => 'f0592bcf', 'phui-icon-set-selector-css' => '87db8fee', diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index 57dad25ab5..fce2c066f7 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -201,7 +201,10 @@ final class PhrictionDocumentController $children = $this->renderDocumentChildren($slug); - $actions = $this->buildActionView($viewer, $document); + $curtain = null; + if ($document->getID()) { + $curtain = $this->buildCurtain($document); + } $crumbs = $this->buildApplicationCrumbs(); $crumbs->setBorder(true); @@ -213,8 +216,7 @@ final class PhrictionDocumentController $header = id(new PHUIHeaderView()) ->setUser($viewer) ->setPolicyObject($document) - ->setHeader($page_title) - ->setActionList($actions); + ->setHeader($page_title); if ($content) { $header->setEpoch($content->getDateCreated()); @@ -237,6 +239,10 @@ final class PhrictionDocumentController $core_content, )); + if ($curtain) { + $page_content->setCurtain($curtain); + } + return $this->newPage() ->setTitle($page_title) ->setCrumbs($crumbs) @@ -258,8 +264,7 @@ final class PhrictionDocumentController $viewer = $this->getViewer(); $view = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setObject($document); + ->setUser($viewer); $view->addProperty( pht('Last Author'), @@ -272,9 +277,9 @@ final class PhrictionDocumentController return $view; } - private function buildActionView( - PhabricatorUser $viewer, - PhrictionDocument $document) { + private function buildCurtain(PhrictionDocument $document) { + $viewer = $this->getViewer(); + $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $document, @@ -282,19 +287,9 @@ final class PhrictionDocumentController $slug = PhabricatorSlug::normalize($this->slug); - $action_view = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObject($document); + $curtain = $this->newCurtainView($document); - if (!$document->getID()) { - return $action_view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Create This Document')) - ->setIcon('fa-plus-square') - ->setHref('/phriction/edit/?slug='.$slug)); - } - - $action_view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Document')) ->setDisabled(!$can_edit) @@ -302,7 +297,7 @@ final class PhrictionDocumentController ->setHref('/phriction/edit/'.$document->getID().'/')); if ($document->getStatus() == PhrictionDocumentStatus::STATUS_EXISTS) { - $action_view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Move Document')) ->setDisabled(!$can_edit) @@ -310,7 +305,7 @@ final class PhrictionDocumentController ->setHref('/phriction/move/'.$document->getID().'/') ->setWorkflow(true)); - $action_view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Delete Document')) ->setDisabled(!$can_edit) @@ -319,7 +314,7 @@ final class PhrictionDocumentController ->setWorkflow(true)); } - $action_view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('View History')) ->setIcon('fa-list') @@ -327,15 +322,14 @@ final class PhrictionDocumentController $print_uri = PhrictionDocument::getSlugURI($slug).'?__print__=1'; - $action_view->addAction( + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Printable Page')) ->setIcon('fa-print') ->setOpenInNewWindow(true) ->setHref($print_uri)); - return $action_view; - + return $curtain; } private function renderDocumentChildren($slug) { diff --git a/src/view/phui/PHUIDocumentView.php b/src/view/phui/PHUIDocumentView.php index 5b6bf9fcf9..32324ddb5a 100644 --- a/src/view/phui/PHUIDocumentView.php +++ b/src/view/phui/PHUIDocumentView.php @@ -8,6 +8,7 @@ final class PHUIDocumentView extends AphrontTagView { private $fluid; private $toc; private $foot; + private $curtain; public function setHeader(PHUIHeaderView $header) { $header->setTall(true); @@ -36,6 +37,15 @@ final class PHUIDocumentView extends AphrontTagView { return $this; } + public function setCurtain(PHUICurtainView $curtain) { + $this->curtain = $curtain; + return $this; + } + + public function getCurtain() { + return $this->curtain; + } + protected function getTagAttributes() { $classes = array(); @@ -61,6 +71,17 @@ final class PHUIDocumentView extends AphrontTagView { $classes[] = 'phui-document-view'; $classes[] = 'phui-document-view-pro'; + if ($this->curtain) { + $classes[] = 'has-curtain'; + } else { + $classes[] = 'has-no-curtain'; + } + + if ($this->curtain) { + $action_list = $this->curtain->getActionList(); + $this->header->setActionListID($action_list->getID()); + } + $book = null; if ($this->bookname) { $book = pht('%s (%s)', $this->bookname, $this->bookdescription); @@ -114,32 +135,51 @@ final class PHUIDocumentView extends AphrontTagView { $this->foot); } - $content_inner = phutil_tag( + $curtain = null; + if ($this->curtain) { + $curtain = phutil_tag( 'div', array( - 'class' => 'phui-document-inner', + 'class' => 'phui-document-curtain', ), + $this->curtain); + } + + $main_content = phutil_tag( + 'div', + array( + 'class' => 'phui-document-content-view', + ), + $main_content); + + $content_inner = phutil_tag( + 'div', + array( + 'class' => 'phui-document-inner', + ), + array( + $table_of_contents, + $this->header, array( - $table_of_contents, - $this->header, + $curtain, $main_content, - $foot_content, - )); + ), + $foot_content, + )); $content = phutil_tag( - 'div', - array( - 'class' => 'phui-document-content', - ), - $content_inner); - - return phutil_tag( - 'div', - array( - 'class' => implode(' ', $classes), - ), - $content); + 'div', + array( + 'class' => 'phui-document-content', + ), + $content_inner); + return phutil_tag( + 'div', + array( + 'class' => implode(' ', $classes), + ), + $content); } } diff --git a/webroot/rsrc/css/phui/phui-document-pro.css b/webroot/rsrc/css/phui/phui-document-pro.css index 35c843f81f..29dc3beffa 100644 --- a/webroot/rsrc/css/phui/phui-document-pro.css +++ b/webroot/rsrc/css/phui/phui-document-pro.css @@ -8,6 +8,35 @@ margin: 0 auto; } +.phui-document-view.phui-document-view-pro.has-curtain { + max-width: 1132px; +} + +.printable .phui-document-view.phui-document-view-pro.has-curtain { + max-width: none; +} + +.device-desktop .phui-document-inner { + overflow: hidden; +} + +.device-desktop .has-curtain .phui-document-content-view { + padding-right: 332px; +} + +.printable .phui-document-content-view { + padding-right: 0; +} + +.device-desktop .phui-document-curtain { + float: right; + width: 300px; +} + +.printable .phui-document-curtain { + display: none; +} + .phui-document-container { background-color: {$page.content}; position: relative; diff --git a/webroot/rsrc/css/phui/phui-document.css b/webroot/rsrc/css/phui/phui-document.css index 954a220229..f554930a5f 100644 --- a/webroot/rsrc/css/phui/phui-document.css +++ b/webroot/rsrc/css/phui/phui-document.css @@ -63,7 +63,8 @@ font-size: 14px; } -.phui-document-view .phui-header-action-links .phui-mobile-menu { +.phui-document-view.has-no-curtain + .phui-header-action-links .phui-mobile-menu { display: block; } @@ -71,10 +72,6 @@ padding: 8px; } -.device-desktop .phui-document-content .phabricator-action-list-view { - display: none; -} - .phui-document-content .phabricator-remarkup .remarkup-code-block { clear: both; margin: 16px 0; diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css index 5a8f06281d..6cafb3e330 100644 --- a/webroot/rsrc/css/phui/phui-header-view.css +++ b/webroot/rsrc/css/phui/phui-header-view.css @@ -259,7 +259,6 @@ body .phui-header-shell.phui-bleed-header color: {$lightgreytext}; } - .phui-header-action-links .phui-mobile-menu { display: none; }