From e5c6a5749ad2dd881c30fcd4964494669bfe12f7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 17 Sep 2018 15:10:29 -0700 Subject: [PATCH 1/3] Fix fatal in rendering Phriction "Moved Away" stories Summary: Ref T13202. See PHI881. These stories have bad rendering methods, but they didn't previously render into the timeilne (since Phriction documents didn't have a timeline). Update the rendering to work. The rendered outcome isn't great (it isn't very clear or explicit about exactly what moved where), but I'll fix that in a followup. This is a net improvement since it doesn't fatal the page, at least. Test Plan: - Moved page "X" to "Y". - Viewed the old page "X". - Before patch: bad timeline story would fatal rendering. - After patch: story renders, at least, just not great. Reviewers: amckinley Maniphest Tasks: T13202 Differential Revision: https://secure.phabricator.com/D19682 --- .../xaction/PhrictionDocumentMoveAwayTransaction.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php index 09ec56dd25..af77a5adea 100644 --- a/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php @@ -38,19 +38,19 @@ final class PhrictionDocumentMoveAwayTransaction $new = $this->getNewValue(); return pht( - '%s moved this document to %s', + '%s moved this document to %s.', $this->renderAuthor(), - $this->renderHandleLink($new['phid'])); + $this->renderObject($new['phid'])); } public function getTitleForFeed() { $new = $this->getNewValue(); return pht( - '%s moved %s to %s', + '%s moved %s to %s.', $this->renderAuthor(), $this->renderObject(), - $this->renderHandleLink($new['phid'])); + $this->renderObject($new['phid'])); } public function getIcon() { From 5ba66e56fd54c5050c61fba4d792995071e0b0a4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 17 Sep 2018 15:16:00 -0700 Subject: [PATCH 2/3] Fix Phriction rendering for initial install and 404 pages Summary: Depends on D19682. Ref T13202. We currently fatal when trying to render a timeline if: - an install is fresh, so there are no pages yet, and you look at "/w/"; or - you're looking at a Phriction page which doesn't exist (yet) like "/w/aadsflknadsflnf/". Rendering a timeline and comment area doesn't make sense in these cases, so don't render them. Test Plan: Hit both cases described above, got "new/empty page" prompts instead of fatals. Reviewers: amckinley Maniphest Tasks: T13202 Differential Revision: https://secure.phabricator.com/D19683 --- .../PhrictionDocumentController.php | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index 77f6a3529c..8da9807064 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -377,16 +377,21 @@ final class PhrictionDocumentController $page_content->setCurtain($curtain); } - $timeline = $this->buildTransactionTimeline( - $document, - new PhrictionTransactionQuery()); + if ($document->getPHID()) { + $timeline = $this->buildTransactionTimeline( + $document, + new PhrictionTransactionQuery()); - $edit_engine = id(new PhrictionDocumentEditEngine()) - ->setViewer($viewer) - ->setTargetObject($document); + $edit_engine = id(new PhrictionDocumentEditEngine()) + ->setViewer($viewer) + ->setTargetObject($document); - $comment_view = $edit_engine - ->buildEditEngineCommentView($document); + $comment_view = $edit_engine + ->buildEditEngineCommentView($document); + } else { + $timeline = null; + $comment_view = null; + } return $this->newPage() ->setTitle($page_title) From 3244324cb1abeb5c7e7a1f78a4edb05914a3eb26 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 18 Sep 2018 07:46:40 -0700 Subject: [PATCH 3/3] Fix comment box borders in timelines after Phriction commenting Summary: Ref T13202. In D19660, I added comments to Phriction and tweaked some CSS. One of these tweaks was getting rid of an extra border which was rendering under the comment area. However, I took off too much and ended up removing borders from other applications. I think we don't actually need this `setNoBorder()` stuff after all -- a later change was sufficient to stop the actual border I was trying to get rid of from rendering. So this mostly just reverts part of D19660. This rendering still isn't perfect, but I'm fine leaving that for another day for now. Test Plan: - Viewed comment areas in Phriction. Saw correct number of borders (1). - Viewed comment areas in Maniphest. Saw correct number of borders (1). - Grepped for extraneous removed classs, no hits. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13202 Differential Revision: https://secure.phabricator.com/D19684 --- .../PhabricatorApplicationTransactionCommentView.php | 2 +- src/view/phui/PHUIObjectBoxView.php | 10 +--------- src/view/phui/PHUITimelineEventView.php | 4 ++-- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index 8be45f8cde..c0f514c904 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -23,6 +23,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { private $fullWidth; private $infoView; private $editEngineLock; + private $noBorder; private $currentVersion; private $versionedDraft; @@ -243,7 +244,6 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { $comment_box = id(new PHUIObjectBoxView()) ->setFlush(true) - ->setNoBorder(true) ->addClass('phui-comment-form-view') ->addSigil('phui-comment-form') ->appendChild( diff --git a/src/view/phui/PHUIObjectBoxView.php b/src/view/phui/PHUIObjectBoxView.php index f19c485a36..f5fdbfffc4 100644 --- a/src/view/phui/PHUIObjectBoxView.php +++ b/src/view/phui/PHUIObjectBoxView.php @@ -25,7 +25,6 @@ final class PHUIObjectBoxView extends AphrontTagView { private $showHideHref; private $showHideContent; private $showHideOpen; - private $noBorder; private $propertyLists = array(); @@ -148,11 +147,6 @@ final class PHUIObjectBoxView extends AphrontTagView { return $this; } - public function setNoBorder($no_border) { - $this->noBorder = $no_border; - return $this; - } - public function setValidationException( PhabricatorApplicationTransactionValidationException $ex = null) { $this->validationException = $ex; @@ -162,9 +156,7 @@ final class PHUIObjectBoxView extends AphrontTagView { protected function getTagAttributes() { $classes = array(); $classes[] = 'phui-box'; - if (!$this->noBorder) { - $classes[] = 'phui-box-border'; - } + $classes[] = 'phui-box-border'; $classes[] = 'phui-object-box'; $classes[] = 'mlt mll mlr'; diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php index 79aeb454b6..1cf6400bb5 100644 --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -402,7 +402,7 @@ final class PHUITimelineEventView extends AphrontView { $wedge = phutil_tag( 'div', array( - 'class' => 'phui-timeline-wedge phui-timeline-border', + 'class' => 'phui-timeline-wedge', 'style' => (nonempty($image_uri)) ? '' : 'display: none;', ), ''); @@ -461,7 +461,7 @@ final class PHUITimelineEventView extends AphrontView { $content = phutil_tag( 'div', array( - 'class' => 'phui-timeline-group phui-timeline-border', + 'class' => 'phui-timeline-group', ), $content);