From 56a970900856e5615cb33771dea5834b22195d4a Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 8 Mar 2015 15:27:16 -0700 Subject: [PATCH] Reduce code duplication for inline "Undo" Summary: Ref T2009. This is another almost-identical copy of the row scaffolding, which has the same 1up/2up bugs as the 8 other copies of this code. Turn the "undo" element into an InlineCommentView so we can scaffold it. Then, scaffold it with the same code as everything else. Test Plan: Hit "Undo", swapped from 1up to 2up, hit "undo" again, swapped back, tried left/right, everything rendered with proper scaffolding. Reviewers: btrahan, chad Reviewed By: chad Subscribers: epriestley Maniphest Tasks: T2009 Differential Revision: https://secure.phabricator.com/D12019 --- src/__phutil_library_map__.php | 2 + .../DifferentialChangesetViewController.php | 3 +- .../render/DifferentialChangesetRenderer.php | 15 +++++++ .../view/DifferentialChangesetDetailView.php | 5 +++ .../view/DifferentialChangesetListView.php | 42 +------------------ .../controller/DiffusionDiffController.php | 3 +- .../diff/PhabricatorChangesetResponse.php | 10 +++++ .../view/PHUIDiffInlineCommentUndoView.php | 40 ++++++++++++++++++ .../differential/changeset-view.css | 8 ++-- .../differential/ChangesetViewManager.js | 17 ++++++++ .../behavior-edit-inline-comments.js | 4 +- 11 files changed, 101 insertions(+), 48 deletions(-) create mode 100644 src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index abf8a15ce3..8a2a20bdc6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1139,6 +1139,7 @@ phutil_register_library_map(array( 'PHUIDiffInlineCommentEditView' => 'infrastructure/diff/view/PHUIDiffInlineCommentEditView.php', 'PHUIDiffInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php', 'PHUIDiffInlineCommentTableScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php', + 'PHUIDiffInlineCommentUndoView' => 'infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php', 'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php', 'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php', 'PHUIDiffTwoUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php', @@ -4371,6 +4372,7 @@ phutil_register_library_map(array( 'PHUIDiffInlineCommentEditView' => 'PHUIDiffInlineCommentView', 'PHUIDiffInlineCommentRowScaffold' => 'AphrontView', 'PHUIDiffInlineCommentTableScaffold' => 'AphrontView', + 'PHUIDiffInlineCommentUndoView' => 'PHUIDiffInlineCommentView', 'PHUIDiffInlineCommentView' => 'AphrontView', 'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold', 'PHUIDiffTwoUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index aab16791ca..b13cee5d28 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -217,7 +217,8 @@ final class DifferentialChangesetViewController extends DifferentialController { return id(new PhabricatorChangesetResponse()) ->setRenderedChangeset($parser->renderChangeset()) - ->setCoverage($coverage); + ->setCoverage($coverage) + ->setUndoTemplates($parser->getRenderer()->renderUndoTemplates()); } $diff = $changeset->getDiff(); diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index c19fa176aa..8edc301b79 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -600,4 +600,19 @@ abstract class DifferentialChangesetRenderer { return array($old, $new); } + public function renderUndoTemplates() { + $views = array( + 'l' => id(new PHUIDiffInlineCommentUndoView())->setIsOnRight(false), + 'r' => id(new PHUIDiffInlineCommentUndoView())->setIsOnRight(true), + ); + + foreach ($views as $key => $view) { + $scaffold = $this->getRowScaffoldForInline($view); + $views[$key] = id(new PHUIDiffInlineCommentTableScaffold()) + ->addRowScaffold($scaffold); + } + + return $views; + } + } diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php index a6dc8a5f3a..918a49e6a7 100644 --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -208,6 +208,9 @@ final class DifferentialChangesetDetailView extends AphrontView { $icon = id(new PHUIIconView()) ->setIconFont($display_icon); + $renderer = DifferentialChangesetHTMLRenderer::getHTMLRendererByKey( + $this->getRenderer()); + return javelin_tag( 'div', array( @@ -224,6 +227,7 @@ final class DifferentialChangesetDetailView extends AphrontView { 'ref' => $this->getRenderingRef(), 'autoload' => $this->getAutoload(), 'loaded' => $this->getLoaded(), + 'undoTemplates' => $renderer->renderUndoTemplates(), ), 'class' => $class, 'id' => $id, @@ -252,4 +256,5 @@ final class DifferentialChangesetDetailView extends AphrontView { )); } + } diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index d965156a85..3e7bfb677c 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -123,6 +123,7 @@ final class DifferentialChangesetListView extends AphrontView { 'collapsed' => pht('This file content has been collapsed.'), ), )); + Javelin::initBehavior( 'differential-dropdown-menus', array( @@ -230,11 +231,8 @@ final class DifferentialChangesetListView extends AphrontView { $this->initBehavior('differential-comment-jump', array()); if ($this->inlineURI) { - $undo_templates = $this->renderUndoTemplates(); - Javelin::initBehavior('differential-edit-inline-comments', array( 'uri' => $this->inlineURI, - 'undo_templates' => $undo_templates, 'stage' => 'differential-review-stage', )); } @@ -257,44 +255,6 @@ final class DifferentialChangesetListView extends AphrontView { return $object_box; } - /** - * Render the "Undo" markup for the inline comment undo feature. - */ - private function renderUndoTemplates() { - $link = javelin_tag( - 'a', - array( - 'href' => '#', - 'sigil' => 'differential-inline-comment-undo', - ), - pht('Undo')); - - $div = phutil_tag( - 'div', - array( - 'class' => 'differential-inline-undo', - ), - array('Changes discarded. ', $link)); - - return array( - 'l' => phutil_tag('table', array(), - phutil_tag('tr', array(), array( - phutil_tag('th', array()), - phutil_tag('td', array(), $div), - phutil_tag('th', array()), - phutil_tag('td', array('colspan' => 3)), - ))), - - 'r' => phutil_tag('table', array(), - phutil_tag('tr', array(), array( - phutil_tag('th', array()), - phutil_tag('td', array()), - phutil_tag('th', array()), - phutil_tag('td', array('colspan' => 3), $div), - ))), - ); - } - private function renderViewOptionsDropdown( DifferentialChangesetDetailView $detail, $ref, diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index 6e84757ec2..52ac964e0a 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -130,6 +130,7 @@ final class DiffusionDiffController extends DiffusionController { $parser->setMask($mask); return id(new PhabricatorChangesetResponse()) - ->setRenderedChangeset($parser->renderChangeset()); + ->setRenderedChangeset($parser->renderChangeset()) + ->setUndoTemplates($parser->getRenderer()->renderUndoTemplates()); } } diff --git a/src/infrastructure/diff/PhabricatorChangesetResponse.php b/src/infrastructure/diff/PhabricatorChangesetResponse.php index b4897eb18e..38c604fc06 100644 --- a/src/infrastructure/diff/PhabricatorChangesetResponse.php +++ b/src/infrastructure/diff/PhabricatorChangesetResponse.php @@ -4,6 +4,7 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse { private $renderedChangeset; private $coverage; + private $undoTemplates; public function setRenderedChangeset($rendered_changeset) { $this->renderedChangeset = $rendered_changeset; @@ -15,6 +16,11 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse { return $this; } + public function setUndoTemplates($undo_templates) { + $this->undoTemplates = $undo_templates; + return $this; + } + protected function buildProxy() { return new AphrontAjaxResponse(); } @@ -28,6 +34,10 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse { $content['coverage'] = $this->coverage; } + if ($this->undoTemplates) { + $content['undoTemplates'] = $this->undoTemplates; + } + return $this->getProxy()->setContent($content); } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php new file mode 100644 index 0000000000..1001661341 --- /dev/null +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php @@ -0,0 +1,40 @@ +isOnRight = $is_on_right; + return $this; + } + + public function getIsOnRight() { + return $this->isOnRight; + } + + public function render() { + $link = javelin_tag( + 'a', + array( + 'href' => '#', + 'sigil' => 'differential-inline-comment-undo', + ), + pht('Undo')); + + return phutil_tag( + 'div', + array( + 'class' => 'differential-inline-undo', + ), + array('Changes discarded. ', $link)); + } + +} diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index b302f8b23b..c77e807bc3 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -379,10 +379,12 @@ td.cov-I { .differential-inline-undo { padding: 4px; text-align: center; - background: #ffeeaa; + background: {$lightyellow}; + border: 1px solid {$yellow}; margin: 3px 0 1px; - font: 12px; - color: 444444; + color: {$darkgreytext}; + font: {$basefont}; + font-size: 12px; } .differential-inline-undo a { diff --git a/webroot/rsrc/js/application/differential/ChangesetViewManager.js b/webroot/rsrc/js/application/differential/ChangesetViewManager.js index f80883f910..df50e7e9c7 100644 --- a/webroot/rsrc/js/application/differential/ChangesetViewManager.js +++ b/webroot/rsrc/js/application/differential/ChangesetViewManager.js @@ -38,6 +38,7 @@ JX.install('ChangesetViewManager', { _renderer: null, _highlight: null, _encoding: null, + _undoTemplates: null, /** @@ -193,6 +194,8 @@ JX.install('ChangesetViewManager', { var root = target.parentNode; this._moveRows(table, root, target); root.removeChild(target); + + this._onchangesetresponse(response); }, _moveRows: function(src, dst, before) { @@ -256,6 +259,10 @@ JX.install('ChangesetViewManager', { return (JX.Device.getDevice() == 'desktop') ? '2up' : '1up'; }, + getUndoTemplates: function() { + return this._undoTemplates; + }, + setEncoding: function(encoding) { this._encoding = encoding; return this; @@ -333,6 +340,12 @@ JX.install('ChangesetViewManager', { this._stabilize = false; } + this._onchangesetresponse(response); + }, + + _onchangesetresponse: function(response) { + // Code shared by autoload and context responses. + if (response.coverage) { for (var k in response.coverage) { try { @@ -342,6 +355,10 @@ JX.install('ChangesetViewManager', { } } } + + if (response.undoTemplates) { + this._undoTemplates = response.undoTemplates; + } }, _getContentFrame: function() { diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js index 853bff03fc..7d10be8592 100644 --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js @@ -243,7 +243,7 @@ JX.behavior('differential-edit-inline-comments', function(config) { var view = JX.ChangesetViewManager.getForNode(root); editor = new JX.DifferentialInlineCommentEditor(config.uri) - .setTemplates(config.undo_templates) + .setTemplates(view.getUndoTemplates()) .setOperation('new') .setChangesetID(changeset) .setLineNumber(o) @@ -335,7 +335,7 @@ JX.behavior('differential-edit-inline-comments', function(config) { var view = JX.ChangesetViewManager.getForNode(changeset_root); editor = new JX.DifferentialInlineCommentEditor(config.uri) - .setTemplates(config.undo_templates) + .setTemplates(view.getUndoTemplates()) .setOperation(op) .setID(data.id) .setChangesetID(data.changesetID)