From ad3c94dd454ff778ddf3e982d20fe075dbc2fba4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 5 Mar 2015 14:03:00 -0800 Subject: [PATCH] Make "Show Context" persist rendering, whitespace, encoding, etc Summary: Ref T2009. Currently, we do not persist view parameters when making context rendering requests. The big one is the renderer (1up vs 2up). This makes context on unified diffs come in with too many columns. However, it impacts other parameters too. For example, at HEAD, if you change highlighting to "rainbow" and then load more context, the context uses the original highlighter instead of the rainbow highlighter. This moves context loads into ChangesetViewManager, which maintains view parameters and can provide them correctly. - This removes "ref"; it is no longer required, as the ChangesetViewManager tracks it. - This removes URI management from `behavior-show-more`; it is no longer required, since the ChangesetViewManager knows how to render. - This removes "whitespace" since this is handled properly by the view manager. Test Plan: - Used "Show Top" / "Show All" / "Show Bottom" in 1-up and 2-up views. - Changed file highlighting to rainbow, loaded stuff, saw rainbow stick. - Used "Show Entire File" in 1-up and 2-up views. - Saw loading chrome. - No loading chrome normally. - Made inlines, verified `copyRows()` code runs. - Poked around Diffusion -- it is missing some parameter handling, but works OK. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2009 Differential Revision: https://secure.phabricator.com/D11977 --- resources/celerity/map.php | 93 +++++++-------- .../DifferentialChangesetViewController.php | 16 +-- .../parser/DifferentialChangesetParser.php | 38 ++++++ .../DifferentialChangesetHTMLRenderer.php | 1 - .../DifferentialChangesetOneUpRenderer.php | 4 + ...DifferentialChangesetOneUpTestRenderer.php | 4 + .../render/DifferentialChangesetRenderer.php | 2 + .../DifferentialChangesetTwoUpRenderer.php | 4 + ...DifferentialChangesetTwoUpTestRenderer.php | 4 + .../view/DifferentialChangesetDetailView.php | 25 ++-- .../view/DifferentialChangesetListView.php | 10 +- .../controller/PhrictionDiffController.php | 8 +- .../PhabricatorUSEnglishTranslation.php | 4 +- .../differential/ChangesetViewManager.js | 110 +++++++++++++++--- .../DifferentialInlineCommentEditor.js | 19 +++ .../differential/behavior-dropdown-menus.js | 11 +- .../differential/behavior-show-more.js | 58 ++------- 17 files changed, 252 insertions(+), 159 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d9c1bcda8d..eef8a2b476 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,7 +11,7 @@ return array( 'core.pkg.js' => 'a77025a1', 'darkconsole.pkg.js' => '8ab24e01', 'differential.pkg.css' => 'd8866ed8', - 'differential.pkg.js' => '7b5a4aa4', + 'differential.pkg.js' => '9e55f9f5', 'diffusion.pkg.css' => '591664fa', 'diffusion.pkg.js' => 'bfc0737b', 'maniphest.pkg.css' => '68d4dd3d', @@ -360,18 +360,18 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '82439934', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/differential/ChangesetViewManager.js' => 'c024db3d', - 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746', + 'rsrc/js/application/differential/ChangesetViewManager.js' => 'fce415a0', + 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '6a049cf7', 'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', 'rsrc/js/application/differential/behavior-comment-preview.js' => '6932def3', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', - 'rsrc/js/application/differential/behavior-dropdown-menus.js' => 'e33d4bc5', + 'rsrc/js/application/differential/behavior-dropdown-menus.js' => '2035b9cb', 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '65936067', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '2c426492', 'rsrc/js/application/differential/behavior-populate.js' => 'bdb3e4d0', 'rsrc/js/application/differential/behavior-show-field-details.js' => 'bba9eedf', - 'rsrc/js/application/differential/behavior-show-more.js' => '954d2de0', + 'rsrc/js/application/differential/behavior-show-more.js' => 'c662904a', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', 'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d', 'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => 'b42eddc7', @@ -510,7 +510,7 @@ return array( 'aphront-two-column-view-css' => '16ab3ad2', 'aphront-typeahead-control-css' => '0e403212', 'auth-css' => '1e655982', - 'changeset-view-manager' => 'c024db3d', + 'changeset-view-manager' => 'fce415a0', 'config-options-css' => '7fedf08b', 'config-welcome-css' => '6abd79be', 'conpherence-durable-column-view' => '3b836442', @@ -521,7 +521,7 @@ return array( 'conpherence-widget-pane-css' => '3d575438', 'differential-changeset-view-css' => 'b600950c', 'differential-core-view-css' => '7ac3cabc', - 'differential-inline-comment-editor' => 'f2441746', + 'differential-inline-comment-editor' => '6a049cf7', 'differential-results-table-css' => '181aa9d9', 'differential-revision-add-comment-css' => 'c478bcaa', 'differential-revision-comment-css' => '48186045', @@ -569,13 +569,13 @@ return array( 'javelin-behavior-differential-add-reviewers-and-ccs' => 'e10f8e18', 'javelin-behavior-differential-comment-jump' => '4fdb476d', 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', - 'javelin-behavior-differential-dropdown-menus' => 'e33d4bc5', + 'javelin-behavior-differential-dropdown-menus' => '2035b9cb', 'javelin-behavior-differential-edit-inline-comments' => '65936067', 'javelin-behavior-differential-feedback-preview' => '6932def3', 'javelin-behavior-differential-keyboard-navigation' => '2c426492', 'javelin-behavior-differential-populate' => 'bdb3e4d0', 'javelin-behavior-differential-show-field-details' => 'bba9eedf', - 'javelin-behavior-differential-show-more' => '954d2de0', + 'javelin-behavior-differential-show-more' => 'c662904a', 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', 'javelin-behavior-differential-user-select' => 'a8d8459d', 'javelin-behavior-diffusion-commit-branches' => 'bdaf4d04', @@ -944,6 +944,18 @@ return array( 'javelin-dom', 'javelin-reactor-dom', ), + '2035b9cb' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phuix-dropdown-menu', + 'phuix-action-list-view', + 'phuix-action-view', + 'phabricator-phtize', + 'changeset-view-manager', + ), '2290aeef' => array( 'javelin-install', 'javelin-dom', @@ -1236,6 +1248,14 @@ return array( '69adf288' => array( 'javelin-install', ), + '6a049cf7' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-request', + 'javelin-workflow', + ), '6c2b09a2' => array( 'javelin-install', 'javelin-util', @@ -1534,13 +1554,6 @@ return array( 'javelin-resource', 'javelin-routable', ), - '954d2de0' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-workflow', - 'javelin-util', - 'javelin-stratcom', - ), '988040b4' => array( 'javelin-install', 'javelin-dom', @@ -1704,16 +1717,6 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), - 'c024db3d' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - ), 'c1700f6f' => array( 'javelin-install', 'javelin-util', @@ -1728,6 +1731,14 @@ return array( 'javelin-stratcom', 'javelin-vector', ), + 'c662904a' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-workflow', + 'javelin-util', + 'javelin-stratcom', + 'changeset-view-manager', + ), 'c90a04fc' => array( 'javelin-dom', 'javelin-dynval', @@ -1827,18 +1838,6 @@ return array( 'javelin-workflow', 'javelin-vector', ), - 'e33d4bc5' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phuix-dropdown-menu', - 'phuix-action-list-view', - 'phuix-action-view', - 'phabricator-phtize', - 'changeset-view-manager', - ), 'e379b58e' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1893,14 +1892,6 @@ return array( 'javelin-install', 'javelin-util', ), - 'f2441746' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-request', - 'javelin-workflow', - ), 'f24f3253' => array( 'javelin-behavior', 'javelin-dom', @@ -1991,6 +1982,16 @@ return array( 'javelin-dom', 'phortune-credit-card-form', ), + 'fce415a0' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + ), 'fe287620' => array( 'javelin-install', 'javelin-dom', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index 33597e0020..59691aa408 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -154,13 +154,8 @@ final class DifferentialChangesetViewController extends DifferentialController { $parser->setRenderCacheKey($render_cache_key); $parser->setRightSideCommentMapping($right_source, $right_new); $parser->setLeftSideCommentMapping($left_source, $left_new); - $parser->setWhitespaceMode($request->getStr('whitespace')); - $parser->setCharacterEncoding($request->getStr('encoding')); - $parser->setHighlightAs($request->getStr('highlight')); - if ($request->getStr('renderer') == '1up') { - $parser->setRenderer(new DifferentialChangesetOneUpRenderer()); - } + $parser->readParametersFromRequest($request); if ($left && $right) { $parser->setOriginals($left, $right); @@ -235,6 +230,9 @@ final class DifferentialChangesetViewController extends DifferentialController { $detail = id(new DifferentialChangesetDetailView()) ->setUser($this->getViewer()) ->setChangeset($changeset) + ->setRenderingRef($rendering_reference) + ->setRenderURI('/differential/changeset/') + ->setRenderer($parser->getRenderer()->getRendererKey()) ->appendChild($output) ->setVsChangesetID($left_source); @@ -242,11 +240,7 @@ final class DifferentialChangesetViewController extends DifferentialController { 'changesetViewIDs' => array($detail->getID()), )); - Javelin::initBehavior('differential-show-more', array( - 'uri' => '/differential/changeset/', - 'whitespace' => $request->getStr('whitespace'), - )); - + Javelin::initBehavior('differential-show-more'); Javelin::initBehavior('differential-comment-jump', array()); $panel = new DifferentialPrimaryPaneView(); diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 7d979beddc..cd689153f0 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -92,6 +92,44 @@ final class DifferentialChangesetParser { return $this->disableCache; } + public static function getDefaultRendererForViewer(PhabricatorUser $viewer) { + $prefs = $viewer->loadPreferences(); + $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; + if ($prefs->getPreference($pref_unified) == 'unified') { + return '1up'; + } + return null; + } + + public function readParametersFromRequest(AphrontRequest $request) { + $this->setWhitespaceMode($request->getStr('whitespace')); + $this->setCharacterEncoding($request->getStr('encoding')); + $this->setHighlightAs($request->getStr('highlight')); + + $renderer = null; + + // If the viewer prefers unified diffs, always set the renderer to unified. + // Otherwise, we leave it unspecified and the client will choose a + // renderer based on the screen size. + + if ($request->getStr('renderer')) { + $renderer = $request->getStr('renderer'); + } else { + $renderer = self::getDefaultRendererForViewer($request->getViewer()); + } + + switch ($renderer) { + case '1up': + $this->setRenderer(new DifferentialChangesetOneUpRenderer()); + break; + default: + $this->setRenderer(new DifferentialChangesetTwoUpRenderer()); + break; + } + + return $this; + } + const CACHE_VERSION = 11; const CACHE_MAX_SIZE = 8e6; diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index b4ef3b640a..b03497560b 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -519,7 +519,6 @@ abstract class DifferentialChangesetHTMLRenderer 'sigil' => 'show-more', 'meta' => array( 'type' => ($is_all ? 'all' : null), - 'ref' => $reference, 'range' => $range, ), ), diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index e3d64f872c..cdefbc303b 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -11,6 +11,10 @@ final class DifferentialChangesetOneUpRenderer return 'diff-1up'; } + public function getRendererKey() { + return '1up'; + } + protected function renderColgroup() { return phutil_tag('colgroup', array(), array( phutil_tag('col', array('class' => 'num')), diff --git a/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php index b6ea30e51f..a9afb43af8 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php @@ -7,4 +7,8 @@ final class DifferentialChangesetOneUpTestRenderer return true; } + public function getRendererKey() { + return '1up-test'; + } + } diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index be440b47e7..09eb27ab47 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -34,6 +34,8 @@ abstract class DifferentialChangesetRenderer { private $oldFile = false; private $newFile = false; + abstract public function getRendererKey(); + public function setShowEditAndReplyLinks($bool) { $this->showEditAndReplyLinks = $bool; return $this; diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index 1129330045..682bcd51e4 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -11,6 +11,10 @@ final class DifferentialChangesetTwoUpRenderer return 'diff-2up'; } + public function getRendererKey() { + return '2up'; + } + protected function renderColgroup() { return phutil_tag('colgroup', array(), array( phutil_tag('col', array('class' => 'num')), diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php index e144577db9..226758b9fc 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php @@ -7,4 +7,8 @@ final class DifferentialChangesetTwoUpTestRenderer return false; } + public function getRendererKey() { + return '2up-test'; + } + } diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php index 6857fe6336..7723a7be7d 100644 --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -12,6 +12,7 @@ final class DifferentialChangesetDetailView extends AphrontView { private $whitespace; private $renderingRef; private $autoload; + private $renderer; public function setAutoload($autoload) { $this->autoload = $autoload; @@ -69,6 +70,15 @@ final class DifferentialChangesetDetailView extends AphrontView { return $this; } + public function setRenderer($renderer) { + $this->renderer = $renderer; + return $this; + } + + public function getRenderer() { + return $this->renderer; + } + public function getID() { if (!$this->id) { $this->id = celerity_generate_unique_node_id(); @@ -188,19 +198,6 @@ final class DifferentialChangesetDetailView extends AphrontView { $icon = id(new PHUIIconView()) ->setIconFont($display_icon); - $renderer = null; - - // If the viewer prefers unified diffs, always set the renderer to unified. - // Otherwise, we leave it unspecified and the client will choose a - // renderer based on the screen size. - - $viewer = $this->getUser(); - $prefs = $viewer->loadPreferences(); - $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; - if ($prefs->getPreference($pref_unified) == 'unified') { - $renderer = '1up'; - } - return javelin_tag( 'div', array( @@ -213,7 +210,7 @@ final class DifferentialChangesetDetailView extends AphrontView { 'renderURI' => $this->getRenderURI(), 'whitespace' => $this->getWhitespace(), 'highlight' => null, - 'renderer' => $renderer, + 'renderer' => $this->getRenderer(), 'ref' => $this->getRenderingRef(), 'autoload' => $this->getAutoload(), ), diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index 4ec0e19a3f..e63c3d613f 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -136,6 +136,9 @@ final class DifferentialChangesetListView extends AphrontView { ), )); + $renderer = DifferentialChangesetParser::getDefaultRendererForViewer( + $this->getUser()); + $output = array(); $ids = array(); foreach ($changesets as $key => $changeset) { @@ -169,6 +172,7 @@ final class DifferentialChangesetListView extends AphrontView { $detail->setRenderURI($this->renderURI); $detail->setWhitespace($this->whitespace); + $detail->setRenderer($renderer); if (isset($this->visibleChangesets[$key])) { $load = 'Loading...'; @@ -205,11 +209,7 @@ final class DifferentialChangesetListView extends AphrontView { 'changesetViewIDs' => $ids, )); - $this->initBehavior('differential-show-more', array( - 'uri' => $this->renderURI, - 'whitespace' => $this->whitespace, - )); - + $this->initBehavior('differential-show-more'); $this->initBehavior('differential-comment-jump', array()); if ($this->inlineURI) { diff --git a/src/applications/phriction/controller/PhrictionDiffController.php b/src/applications/phriction/controller/PhrictionDiffController.php index dea82dcc04..7c4dbc0bc8 100644 --- a/src/applications/phriction/controller/PhrictionDiffController.php +++ b/src/applications/phriction/controller/PhrictionDiffController.php @@ -97,6 +97,8 @@ final class PhrictionDiffController extends PhrictionController { $output = id(new DifferentialChangesetDetailView()) ->setUser($this->getViewer()) ->setChangeset($changeset) + ->setRenderingRef("{$l},{$r}") + ->setRenderURI('/phriction/diff/'.$document->getID().'/') ->appendChild($output); require_celerity_resource('differential-changeset-view-css'); @@ -106,11 +108,7 @@ final class PhrictionDiffController extends PhrictionController { Javelin::initBehavior('differential-populate', array( 'changesetViewIDs' => array($output->getID()), )); - - Javelin::initBehavior('differential-show-more', array( - 'uri' => '/phriction/diff/'.$document->getID().'/', - 'whitespace' => $whitespace_mode, - )); + Javelin::initBehavior('differential-show-more'); $slug = $document->getSlug(); diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index a2ffe78639..31881310d0 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -948,8 +948,8 @@ final class PhabricatorUSEnglishTranslation ), "\xE2\x96\xB2 Show %d Line(s)" => array( - "\xE2\x96\xB2 Show %d Line(s)", - "\xE2\x96\xB2 Show %d Line(s)", + "\xE2\x96\xB2 Show Line", + "\xE2\x96\xB2 Show %d Lines", ), 'Show All %d Line(s)' => array( diff --git a/webroot/rsrc/js/application/differential/ChangesetViewManager.js b/webroot/rsrc/js/application/differential/ChangesetViewManager.js index a18eeda8f1..36f097cf49 100644 --- a/webroot/rsrc/js/application/differential/ChangesetViewManager.js +++ b/webroot/rsrc/js/application/differential/ChangesetViewManager.js @@ -118,25 +118,12 @@ JX.install('ChangesetViewManager', { this._loaded = true; this._sequence++; - var params = { - ref: this._ref, - whitespace: this._whitespace || '', - renderer: this.getRenderer() || '', - highlight: this._highlight || '', - encoding: this._encoding || '' - }; + var params = this._getViewParameters(); var workflow = new JX.Workflow(this._renderURI, params) .setHandler(JX.bind(this, this._onresponse, this._sequence)); - var routable = workflow.getRoutable(); - - routable - .setPriority(500) - .setType('content') - .setKey(this._getRoutableKey()); - - JX.Router.getInstance().queue(routable); + this._startContentWorkflow(workflow); JX.DOM.setContent( this._getContentFrame(), @@ -148,6 +135,95 @@ JX.install('ChangesetViewManager', { return this; }, + /** + * Load missing context in a changeset. + * + * We do this when the user clicks "Show X Lines". We also expand all of + * the missing context when they "Show Entire File". + * + * @param string Line range specification, like "0-40/0-20". + * @param node Row where the context should be rendered after loading. + * @param bool True if this is a bulk load of multiple context blocks. + * @return this + */ + loadContext: function(range, target, bulk) { + var params = this._getViewParameters(); + params.range = range; + + var container = JX.DOM.scry(target, 'td')[0]; + // TODO: pht() + JX.DOM.setContent(container, 'Loading...'); + JX.DOM.alterClass(target, 'differential-show-more-loading', true); + + var workflow = new JX.Workflow(this._renderURI, params) + .setHandler(JX.bind(this, this._oncontext, target)); + + if (bulk) { + // If we're loading a bunch of these because the viewer clicked + // "Show Entire File Content" or similar, use lower-priority requests + // and draw a progress bar. + this._startContentWorkflow(workflow); + } else { + // If this is a single click on a context link, use a higher priority + // load without a chrome change. + workflow.start(); + } + + return this; + }, + + _startContentWorkflow: function(workflow) { + var routable = workflow.getRoutable(); + + routable + .setPriority(500) + .setType('content') + .setKey(this._getRoutableKey()); + + JX.Router.getInstance().queue(routable); + }, + + + /** + * Receive a response to a context request. + */ + _oncontext: function(target, response) { + var table = JX.$H(response.changeset).getNode(); + var root = target.parentNode; + this._moveRows(table, root, target); + root.removeChild(target); + }, + + _moveRows: function(src, dst, before) { + var rows = JX.DOM.scry(src, 'tr'); + for (var ii = 0; ii < rows.length; ii++) { + + // Find the table this belongs to. If it's a sub-table, like a + // table in an inline comment, don't copy it. + if (JX.DOM.findAbove(rows[ii], 'table') !== src) { + continue; + } + + if (before) { + dst.insertBefore(rows[ii], before); + } else { + dst.appendChild(rows[ii]); + } + } + }, + + /** + * Get parameters which define the current rendering options. + */ + _getViewParameters: function() { + return { + ref: this._ref, + whitespace: this._whitespace || '', + renderer: this.getRenderer() || '', + highlight: this._highlight || '', + encoding: this._encoding || '' + }; + }, /** * Get the active @{class:JX.Routable} for this changeset. @@ -176,9 +252,7 @@ JX.install('ChangesetViewManager', { // a different one we don't re-render the diffs, because it's a // complicated mess and you could lose inline comments, cursor positions, // etc. - var renderer = (JX.Device.getDevice() == 'desktop') ? '2up' : '1up'; - - return renderer; + return (JX.Device.getDevice() == 'desktop') ? '2up' : '1up'; }, setEncoding: function(encoding) { diff --git a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js index 7d24af836f..d47ec93b13 100644 --- a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js +++ b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js @@ -43,6 +43,25 @@ JX.install('DifferentialInlineCommentEditor', { var table = this.getTable(); var target = exact_row ? row : this._skipOverInlineCommentRows(row); + function copyRows(dst, src, before) { + var rows = JX.DOM.scry(src, 'tr'); + for (var ii = 0; ii < rows.length; ii++) { + + // Find the table this belongs to. If it's a sub-table, like a + // table in an inline comment, don't copy it. + if (JX.DOM.findAbove(rows[ii], 'table') !== src) { + continue; + } + + if (before) { + dst.insertBefore(rows[ii], before); + } else { + dst.appendChild(rows[ii]); + } + } + return rows; + } + return copyRows(table, content, target); }, _removeUndoLink : function() { diff --git a/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js b/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js index bc2e015d53..434a057c07 100644 --- a/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js +++ b/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js @@ -16,18 +16,17 @@ JX.behavior('differential-dropdown-menus', function(config) { var pht = JX.phtize(config.pht); function show_more(container) { + var view = JX.ChangesetViewManager.getForNode(container); + var nodes = JX.DOM.scry(container, 'tr', 'context-target'); for (var ii = 0; ii < nodes.length; ii++) { var show = JX.DOM.scry(nodes[ii], 'a', 'show-more'); for (var jj = 0; jj < show.length; jj++) { - if (JX.Stratcom.getData(show[jj]).type != 'all') { + var data = JX.Stratcom.getData(show[jj]); + if (data.type != 'all') { continue; } - var event_data = { - context : nodes[ii], - show : show[jj] - }; - JX.Stratcom.invoke('differential-reveal-context', null, event_data); + view.loadContext(data.range, nodes[ii], true); } } } diff --git a/webroot/rsrc/js/application/differential/behavior-show-more.js b/webroot/rsrc/js/application/differential/behavior-show-more.js index 3d85674542..4ef4964f7c 100644 --- a/webroot/rsrc/js/application/differential/behavior-show-more.js +++ b/webroot/rsrc/js/application/differential/behavior-show-more.js @@ -5,67 +5,23 @@ * javelin-workflow * javelin-util * javelin-stratcom + * changeset-view-manager */ -JX.behavior('differential-show-more', function(config) { - - function onresponse(context, response) { - var table = JX.$H(response.changeset).getNode(); - var root = context.parentNode; - copyRows(root, table, context); - root.removeChild(context); - } +JX.behavior('differential-show-more', function() { JX.Stratcom.listen( 'click', 'show-more', function(e) { - var event_data = { - context : e.getNodes()['context-target'], - show : e.getNodes()['show-more'] - }; - - JX.Stratcom.invoke('differential-reveal-context', null, event_data); e.kill(); - }); - JX.Stratcom.listen( - 'differential-reveal-context', - null, - function(e) { - var context = e.getData().context; - var data = JX.Stratcom.getData(e.getData().show); + var changeset = e.getNode('differential-changeset'); + var view = JX.ChangesetViewManager.getForNode(changeset); + var data = e.getNodeData('show-more'); + var target = e.getNode('context-target'); - var container = JX.DOM.scry(context, 'td')[0]; - JX.DOM.setContent(container, 'Loading...'); - JX.DOM.alterClass(context, 'differential-show-more-loading', true); - - if (!data.whitespace) { - data.whitespace = config.whitespace; - } - - new JX.Workflow(config.uri, data) - .setHandler(JX.bind(null, onresponse, context)) - .start(); + view.loadContext(data.range, target); }); }); - -function copyRows(dst, src, before) { - var rows = JX.DOM.scry(src, 'tr'); - for (var ii = 0; ii < rows.length; ii++) { - - // Find the table this belongs to. If it's a sub-table, like a - // table in an inline comment, don't copy it. - if (JX.DOM.findAbove(rows[ii], 'table') !== src) { - continue; - } - - if (before) { - dst.insertBefore(rows[ii], before); - } else { - dst.appendChild(rows[ii]); - } - } - return rows; -}