From f78ce156f1144fccd356803eb2dcb88ab200585c Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 May 2017 12:51:29 -0700 Subject: [PATCH] Restore "h" to hide or show files, and modernize file visibility toggling Summary: Ref T12616. This puts "h" back to collapse or expand the current file. This removes some very complicated/messy code around following links in the table of contents and getting files auto-expanded. I suspect no one will miss this, but we can restore it if ayone notices. Test Plan: Pressed "h" to collapse/expand a file. Also used the menu items. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12616 Differential Revision: https://secure.phabricator.com/D17940 --- resources/celerity/map.php | 59 ++++---- resources/celerity/packages.php | 1 - .../view/DifferentialChangesetListView.php | 16 ++- .../differential/changeset-view.css | 1 + .../rsrc/js/application/diff/DiffChangeset.js | 61 ++++++++- .../js/application/diff/DiffChangesetList.js | 25 +++- .../differential/behavior-toggle-files.js | 128 ------------------ 7 files changed, 114 insertions(+), 177 deletions(-) delete mode 100644 webroot/rsrc/js/application/differential/behavior-toggle-files.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 10ff2b3e47..2f9026785d 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,8 +12,8 @@ return array( 'core.pkg.css' => 'a5a2d647', 'core.pkg.js' => '0f87a6eb', 'darkconsole.pkg.js' => '1f9a31bc', - 'differential.pkg.css' => 'ea471cb0', - 'differential.pkg.js' => '31e1b646', + 'differential.pkg.css' => '697405d4', + 'differential.pkg.js' => '07c56ffc', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -64,7 +64,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => 'fe5b1869', 'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', - 'rsrc/css/application/differential/changeset-view.css' => '6a77323e', + 'rsrc/css/application/differential/changeset-view.css' => '15be1064', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -390,14 +390,13 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/diff/DiffChangeset.js' => 'ec32b333', - 'rsrc/js/application/diff/DiffChangesetList.js' => '796922e0', + 'rsrc/js/application/diff/DiffChangeset.js' => '731125f3', + 'rsrc/js/application/diff/DiffChangesetList.js' => '59d1ceb1', 'rsrc/js/application/diff/DiffInline.js' => '3337c065', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', 'rsrc/js/application/differential/behavior-populate.js' => '5e41c819', - 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', 'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d', 'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => 'c93358e3', 'rsrc/js/application/diffusion/behavior-audit-preview.js' => 'd835b03a', @@ -566,7 +565,7 @@ return array( 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '6a77323e', + 'differential-changeset-view-css' => '15be1064', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -620,7 +619,6 @@ return array( 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', 'javelin-behavior-differential-feedback-preview' => 'b064af76', 'javelin-behavior-differential-populate' => '5e41c819', - 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', 'javelin-behavior-differential-user-select' => 'a8d8459d', 'javelin-behavior-diffusion-browse-file' => '054a0f0b', 'javelin-behavior-diffusion-commit-branches' => 'bdaf4d04', @@ -777,8 +775,8 @@ return array( 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', - 'phabricator-diff-changeset' => 'ec32b333', - 'phabricator-diff-changeset-list' => '796922e0', + 'phabricator-diff-changeset' => '731125f3', + 'phabricator-diff-changeset-list' => '59d1ceb1', 'phabricator-diff-inline' => '3337c065', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', @@ -1001,6 +999,9 @@ return array( 'javelin-dom', 'javelin-history', ), + '15be1064' => array( + 'phui-inline-comment-view-css', + ), '17bb8539' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1348,6 +1349,9 @@ return array( 'javelin-vector', 'javelin-dom', ), + '59d1ceb1' => array( + 'javelin-install', + ), '5c54cbf3' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1409,9 +1413,6 @@ return array( '69adf288' => array( 'javelin-install', ), - '6a77323e' => array( - 'phui-inline-comment-view-css', - ), '6ad39b6f' => array( 'javelin-install', 'javelin-event', @@ -1449,6 +1450,17 @@ return array( 'javelin-workflow', 'phabricator-draggable-list', ), + '731125f3' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '7319e029' => array( 'javelin-behavior', 'javelin-dom', @@ -1482,9 +1494,6 @@ return array( 'javelin-behavior', 'javelin-quicksand', ), - '796922e0' => array( - 'javelin-install', - ), '7a68dda3' => array( 'owners-path-editor', 'javelin-behavior', @@ -1960,12 +1969,6 @@ return array( 'phabricator-shaped-request', 'conpherence-thread-manager', ), - 'ca3f91eb' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'phabricator-phtize', - ), 'caade6f2' => array( 'javelin-behavior', 'javelin-request', @@ -2131,17 +2134,6 @@ return array( 'javelin-dom', 'phabricator-draggable-list', ), - 'ec32b333' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), 'eded9ee8' => array( 'javelin-behavior', 'javelin-typeahead-ondemand-source', @@ -2418,7 +2410,6 @@ return array( 'javelin-behavior-phabricator-object-selector', 'javelin-behavior-repository-crossreference', 'javelin-behavior-load-blame', - 'javelin-behavior-differential-toggle-files', 'javelin-behavior-differential-user-select', 'javelin-behavior-aphront-more', 'phabricator-diff-inline', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index 619a5d6389..e756e696cb 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -200,7 +200,6 @@ return array( 'javelin-behavior-repository-crossreference', 'javelin-behavior-load-blame', - 'javelin-behavior-differential-toggle-files', 'javelin-behavior-differential-user-select', 'javelin-behavior-aphront-more', diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index aa34b0415f..4ac2612ecd 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -131,13 +131,6 @@ final class DifferentialChangesetListView extends AphrontView { $changesets = $this->changesets; - Javelin::initBehavior('differential-toggle-files', array( - 'pht' => array( - 'undo' => pht('Undo'), - 'collapsed' => pht('This file content has been collapsed.'), - ), - )); - $renderer = DifferentialChangesetParser::getDefaultRendererForViewer( $viewer); @@ -271,6 +264,15 @@ final class DifferentialChangesetListView extends AphrontView { pht('Jump to next inline comment, including hidden comments.'), 'Jump to previous inline comment, including hidden comments.' => pht('Jump to previous inline comment, including hidden comments.'), + + 'This file content has been collapsed.' => + pht('This file content has been collapsed.'), + 'Show Content' => pht('Show Content'), + + 'Hide or show the current file.' => + pht('Hide or show the current file.'), + 'You must select a file to hide or show.' => + pht('You must select a file to hide or show.'), ), )); diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 92a544c24c..0cec38ffdc 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -325,6 +325,7 @@ td.cov-I { border: 1px solid {$blue}; text-align: center; background-color: {$lightblue}; + margin: 8px; } .differential-collapse-undo a { diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index aca035ea0c..c3359c2007 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -55,6 +55,9 @@ JX.install('DiffChangeset', { _rightID: null, _inlines: null, + _visible: true, + + _undoNode: null, getLeftChangesetID: function() { return this._leftID; @@ -345,6 +348,10 @@ JX.install('DiffChangeset', { } }); + if (!this._visible) { + return items; + } + var rows = JX.DOM.scry(this._node, 'tr'); var blocks = []; @@ -635,8 +642,60 @@ JX.install('DiffChangeset', { // them to this Changeset's list of inlines. this.getInlineForRow(row); } - } + }, + toggleVisibility: function() { + this._visible = !this._visible; + + var diff = JX.DOM.find(this._node, 'table', 'differential-diff'); + var undo = this._getUndoNode(); + + if (this._visible) { + JX.DOM.show(diff); + JX.DOM.remove(undo); + } else { + JX.DOM.hide(diff); + JX.DOM.appendContent(diff.parentNode, undo); + } + + JX.Stratcom.invoke('resize'); + }, + + _getUndoNode: function() { + if (!this._undoNode) { + var pht = this.getChangesetList().getTranslations(); + + var link_attributes = { + href: '#' + }; + + var undo_link = JX.$N('a', link_attributes, pht('Show Content')); + + var onundo = JX.bind(this, this._onundo); + JX.DOM.listen(undo_link, 'click', null, onundo); + + var node_attributes = { + className: 'differential-collapse-undo' + }; + + var node_content = [ + pht('This file content has been collapsed.'), + ' ', + undo_link + ]; + + var undo_node = JX.$N('div', node_attributes, node_content); + + this._undoNode = undo_node; + } + + return this._undoNode; + }, + + _onundo: function(e) { + e.kill(); + this.toggleVisibility(); + } }, statics: { diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index 6e3853a9d1..ff51cd4666 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -161,6 +161,9 @@ JX.install('DiffChangesetList', { 'Jump to previous inline comment, including hidden comments.'); this._installJumpKey('P', label, -1, 'comment', true); + label = pht('Hide or show the current file.'); + this._installKey('h', label, this._onkeytogglefile); + label = pht('Jump to the table of contents.'); this._installKey('t', label, this._ontoc); @@ -385,6 +388,20 @@ JX.install('DiffChangesetList', { this._warnUser(pht('You must select a comment to mark done.')); }, + _onkeytogglefile: function() { + var cursor = this._cursorItem; + + if (cursor) { + if (cursor.type == 'file') { + cursor.changeset.toggleVisibility(); + return; + } + } + + var pht = this.getTranslations(); + this._warnUser(pht('You must select a file to hide or show.')); + }, + _onkeyhide: function() { var cursor = this._cursorItem; @@ -616,14 +633,10 @@ JX.install('DiffChangesetList', { var visible_item = new JX.PHUIXActionView() .setHandler(function(e) { - var diff = JX.DOM.scry( - JX.$(data.containerID), - 'table', - 'differential-diff'); - - JX.Stratcom.invoke('differential-toggle-file', null, {diff: diff}); e.prevent(); menu.close(); + + changeset.toggleVisibility(); }); list.addItem(visible_item); diff --git a/webroot/rsrc/js/application/differential/behavior-toggle-files.js b/webroot/rsrc/js/application/differential/behavior-toggle-files.js deleted file mode 100644 index 296b6f91b2..0000000000 --- a/webroot/rsrc/js/application/differential/behavior-toggle-files.js +++ /dev/null @@ -1,128 +0,0 @@ -/** - * @provides javelin-behavior-differential-toggle-files - * @requires javelin-behavior - * javelin-dom - * javelin-stratcom - * phabricator-phtize - */ - -JX.behavior('differential-toggle-files', function(config) { - var pht = JX.phtize(config.pht); - - JX.Stratcom.listen( - 'differential-toggle-file', - null, - function(e) { - if (e.getData().diff.length != 1) { - return; - } - - var diff = e.getData().diff[0], - data = JX.Stratcom.getData(diff); - if (data.hidden) { - data.hidden = false; - JX.DOM.show(diff); - JX.DOM.remove(data.undo); - data.undo = null; - } else { - data.hidden = true; - data.undo = render_collapse_undo(); - JX.DOM.hide(diff); - JX.DOM.listen( - data.undo, - 'click', - 'differential-collapse-undo', - function(e) { - e.kill(); - data.hidden = false; - JX.DOM.show(diff); - JX.DOM.remove(data.undo); - data.undo = null; - }); - JX.DOM.appendContent(diff.parentNode, data.undo); - } - JX.Stratcom.invoke('differential-toggle-file-toggled'); - }); - - JX.Stratcom.listen( - 'differential-toggle-file-request', - null, - function(e) { - var elt = e.getData().element; - while (elt !== document.body) { - if (JX.Stratcom.hasSigil(elt, 'differential-changeset')) { - var diffs = JX.DOM.scry(elt, 'table', 'differential-diff'); - var invoked = false; - for (var i = 0; i < diffs.length; ++i) { - if (JX.Stratcom.getData(diffs[i]).hidden) { - JX.Stratcom.invoke('differential-toggle-file', null, { - diff: [ diffs[i] ] - }); - invoked = true; - } - } - if (!invoked) { - e.prevent(); - } - return; - } - elt = elt.parentNode; - } - e.prevent(); - }); - - JX.Stratcom.listen( - 'click', - 'tag:a', - function(e) { - var link = e.getNode('tag:a'); - var id = link.getAttribute('href'); - if (!id || !id.match(/^#.+/)) { - return; - } - var raw = e.getRawEvent(); - if (raw.altKey || raw.ctrlKey || raw.metaKey || raw.shiftKey) { - return; - } - // The target may have either a matching name or a matching id. - var target; - try { - target = JX.$(id.substr(1)); - } catch(err) { - var named = document.getElementsByName(id.substr(1)); - for (var i = 0; i < named.length; ++i) { - if (named[i].tagName.toLowerCase() == 'a') { - if (target) { - return; - } - target = named[i]; - } - } - if (!target) { - return; - } - } - var event = JX.Stratcom.invoke('differential-toggle-file-request', null, { - element: target - }); - if (!event.getPrevented()) { - // This event is processed after the hash has changed, so it doesn't - // automatically jump there like we want. - JX.DOM.scrollTo(target); - } - }); - - var render_collapse_undo = function() { - var link = JX.$N( - 'a', - {href: '#', sigil: 'differential-collapse-undo'}, - pht('undo')); - - return JX.$N( - 'div', - {className: 'differential-collapse-undo', - sigil: 'differential-collapse-undo-div'}, - [pht('collapsed'), ' ', link]); - }; - -});