From f9cb366f0071620555bc6f446a1e9af245d09f7c Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 6 Mar 2015 05:57:24 -0800 Subject: [PATCH] Remove duplicate inline scaffold in 2up renderer Summary: Ref T2009. Remove the 4 (!!) copies of this code. Test Plan: - Added, edited, and removed inline comments in 2up view. - Stacked a bunch of comments on the same line and saw the JS place them correctly. - Created an image diff and added, edited and removed inlines on it. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2009 Differential Revision: https://secure.phabricator.com/D12000 --- .../DifferentialChangesetHTMLRenderer.php | 7 --- .../DifferentialChangesetTwoUpRenderer.php | 63 +++++++------------ .../view/PHUIDiffInlineCommentRowScaffold.php | 8 +++ .../PHUIDiffOneUpInlineCommentRowScaffold.php | 2 +- .../PHUIDiffTwoUpInlineCommentRowScaffold.php | 2 +- 5 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index 5ced1963ce..7e21fd2054 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -425,13 +425,6 @@ abstract class DifferentialChangesetHTMLRenderer )); } - protected function renderInlineComment( - PhabricatorInlineCommentInterface $comment, - $on_right = false) { - - return $this->buildInlineComment($comment, $on_right)->render(); - } - protected function buildInlineComment( PhabricatorInlineCommentInterface $comment, $on_right = false) { diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index ebf12ba439..01084087c1 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -264,39 +264,33 @@ final class DifferentialChangesetTwoUpRenderer if ($o_num && isset($old_comments[$o_num])) { foreach ($old_comments[$o_num] as $comment) { - $comment_html = $this->renderInlineComment($comment, - $on_right = false); - $new = ''; + $inline = $this->buildInlineComment( + $comment, + $on_right = false); + $scaffold = $this->getRowScaffoldForInline($inline); + if ($n_num && isset($new_comments[$n_num])) { foreach ($new_comments[$n_num] as $key => $new_comment) { if ($comment->isCompatible($new_comment)) { - $new = $this->renderInlineComment($new_comment, - $on_right = true); + $companion = $this->buildInlineComment( + $new_comment, + $on_right = true); + + $scaffold->addInlineView($companion); unset($new_comments[$n_num][$key]); } } } - $html[] = phutil_tag('tr', array('class' => 'inline'), array( - phutil_tag('th', array()), - phutil_tag('td', array(), $comment_html), - phutil_tag('th', array()), - phutil_tag('td', array('colspan' => 3), $new), - )); + + $html[] = $scaffold; } } if ($n_num && isset($new_comments[$n_num])) { foreach ($new_comments[$n_num] as $comment) { - $comment_html = $this->renderInlineComment($comment, - $on_right = true); - $html[] = phutil_tag('tr', array('class' => 'inline'), array( - phutil_tag('th', array()), - phutil_tag('td', array()), - phutil_tag('th', array()), - phutil_tag( - 'td', - array('colspan' => 3), - $comment_html), - )); + $inline = $this->buildInlineComment( + $comment, + $on_right = true); + $html[] = $this->getRowScaffoldForInline($inline); } } } @@ -340,27 +334,18 @@ final class DifferentialChangesetTwoUpRenderer $html_new = array(); foreach ($this->getOldComments() as $on_line => $comment_group) { foreach ($comment_group as $comment) { - $comment_html = $this->renderInlineComment($comment, $on_right = false); - $html_old[] = phutil_tag('tr', array('class' => 'inline'), array( - phutil_tag('th', array()), - phutil_tag('td', array(), $comment_html), - phutil_tag('th', array()), - phutil_tag('td', array('colspan' => 3)), - )); + $inline = $this->buildInlineComment( + $comment, + $on_right = false); + $html_old[] = $this->getRowScaffoldForInline($inline); } } foreach ($this->getNewComments() as $lin_line => $comment_group) { foreach ($comment_group as $comment) { - $comment_html = $this->renderInlineComment($comment, $on_right = true); - $html_new[] = phutil_tag('tr', array('class' => 'inline'), array( - phutil_tag('th', array()), - phutil_tag('td', array()), - phutil_tag('th', array()), - phutil_tag( - 'td', - array('colspan' => 3), - $comment_html), - )); + $inline = $this->buildInlineComment( + $comment, + $on_right = true); + $html_new[] = $this->getRowScaffoldForInline($inline); } } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php index 81753995ef..194ff6f5cc 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php @@ -20,4 +20,12 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView { return $this; } + protected function getRowAttributes() { + // TODO: This is semantic information used by the JS when placing comments + // and using keyboard navigation; we should move it out of class names. + return array( + 'class' => 'inline', + ); + } + } diff --git a/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php index b71612a410..3ffb732a73 100644 --- a/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php +++ b/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php @@ -27,7 +27,7 @@ final class PHUIDiffOneUpInlineCommentRowScaffold phutil_tag('td', $attrs, $inline), ); - return phutil_tag('tr', array(), $cells); + return phutil_tag('tr', $this->getRowAttributes(), $cells); } } diff --git a/src/infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php index 29ae88daa4..a1284ae39a 100644 --- a/src/infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php +++ b/src/infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php @@ -66,7 +66,7 @@ final class PHUIDiffTwoUpInlineCommentRowScaffold phutil_tag('td', $right_attrs, $right_side), ); - return phutil_tag('tr', array(), $cells); + return phutil_tag('tr', $this->getRowAttributes(), $cells); } }