Add options for hidding inlines to the Differential header banner

Summary:
Fixes T8909. Ref T12733.

UI attempts to follow the mock, but is a bit rough since PHUIXButtonView without text in this menu gets weird spacing, we don't have circular buttons yet, and PHUIXActionView without an icon also gets odd spacing.

Test Plan: {F5003125}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733, T8909

Differential Revision: https://secure.phabricator.com/D18128

resources/celerity/map.php

@@ -13,7 +13,7 @@ return array(
     'core.pkg.js' => '1475bd91',
     'darkconsole.pkg.js' => '1f9a31bc',
     'differential.pkg.css' => '4ec4a37a',
-    'differential.pkg.js' => '3442216b',
+    'differential.pkg.js' => 'a55a2c13',
     'diffusion.pkg.css' => 'b93d9b8c',
     'diffusion.pkg.js' => '6134c5a1',
     'favicon.ico' => '30672e08',
@@ -395,9 +395,9 @@ 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' => 'cdc5fa19',
+    'rsrc/js/application/diff/DiffChangeset.js' => '99abf4cd',
-    'rsrc/js/application/diff/DiffChangesetList.js' => '4ca11264',
+    'rsrc/js/application/diff/DiffChangesetList.js' => 'd442be4a',
-    'rsrc/js/application/diff/DiffInline.js' => '27b6d01f',
+    'rsrc/js/application/diff/DiffInline.js' => '1bfa31c7',
     'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
     'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07',
     'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
@@ -774,9 +774,9 @@ return array(
     'phabricator-darklog' => 'c8e1ffe3',
     'phabricator-darkmessage' => 'c48cccdd',
     'phabricator-dashboard-css' => 'fe5b1869',
-    'phabricator-diff-changeset' => 'cdc5fa19',
+    'phabricator-diff-changeset' => '99abf4cd',
-    'phabricator-diff-changeset-list' => '4ca11264',
+    'phabricator-diff-changeset-list' => 'd442be4a',
-    'phabricator-diff-inline' => '27b6d01f',
+    'phabricator-diff-inline' => '1bfa31c7',
     'phabricator-drag-and-drop-file-upload' => '58dea2fa',
     'phabricator-draggable-list' => 'bea6e7f4',
     'phabricator-fatal-config-template-css' => '8f18fa41',
@@ -1016,6 +1016,9 @@ return array(
       'javelin-request',
       'javelin-uri',
     ),
+    '1bfa31c7' => array(
+      'javelin-dom',
+    ),
     '1e911d0f' => array(
       'javelin-stratcom',
       'javelin-request',
@@ -1056,9 +1059,6 @@ return array(
       'phabricator-drag-and-drop-file-upload',
       'javelin-workboard-board',
     ),
-    '27b6d01f' => array(
-      'javelin-dom',
-    ),
     '2926fff2' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1234,10 +1234,6 @@ return array(
       'javelin-uri',
       'phabricator-notification',
     ),
-    '4ca11264' => array(
-      'javelin-install',
-      'phuix-button-view',
-    ),
     '4d863052' => array(
       'javelin-dom',
       'javelin-util',
@@ -1626,6 +1622,17 @@ return array(
       'javelin-mask',
       'phabricator-drag-and-drop-file-upload',
     ),
+    '99abf4cd' => array(
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-install',
+      'javelin-workflow',
+      'javelin-router',
+      'javelin-behavior-device',
+      'javelin-vector',
+      'phabricator-diff-inline',
+    ),
     '9a6dd75c' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1956,17 +1963,6 @@ return array(
     'cd2b9b77' => array(
       'phui-oi-list-view-css',
     ),
-    'cdc5fa19' => array(
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-install',
-      'javelin-workflow',
-      'javelin-router',
-      'javelin-behavior-device',
-      'javelin-vector',
-      'phabricator-diff-inline',
-    ),
     'd0a99ab4' => array(
       'javelin-behavior',
       'javelin-typeahead-ondemand-source',
@@ -1991,6 +1987,10 @@ return array(
     'd254d646' => array(
       'javelin-util',
     ),
+    'd442be4a' => array(
+      'javelin-install',
+      'phuix-button-view',
+    ),
     'd4505101' => array(
       'javelin-stratcom',
       'javelin-install',

src/applications/differential/view/DifferentialChangesetListView.php

@@ -279,6 +279,12 @@ final class DifferentialChangesetListView extends AphrontView {
         'Unsaved' => pht('Unsaved'),
         'Unsubmitted' => pht('Unsubmitted'),
         'Comments' => pht('Comments'),
+
+        'Hide "Done" Inlines' => pht('Hide "Done" Inlines'),
+        'Hide Collapsed Inlines' => pht('Hide Collapsed Inlines'),
+        'Hide Older Inlines' => pht('Hide Older Inlines'),
+        'Hide All Inlines' => pht('Hide All Inlines'),
+        'Show All Inlines' => pht('Show All Inlines'),
       ),
     ));
 

webroot/rsrc/js/application/diff/DiffChangeset.js

@@ -446,6 +446,7 @@ JX.install('DiffChangeset', {
               type: block.type,
               changeset: this,
               target: inline,
+              hidden: inline.isHidden(),
               collapsed: inline.isCollapsed(),
               deleted: !inline.getID() && !inline.isEditing(),
               nodes: {

webroot/rsrc/js/application/diff/DiffChangesetList.js

@@ -117,6 +117,10 @@ JX.install('DiffChangesetList', {
     _doneButton: null,
     _doneMode: null,
 
+    _dropdownMenu: null,
+    _menuButton: null,
+    _menuItems: null,
+
     sleep: function() {
       this._asleep = true;
 
@@ -458,6 +462,7 @@ JX.install('DiffChangesetList', {
       var collapsed = options.collapsed || false;
       var wrap = options.wrap || false;
       var attribute = options.attribute || null;
+      var show = options.show || false;
 
       var cursor = state.cursor;
       var items = state.items;
@@ -529,6 +534,15 @@ JX.install('DiffChangesetList', {
           }
         }
 
+        // If this item is a hidden inline but we're clicking a button which
+        // selects inlines of a particular type, make it visible again.
+        if (items[cursor].hidden) {
+          if (!show) {
+            continue;
+          }
+          items[cursor].target.setHidden(false);
+        }
+
         // Otherwise, we've found a valid item to select.
         break;
       }
@@ -1314,6 +1328,13 @@ JX.install('DiffChangesetList', {
     },
 
     _redrawBanner: function() {
+      // If the inline comment menu is open and we've done a redraw, close it.
+      // In particular, this makes it close when you scroll the document:
+      // otherwise, it stays open but the banner moves underneath it.
+      if (this._dropdownMenu) {
+        this._dropdownMenu.close();
+      }
+
       var node = this._getBannerNode();
       var changeset = this._getVisibleChangeset();
 
@@ -1329,49 +1350,13 @@ JX.install('DiffChangesetList', {
         return;
       }
 
-      var changesets = this._changesets;
+      var inlines = this._getInlinesByType();
-      var unsaved = [];
-      var unsubmitted = [];
-      var draft_done = [];
-      var undone = [];
-      var done = [];
 
-      for (var ii = 0; ii < changesets.length; ii++) {
+      var unsaved = inlines.unsaved;
-        var inlines = changesets[ii].getInlines();
+      var unsubmitted = inlines.unsubmitted;
-        for (var jj = 0; jj < inlines.length; jj++) {
+      var undone = inlines.undone;
-          var inline = inlines[jj];
+      var done = inlines.done;
-
+      var draft_done = inlines.draftDone;
-          if (inline.isDeleted()) {
-            continue;
-          }
-
-          if (inline.isSynthetic()) {
-            continue;
-          }
-
-          if (inline.isEditing()) {
-            unsaved.push(inline);
-          } else if (!inline.getID()) {
-            // These are new comments which have been cancelled, and do not
-            // count as anything.
-            continue;
-          } else if (inline.isDraft()) {
-            unsubmitted.push(inline);
-          } else {
-            // NOTE: Unlike other states, an inline may be marked with a
-            // draft checkmark and still be a "done" or "undone" comment.
-            if (inline.isDraftDone()) {
-              draft_done.push(inline);
-            }
-
-            if (!inline.isDone()) {
-              undone.push(inline);
-            } else {
-              done.push(inline);
-            }
-          }
-        }
-      }
 
       JX.DOM.alterClass(
         node,
@@ -1392,6 +1377,7 @@ JX.install('DiffChangesetList', {
       var unsaved_button = this._getUnsavedButton();
       var unsubmitted_button = this._getUnsubmittedButton();
       var done_button = this._getDoneButton();
+      var menu_button = this._getMenuButton();
 
       if (unsaved.length) {
         unsaved_button.setText(unsaved.length + ' ' + pht('Unsaved'));
@@ -1457,7 +1443,8 @@ JX.install('DiffChangesetList', {
       var buttons_list = [
         unsaved_button.getNode(),
         unsubmitted_button.getNode(),
-        done_button.getNode()
+        done_button.getNode(),
+        menu_button.getNode()
       ];
 
       var buttons_view = JX.$N('div', buttons_attrs, buttons_list);
@@ -1470,6 +1457,104 @@ JX.install('DiffChangesetList', {
       document.body.appendChild(node);
     },
 
+    _getInlinesByType: function() {
+      var changesets = this._changesets;
+      var unsaved = [];
+      var unsubmitted = [];
+      var undone = [];
+      var done = [];
+      var draft_done = [];
+
+      var visible_done = [];
+      var visible_collapsed = [];
+      var visible_ghosts = [];
+      var visible = [];
+      var hidden = [];
+
+      for (var ii = 0; ii < changesets.length; ii++) {
+        var inlines = changesets[ii].getInlines();
+        var inline;
+        var jj;
+        for (jj = 0; jj < inlines.length; jj++) {
+          inline = inlines[jj];
+
+          if (inline.isDeleted()) {
+            continue;
+          }
+
+          if (inline.isSynthetic()) {
+            continue;
+          }
+
+          if (inline.isEditing()) {
+            unsaved.push(inline);
+          } else if (!inline.getID()) {
+            // These are new comments which have been cancelled, and do not
+            // count as anything.
+            continue;
+          } else if (inline.isDraft()) {
+            unsubmitted.push(inline);
+          } else {
+            // NOTE: Unlike other states, an inline may be marked with a
+            // draft checkmark and still be a "done" or "undone" comment.
+            if (inline.isDraftDone()) {
+              draft_done.push(inline);
+            }
+
+            if (!inline.isDone()) {
+              undone.push(inline);
+            } else {
+              done.push(inline);
+            }
+          }
+        }
+
+        for (jj = 0; jj < inlines.length; jj++) {
+          inline = inlines[jj];
+          if (inline.isDeleted()) {
+            continue;
+          }
+
+          if (inline.isEditing()) {
+            continue;
+          }
+
+          if (inline.isHidden()) {
+            hidden.push(inline);
+            continue;
+          }
+
+          visible.push(inline);
+
+          if (inline.isDone()) {
+            visible_done.push(inline);
+          }
+
+          if (inline.isCollapsed()) {
+            visible_collapsed.push(inline);
+          }
+
+          if (inline.isGhost()) {
+            visible_ghosts.push(inline);
+          }
+        }
+      }
+
+      return {
+        unsaved: unsaved,
+        unsubmitted: unsubmitted,
+        undone: undone,
+        done: done,
+        draftDone: draft_done,
+        visibleDone: visible_done,
+        visibleGhosts: visible_ghosts,
+        visibleCollapsed: visible_collapsed,
+        visible: visible,
+        hidden: hidden
+      };
+
+    },
+
     _getUnsavedButton: function() {
       if (!this._unsavedButton) {
         var button = new JX.PHUIXButtonView()
@@ -1520,12 +1605,126 @@ JX.install('DiffChangesetList', {
 
       return this._doneButton;
     },
+
+    _getMenuButton: function() {
+      if (!this._menuButton) {
+        var button = new JX.PHUIXButtonView()
+          .setIcon('fa-gear')
+          .setButtonType(JX.PHUIXButtonView.BUTTONTYPE_SIMPLE);
+
+        var dropdown = new JX.PHUIXDropdownMenu(button.getNode());
+        this._menuItems = {};
+
+        var list = new JX.PHUIXActionListView();
+        dropdown.setContent(list.getNode());
+
+        var map = {
+          hideDone: {
+            type: 'done'
+          },
+          hideCollapsed: {
+            type: 'collapsed'
+          },
+          hideGhosts: {
+            type: 'ghosts'
+          },
+          hideAll: {
+            type: 'all'
+          },
+          showAll: {
+            type: 'show'
+          }
+        };
+
+        for (var k in map) {
+          var spec = map[k];
+
+          var handler = JX.bind(this, this._onhideinlines, spec.type);
+          var item = new JX.PHUIXActionView()
+            .setHandler(handler);
+
+          list.addItem(item);
+          this._menuItems[k] = item;
+        }
+
+        dropdown.listen('open', JX.bind(this, this._ondropdown));
+
+        this._menuButton = button;
+        this._dropdownMenu = dropdown;
+      }
+
+      return this._menuButton;
+    },
+
+    _ondropdown: function() {
+      var inlines = this._getInlinesByType();
+      var items = this._menuItems;
+      var pht = this.getTranslations();
+
+      items.hideDone
+        .setName(pht('Hide "Done" Inlines'))
+        .setDisabled(!inlines.visibleDone.length);
+
+      items.hideCollapsed
+        .setName(pht('Hide Collapsed Inlines'))
+        .setDisabled(!inlines.visibleCollapsed.length);
+
+      items.hideGhosts
+        .setName(pht('Hide Older Inlines'))
+        .setDisabled(!inlines.visibleGhosts.length);
+
+      items.hideAll
+        .setName(pht('Hide All Inlines'))
+        .setDisabled(!inlines.visible.length);
+
+      items.showAll
+        .setName(pht('Show All Inlines'))
+        .setDisabled(!inlines.hidden.length);
+    },
+
+    _onhideinlines: function(type, e) {
+      this._dropdownMenu.close();
+      e.prevent();
+
+      var inlines = this._getInlinesByType();
+
+      // Clear the selection state since we end up in a weird place if the
+      // user hides the selected inline.
+      this._setSelectionState(null);
+
+      var targets;
+      var mode = true;
+      switch (type) {
+        case 'done':
+          targets = inlines.visibleDone;
+          break;
+        case 'collapsed':
+          targets = inlines.visibleCollapsed;
+          break;
+        case 'ghosts':
+          targets = inlines.visibleGhosts;
+          break;
+        case 'all':
+          targets = inlines.visible;
+          break;
+        case 'show':
+          targets = inlines.hidden;
+          mode = false;
+          break;
+      }
+
+      for (var ii = 0; ii < targets.length; ii++) {
+        targets[ii].setHidden(mode);
+      }
+    },
+
     _onunsavedclick: function(e) {
       e.kill();
 
       var options = {
         filter: 'comment',
         wrap: true,
+        show: true,
         attribute: 'unsaved'
       };
 
@@ -1538,6 +1737,7 @@ JX.install('DiffChangesetList', {
       var options = {
         filter: 'comment',
         wrap: true,
+        show: true,
         attribute: 'anyDraft'
       };
 
@@ -1550,6 +1750,7 @@ JX.install('DiffChangesetList', {
       var options = {
         filter: 'comment',
         wrap: true,
+        show: true,
         attribute: this._doneMode
       };
 

webroot/rsrc/js/application/diff/DiffInline.js

@@ -36,6 +36,7 @@ JX.install('DiffInline', {
     _isEditing: false,
     _isNew: false,
     _isSynthetic: false,
+    _isHidden: false,
 
     bindToRow: function(row) {
       this._row = row;
@@ -109,6 +110,14 @@ JX.install('DiffInline', {
       return this._isDraftDone;
     },
 
+    isHidden: function() {
+      return this._isHidden;
+    },
+
+    isGhost: function() {
+      return this._isGhost;
+    },
+
     bindToRange: function(data) {
       this._displaySide = data.displaySide;
       this._number = parseInt(data.number, 10);
@@ -207,6 +216,12 @@ JX.install('DiffInline', {
       return this;
     },
 
+    setHidden: function(hidden) {
+      this._isHidden = hidden;
+      this._redraw();
+      return this;
+    },
+
     canReply: function() {
       if (!this._hasAction('reply')) {
         return false;
@@ -708,9 +723,10 @@ JX.install('DiffInline', {
     },
 
     _redraw: function() {
-      var is_invisible = (this._isInvisible || this._isDeleted);
+      var is_invisible =
+        (this._isInvisible || this._isDeleted || this._isHidden);
       var is_loading = this._isLoading;
-      var is_collapsed = this._isCollapsed;
+      var is_collapsed = (this._isCollapsed && !this._isHidden);
 
       var row = this._row;
       JX.DOM.alterClass(row, 'differential-inline-hidden', is_invisible);