From 0a9f378594f07df699f6fb6f6ea8c9ef6b9d8300 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 30 Jan 2013 10:58:21 -0800 Subject: [PATCH] render_tag -> tag: Differential Changeset stuff Summary: I cheated in a couple of places here, but this is in the process of getting refactored anyway, and there's a pretty clear boundary. Test Plan: Viewed changesets in Differential, viewed standalone. Viewed context elements. Reviewers: vrana Reviewed By: vrana CC: aran Maniphest Tasks: T2432 Differential Revision: https://secure.phabricator.com/D4723 --- .../DifferentialChangesetViewController.php | 4 +++ .../DifferentialChangesetHTMLRenderer.php | 24 ++++++++----- .../DifferentialChangesetTwoUpRenderer.php | 32 +++++++++++++---- .../view/DifferentialChangesetDetailView.php | 34 ++++++++++--------- .../view/DifferentialChangesetListView.php | 22 ++++++------ .../application/core/behavior-dark-console.js | 1 - 6 files changed, 76 insertions(+), 41 deletions(-) diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index dc09fd6db5..3d7454c229 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -234,6 +234,10 @@ final class DifferentialChangesetViewController extends DifferentialController { Javelin::initBehavior('differential-comment-jump', array()); + // TODO: [HTML] Clean up DifferentialChangesetParser output, but it's + // undergoing like six kinds of refactoring anyway. + $output = phutil_safe_html($output); + $detail = new DifferentialChangesetDetailView(); $detail->setChangeset($changeset); $detail->appendChild($output); diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index 1a60831ac9..4efe69250a 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -319,9 +319,11 @@ abstract class DifferentialChangesetHTMLRenderer $meta['whitespace'] = DifferentialChangesetParser::WHITESPACE_SHOW_ALL; } - $more = null; + $content = array(); + $content[] = $message; if ($force !== 'none') { - $more = ' '.javelin_tag( + $content[] = ' '; + $content[] = javelin_tag( 'a', array( 'mustcapture' => true, @@ -334,15 +336,18 @@ abstract class DifferentialChangesetHTMLRenderer } return $this->wrapChangeInTable( - javelin_render_tag( + javelin_tag( 'tr', array( 'sigil' => 'context-target', ), - ''. - phutil_escape_html($message). - $more. - '')); + phutil_tag( + 'td', + array( + 'class' => 'differential-shield', + 'colspan' => 6, + ), + $content))); } protected function wrapChangeInTable($content) { @@ -350,7 +355,10 @@ abstract class DifferentialChangesetHTMLRenderer return null; } - return javelin_render_tag( + // TODO: [HTML] After TwoUpRenderer gets refactored, fix this. + $content = phutil_safe_html($content); + + return javelin_tag( 'table', array( 'class' => 'differential-diff remarkup-code PhabricatorMonospaced', diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index cb22b44341..7a9bb3a1af 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -148,16 +148,36 @@ final class DifferentialChangesetTwoUpRenderer } } - $container = javelin_render_tag( + $container = javelin_tag( 'tr', array( 'sigil' => 'context-target', ), - ''. - implode(' • ', $contents). - ''. - ''.$context_line.''. - ''.$context.''); + array( + phutil_tag( + 'td', + array( + 'colspan' => 2, + 'class' => 'show-more', + ), + array_interleave( + " \xE2\x80\xA2 ", // Bullet + $contents)), + phutil_tag( + 'th', + array( + 'class' => 'show-context-line', + ), + $context_line ? (int)$context_line : null), + phutil_tag( + 'td', + array( + 'colspan' => 3, + 'class' => 'show-context', + ), + // TODO: [HTML] Escaping model here isn't ideal. + phutil_safe_html($context)), + )); $html[] = $container; diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php index 364fe35701..f77b5377d3 100644 --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -59,10 +59,12 @@ final class DifferentialChangesetDetailView extends AphrontView { $buttons = null; if ($this->buttons) { - $buttons = - '
'. - implode('', $this->buttons). - '
'; + $buttons = phutil_tag( + 'div', + array( + 'class' => 'differential-changeset-buttons', + ), + $this->buttons); } $id = $this->getID(); @@ -77,7 +79,7 @@ final class DifferentialChangesetDetailView extends AphrontView { $display_filename = $changeset->getDisplayFilename(); - $output = javelin_render_tag( + return javelin_tag( 'div', array( 'sigil' => 'differential-changeset', @@ -90,17 +92,17 @@ final class DifferentialChangesetDetailView extends AphrontView { 'class' => $class, 'id' => $id, ), - id(new PhabricatorAnchorView()) - ->setAnchorName($changeset->getAnchorName()) - ->setNavigationMarker(true) - ->render(). - $buttons. - '

'.phutil_escape_html($display_filename).'

'. - '
'. - $this->renderChildren()); - - - return $output; + $this->renderHTMLView( + array( + id(new PhabricatorAnchorView()) + ->setAnchorName($changeset->getAnchorName()) + ->setNavigationMarker(true) + ->render(), + $buttons, + phutil_tag('h1', array(), $display_filename), + phutil_tag('div', array('style' => 'clear: both'), ''), + $this->renderHTMLChildren(), + ))); } } diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index abbca39906..00094164f4 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -192,17 +192,19 @@ final class DifferentialChangesetListView extends AphrontView { )); } - return - id(new PhabricatorHeaderView()) - ->setHeader($this->getTitle()) - ->render(). - phutil_render_tag( - 'div', + return $this->renderHTMLView( array( - 'class' => 'differential-review-stage', - 'id' => 'differential-review-stage', - ), - implode("\n", $output)); + id(new PhabricatorHeaderView()) + ->setHeader($this->getTitle()) + ->render(), + phutil_tag( + 'div', + array( + 'class' => 'differential-review-stage', + 'id' => 'differential-review-stage', + ), + $output), + )); } /** diff --git a/webroot/rsrc/js/application/core/behavior-dark-console.js b/webroot/rsrc/js/application/core/behavior-dark-console.js index 0c72d0469c..99370a9342 100644 --- a/webroot/rsrc/js/application/core/behavior-dark-console.js +++ b/webroot/rsrc/js/application/core/behavior-dark-console.js @@ -81,7 +81,6 @@ JX.behavior('dark-console', function(config, statics) { JX.DOM.alterClass(req.all[req.current], 'dark-selected', true); if (statics.visible) { - JX.log('visible!'); draw_request(key); } }