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
2017-05-17 12:51:29 -07:00
parent 1e3c8df1c8
commit f78ce156f1
7 changed files with 114 additions and 177 deletions
--- 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',
--- 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',

--- 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.'),
      ),
    ));

--- 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 {
--- 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: {
--- 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);

--- a/webroot/rsrc/js/application/differential/behavior-toggle-files.js
+++ b/webroot/rsrc/js/application/differential/behavior-toggle-files.js
@@ -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]);
-  };
-
-});