Update the "View Options" menu for recent filetree changes

Summary:
Ref T13516. Minor improvements here:

  - Show key commands in the "View Options" dropdown.
  - Organize it slightly better.
  - Improve disabled item behaviors a little bit.
  - Add a "Browse Directory" action.
  - Rename "...in Diffusion" to "...in Repository".
  - Make "d", "D", and "h" use the same targeting rules as "\".
  - When you hide a file with the "h" menu item, select it.

Test Plan: Poked at the menu a lot, ran into less questionable behavior.

Maniphest Tasks: T13516

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

resources/celerity/map.php

@@ -9,10 +9,10 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => 'dd5f04a3',
+    'core.pkg.css' => '61b7e380',
-    'core.pkg.js' => '544bc792',
+    'core.pkg.js' => 'fc49f65b',
     'differential.pkg.css' => 'cb99cd21',
-    'differential.pkg.js' => '54613dd5',
+    'differential.pkg.js' => 'ae77bf85',
     'diffusion.pkg.css' => '42c75c37',
     'diffusion.pkg.js' => 'a98c0bf7',
     'maniphest.pkg.css' => '35995d6d',
@@ -134,7 +134,7 @@ return array(
     'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e',
     'rsrc/css/phui/object-item/phui-oi-list-view.css' => 'd7723ecc',
     'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46',
-    'rsrc/css/phui/phui-action-list.css' => 'e820263c',
+    'rsrc/css/phui/phui-action-list.css' => '1b0085b2',
     'rsrc/css/phui/phui-action-panel.css' => '6c386cbf',
     'rsrc/css/phui/phui-badge.css' => '666e25ad',
     'rsrc/css/phui/phui-basic-nav-view.css' => '56ebd66d',
@@ -378,8 +378,8 @@ return array(
     'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be',
     'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
     'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
-    'rsrc/js/application/diff/DiffChangeset.js' => 'dd1a6f34',
+    'rsrc/js/application/diff/DiffChangeset.js' => '27305b60',
-    'rsrc/js/application/diff/DiffChangesetList.js' => '57035863',
+    'rsrc/js/application/diff/DiffChangesetList.js' => '62a3a351',
     'rsrc/js/application/diff/DiffInline.js' => '16e97ebc',
     'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
     'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
@@ -520,7 +520,7 @@ return array(
     'rsrc/js/phui/behavior-phui-tab-group.js' => '242aa08b',
     'rsrc/js/phui/behavior-phui-timer-control.js' => 'f84bcbf4',
     'rsrc/js/phuix/PHUIXActionListView.js' => 'c68f183f',
-    'rsrc/js/phuix/PHUIXActionView.js' => 'aaa08f3b',
+    'rsrc/js/phuix/PHUIXActionView.js' => 'a8f573a9',
     'rsrc/js/phuix/PHUIXAutocomplete.js' => '2fbe234d',
     'rsrc/js/phuix/PHUIXButtonView.js' => '55a24e84',
     'rsrc/js/phuix/PHUIXDropdownMenu.js' => '7acfd98b',
@@ -766,7 +766,7 @@ return array(
     'path-typeahead' => 'ad486db3',
     'people-picture-menu-item-css' => 'fe8e07cf',
     'people-profile-css' => '2ea2daa1',
-    'phabricator-action-list-view-css' => 'e820263c',
+    'phabricator-action-list-view-css' => '1b0085b2',
     'phabricator-busy' => '5202e831',
     'phabricator-chatlog-css' => 'abdc76ee',
     'phabricator-content-source-view-css' => 'cdf0d579',
@@ -775,8 +775,8 @@ return array(
     'phabricator-darklog' => '3b869402',
     'phabricator-darkmessage' => '26cd4b73',
     'phabricator-dashboard-css' => '5a205b9d',
-    'phabricator-diff-changeset' => 'dd1a6f34',
+    'phabricator-diff-changeset' => '27305b60',
-    'phabricator-diff-changeset-list' => '57035863',
+    'phabricator-diff-changeset-list' => '62a3a351',
     'phabricator-diff-inline' => '16e97ebc',
     'phabricator-diff-path-view' => '8207abf9',
     'phabricator-diff-tree-view' => '5d83623b',
@@ -884,7 +884,7 @@ return array(
     'phui-workcard-view-css' => '913441b6',
     'phui-workpanel-view-css' => '3ae89b20',
     'phuix-action-list-view' => 'c68f183f',
-    'phuix-action-view' => 'aaa08f3b',
+    'phuix-action-view' => 'a8f573a9',
     'phuix-autocomplete' => '2fbe234d',
     'phuix-button-view' => '55a24e84',
     'phuix-dropdown-menu' => '7acfd98b',
@@ -1146,6 +1146,18 @@ return array(
       'javelin-json',
       'phabricator-prefab',
     ),
+    '27305b60' => array(
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-install',
+      'javelin-workflow',
+      'javelin-router',
+      'javelin-behavior-device',
+      'javelin-vector',
+      'phabricator-diff-inline',
+      'phabricator-diff-path-view',
+    ),
     '289bf236' => array(
       'javelin-install',
       'javelin-util',
@@ -1424,11 +1436,6 @@ return array(
       'javelin-stratcom',
       'javelin-dom',
     ),
-    57035863 => array(
-      'javelin-install',
-      'phuix-button-view',
-      'phabricator-diff-tree-view',
-    ),
     '5793d835' => array(
       'javelin-install',
       'javelin-util',
@@ -1514,6 +1521,11 @@ return array(
     '60cd9241' => array(
       'javelin-behavior',
     ),
+    '62a3a351' => array(
+      'javelin-install',
+      'phuix-button-view',
+      'phabricator-diff-tree-view',
+    ),
     '65bb0011' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1863,6 +1875,11 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
+    'a8f573a9' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-util',
+    ),
     'a9942052' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1898,11 +1915,6 @@ return array(
       'javelin-json',
       'phuix-form-control-view',
     ),
-    'aaa08f3b' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-util',
-    ),
     'ab85e184' => array(
       'javelin-install',
       'javelin-dom',
@@ -2108,18 +2120,6 @@ return array(
       'javelin-uri',
       'phabricator-notification',
     ),
-    'dd1a6f34' => array(
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-install',
-      'javelin-workflow',
-      'javelin-router',
-      'javelin-behavior-device',
-      'javelin-vector',
-      'phabricator-diff-inline',
-      'phabricator-diff-path-view',
-    ),
     'e150bd50' => array(
       'javelin-behavior',
       'javelin-stratcom',

src/applications/differential/view/DifferentialChangesetDetailView.php

@@ -14,6 +14,7 @@ final class DifferentialChangesetDetailView extends AphrontView {
   private $repository;
   private $diff;
   private $changesetResponse;
+  private $branch;
 
   public function setAutoload($autoload) {
     $this->autoload = $autoload;
@@ -71,6 +72,15 @@ final class DifferentialChangesetDetailView extends AphrontView {
     return $this;
   }
 
+  public function setBranch($branch) {
+    $this->branch = $branch;
+    return $this;
+  }
+
+  public function getBranch() {
+    return $this->branch;
+  }
+
   public function getID() {
     if (!$this->id) {
       $this->id = celerity_generate_unique_node_id();
@@ -93,6 +103,8 @@ final class DifferentialChangesetDetailView extends AphrontView {
   }
 
   public function render() {
+    $viewer = $this->getViewer();
+
     $this->requireResource('differential-changeset-view-css');
     $this->requireResource('syntax-highlighting-css');
 
@@ -181,6 +193,46 @@ final class DifferentialChangesetDetailView extends AphrontView {
     $path_parts = trim($display_filename, '/');
     $path_parts = explode('/', $path_parts);
 
+    $show_path_uri = null;
+    $show_directory_uri = null;
+
+    $repository = $this->getRepository();
+    if ($repository) {
+      $diff = $this->getDiff();
+      if ($diff) {
+        $repo_path = $changeset->getAbsoluteRepositoryPath($repository, $diff);
+
+        $repo_dir = dirname($repo_path);
+        if ($repo_dir === $repo_path) {
+          $repo_dir = null;
+        }
+
+        $show_path_uri = $repository->getDiffusionBrowseURIForPath(
+          $viewer,
+          $repo_path,
+          idx($changeset->getMetadata(), 'line:first'),
+          $this->getBranch());
+
+        if ($repo_dir !== null) {
+          $repo_dir = rtrim($repo_dir, '/').'/';
+
+          $show_directory_uri = $repository->getDiffusionBrowseURIForPath(
+            $viewer,
+            $repo_dir,
+            null,
+            $this->getBranch());
+        }
+      }
+    }
+
+    if ($show_path_uri) {
+      $show_path_uri = phutil_string_cast($show_path_uri);
+    }
+
+    if ($show_directory_uri) {
+      $show_directory_uri = phutil_string_cast($show_directory_uri);
+    }
+
     return javelin_tag(
       'div',
       array(
@@ -205,6 +257,9 @@ final class DifferentialChangesetDetailView extends AphrontView {
 
           'loaded' => $is_loaded,
           'changesetState' => $changeset_state,
+
+          'showPathURI' => $show_path_uri,
+          'showDirectoryURI' => $show_directory_uri,
         ),
         'class' => $class,
         'id'    => $id,

src/applications/differential/view/DifferentialChangesetListView.php

@@ -198,6 +198,7 @@ final class DifferentialChangesetListView extends AphrontView {
       $detail->setVsChangesetID(idx($this->vsMap, $changeset->getID()));
       $detail->setEditable(true);
       $detail->setRenderingRef($ref);
+      $detail->setBranch($this->getBranch());
 
       $detail->setRenderURI($this->renderURI);
 
@@ -263,17 +264,18 @@ final class DifferentialChangesetListView extends AphrontView {
         "Can't Toggle Unloaded File" => pht("Can't Toggle Unloaded File"),
         'Expand File' => pht('Expand File'),
         'Collapse File' => pht('Collapse File'),
-        'Browse in Diffusion' => pht('Browse in Diffusion'),
+        'Show Path in Repository' => pht('Show Path in Repository'),
+        'Show Directory in Repository' => pht('Show Directory in Repository'),
         'View Standalone' => pht('View Standalone'),
         'Show Raw File (Left)' => pht('Show Raw File (Left)'),
         'Show Raw File (Right)' => pht('Show Raw File (Right)'),
         'Configure Editor' => pht('Configure Editor'),
         'Load Changes' => pht('Load Changes'),
-        'View Side-by-Side' => pht('View Side-by-Side'),
+        'View Side-by-Side Diff' => pht('View Side-by-Side Diff'),
-        'View Unified' => pht('View Unified'),
+        'View Unified Diff' => pht('View Unified Diff'),
         'Change Text Encoding...' => pht('Change Text Encoding...'),
         'Highlight As...' => pht('Highlight As...'),
-        'View As...' => pht('View As...'),
+        'View As Document Type...' => pht('View As Document Type...'),
 
         'Loading...' => pht('Loading...'),
 
@@ -350,8 +352,19 @@ final class DifferentialChangesetListView extends AphrontView {
         'You must select a file to edit.' =>
           pht('You must select a file to edit.'),
 
+        'You must select a file to open.' =>
+          pht('You must select a file to open.'),
+
         'No external editor is configured.' =>
           pht('No external editor is configured.'),
+
+        'Hide or show the paths panel.' =>
+          pht('Hide or show the paths panel.'),
+
+        'Show path in repository.' =>
+          pht('Show path in repository.'),
+        'Show directory in repository.' =>
+          pht('Show directory in repository.'),
       ),
     ));
 
@@ -397,20 +410,6 @@ final class DifferentialChangesetListView extends AphrontView {
       $meta['standaloneURI'] = (string)$uri;
     }
 
-    $repository = $this->repository;
-    if ($repository) {
-      try {
-        $meta['diffusionURI'] =
-          (string)$repository->getDiffusionBrowseURIForPath(
-            $viewer,
-            $changeset->getAbsoluteRepositoryPath($repository, $this->diff),
-            idx($changeset->getMetadata(), 'line:first'),
-            $this->getBranch());
-      } catch (DiffusionSetupException $e) {
-        // Ignore
-      }
-    }
-
     $change = $changeset->getChangeType();
 
     if ($this->leftRawFileURI) {

webroot/rsrc/css/phui/phui-action-list.css

@@ -242,3 +242,25 @@
    .phui-icon-view {
   margin-left: 12px;
 }
+
+.phabricator-action-view .keyboard-shortcut-key {
+  display: none;
+}
+
+.phabricator-action-view.has-key-command .keyboard-shortcut-key {
+  display: block;
+  position: absolute;
+  width: 12px;
+  height: 12px;
+  right: 0;
+  top: 2px;
+  line-height: 12px;
+  padding: 4px;
+  color: {$greytext};
+  background: {$bluebackground};
+  border: none;
+}
+
+.phabricator-action-view.has-key-command .phabricator-action-view-item {
+  padding-right: 24px;
+}

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

@@ -34,6 +34,8 @@ JX.install('DiffChangeset', {
 
     this._editorURI = data.editorURI;
     this._editorConfigureURI = data.editorConfigureURI;
+    this._showPathURI = data.showPathURI;
+    this._showDirectoryURI = data.showDirectoryURI;
 
     this._pathIconIcon = data.pathIconIcon;
     this._pathIconColor = data.pathIconColor;
@@ -76,6 +78,9 @@ JX.install('DiffChangeset', {
 
     _editorURI: null,
     _editorConfigureURI: null,
+    _showPathURI: null,
+    _showDirectoryURI: null,
+
     _pathView: null,
 
     _pathIconIcon: null,
@@ -92,6 +97,14 @@ JX.install('DiffChangeset', {
       return this._editorConfigureURI;
     },
 
+    getShowPathURI: function() {
+      return this._showPathURI;
+    },
+
+    getShowDirectoryURI: function() {
+      return this._showDirectoryURI;
+    },
+
     getLeftChangesetID: function() {
       return this._leftID;
     },

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

@@ -198,6 +198,7 @@ JX.install('DiffChangesetList', {
       if (formation) {
         var filetree = formation.getColumn(0);
         var toggletree = JX.bind(filetree, filetree.toggleVisibility);
+        label = pht('Hide or show the paths panel.');
         this._installKey('f', 'diff-vis', label, toggletree);
       }
 
@@ -226,9 +227,14 @@ JX.install('DiffChangesetList', {
       label = pht('Hide or show all inline comments.');
       this._installKey('A', 'diff-vis', label, this._onkeyhideall);
 
+      label = pht('Show path in repository.');
+      this._installKey('d', 'diff-nav', label, this._onkeyshowpath);
+
+      label = pht('Show directory in repository.');
+      this._installKey('D', 'diff-nav', label, this._onkeyshowdirectory);
+
       label = pht('Open file in external editor.');
       this._installKey('\\', 'diff-nav', label, this._onkeyopeneditor);
-
     },
 
     isAsleep: function() {
@@ -456,21 +462,18 @@ JX.install('DiffChangesetList', {
     },
 
     _onkeytogglefile: function() {
-      var cursor = this._cursorItem;
+      var pht = this.getTranslations();
+      var changeset = this._getChangesetForKeyCommand();
 
-      if (cursor) {
+      if (!changeset) {
-        if (cursor.type == 'file') {
+        this._warnUser(pht('You must select a file to hide or show.'));
-          cursor.changeset.toggleVisibility();
+        return;
-          return;
-        }
       }
 
-      var pht = this.getTranslations();
+      changeset.toggleVisibility();
-      this._warnUser(pht('You must select a file to hide or show.'));
     },
 
-    _onkeyopeneditor: function() {
+    _getChangesetForKeyCommand: function() {
-      var pht = this.getTranslations();
       var cursor = this._cursorItem;
 
       var changeset;
@@ -482,6 +485,13 @@ JX.install('DiffChangesetList', {
         changeset = this._getVisibleChangeset();
       }
 
+      return changeset;
+    },
+
+    _onkeyopeneditor: function() {
+      var pht = this.getTranslations();
+      var changeset = this._getChangesetForKeyCommand();
+
       if (!changeset) {
         this._warnUser(pht('You must select a file to edit.'));
         return;
@@ -497,6 +507,37 @@ JX.install('DiffChangesetList', {
       JX.$U(editor_uri).go();
     },
 
+    _onkeyshowpath: function() {
+      this._onrepositorykey(false);
+    },
+
+    _onkeyshowdirectory: function() {
+      this._onrepositorykey(true);
+    },
+
+    _onrepositorykey: function(is_directory) {
+      var pht = this.getTranslations();
+      var changeset = this._getChangesetForKeyCommand();
+
+      if (!changeset) {
+        this._warnUser(pht('You must select a file to open.'));
+        return;
+      }
+
+      var show_uri;
+      if (is_directory) {
+        show_uri = changeset.getShowDirectoryURI();
+      } else {
+        show_uri = changeset.getShowPathURI();
+      }
+
+      if (show_uri === null) {
+        return;
+      }
+
+      window.open(show_uri);
+    },
+
     _onkeycollapse: function() {
       var cursor = this._cursorItem;
 
@@ -661,7 +702,7 @@ JX.install('DiffChangesetList', {
       }
 
       if (cursor !== null) {
-        this._setSelectionState(items[cursor], true);
+        this._setSelectionState(items[cursor], scroll);
       }
 
       return this;
@@ -784,18 +825,14 @@ JX.install('DiffChangesetList', {
       var changeset_list = this;
       var changeset = this.getChangesetForNode(node);
 
-      var menu = new JX.PHUIXDropdownMenu(button);
+      var menu = new JX.PHUIXDropdownMenu(button)
+        .setWidth(240);
       var list = new JX.PHUIXActionListView();
 
       var add_link = function(icon, name, href, local) {
-        if (!href) {
-          return;
-        }
-
         var link = new JX.PHUIXActionView()
           .setIcon(icon)
           .setName(name)
-          .setHref(href)
           .setHandler(function(e) {
             if (local) {
               window.location.assign(href);
@@ -806,25 +843,36 @@ JX.install('DiffChangesetList', {
             e.prevent();
           });
 
+        if (href) {
+          link.setHref(href);
+        } else {
+          link
+            .setDisabled(true)
+            .setUnresponsive(true);
+        }
+
         list.addItem(link);
         return link;
       };
 
+      var visible_item = new JX.PHUIXActionView()
+        .setKeyCommand('h')
+        .setHandler(function(e) {
+          e.prevent();
+          menu.close();
+
+          changeset.select(false);
+          changeset.toggleVisibility();
+        });
+      list.addItem(visible_item);
+
       var reveal_item = new JX.PHUIXActionView()
         .setIcon('fa-eye');
       list.addItem(reveal_item);
 
-      var visible_item = new JX.PHUIXActionView()
+      list.addItem(
-        .setHandler(function(e) {
+        new JX.PHUIXActionView()
-          e.prevent();
+          .setDivider(true));
-          menu.close();
-
-          changeset.toggleVisibility();
-        });
-      list.addItem(visible_item);
-
-      add_link('fa-file-text', pht('Browse in Diffusion'), data.diffusionURI);
-      add_link('fa-file-o', pht('View Standalone'), data.standaloneURI);
 
       var up_item = new JX.PHUIXActionView()
         .setHandler(function(e) {
@@ -901,7 +949,7 @@ JX.install('DiffChangesetList', {
 
       var engine_item = new JX.PHUIXActionView()
         .setIcon('fa-file-image-o')
-        .setName(pht('View As...'))
+        .setName(pht('View As Document Type...'))
         .setHandler(function(e) {
           var params = {
             engine: changeset.getDocumentEngine(),
@@ -918,12 +966,31 @@ JX.install('DiffChangesetList', {
         });
       list.addItem(engine_item);
 
+      list.addItem(
+        new JX.PHUIXActionView()
+          .setDivider(true));
+
+      add_link('fa-external-link', pht('View Standalone'), data.standaloneURI);
+
       add_link('fa-arrow-left', pht('Show Raw File (Left)'), data.leftURI);
       add_link('fa-arrow-right', pht('Show Raw File (Right)'), data.rightURI);
 
+      add_link(
+        'fa-folder-open-o',
+        pht('Show Directory in Repository'),
+        changeset.getShowDirectoryURI())
+        .setKeyCommand('D');
+
+      add_link(
+        'fa-file-text-o',
+        pht('Show Path in Repository'),
+        changeset.getShowPathURI())
+        .setKeyCommand('d');
+
       var editor_uri = changeset.getEditorURI();
       if (editor_uri !== null) {
-        add_link('fa-pencil', pht('Open in Editor'), editor_uri, true);
+        add_link('fa-i-cursor', pht('Open in Editor'), editor_uri, true)
+          .setKeyCommand('\\');
       } else {
         var configure_uri = changeset.getEditorConfigureURI();
         if (configure_uri !== null) {
@@ -943,7 +1010,7 @@ JX.install('DiffChangesetList', {
           reveal_item
             .setDisabled(false)
             .setName(pht('Show All Context'))
-            .setIcon('fa-file-o')
+            .setIcon('fa-arrows-v')
             .setHandler(function(e) {
               changeset.loadAllContext();
               e.prevent();
@@ -952,9 +1019,10 @@ JX.install('DiffChangesetList', {
         } else {
           reveal_item
             .setDisabled(true)
+            .setUnresponsive(true)
             .setIcon('fa-file')
             .setName(pht('All Context Shown'))
-            .setHandler(function(e) { e.prevent(); });
+            .setHref(null);
         }
 
         encoding_item.setDisabled(!changeset.isLoaded());
@@ -965,11 +1033,11 @@ JX.install('DiffChangesetList', {
           if (changeset.getRendererKey() == '2up') {
             up_item
               .setIcon('fa-list-alt')
-              .setName(pht('View Unified'));
+              .setName(pht('View Unified Diff'));
           } else {
             up_item
-              .setIcon('fa-files-o')
+              .setIcon('fa-columns')
-              .setName(pht('View Side-by-Side'));
+              .setName(pht('View Side-by-Side Diff'));
           }
         } else {
           up_item

webroot/rsrc/js/phuix/PHUIXActionView.js

@@ -18,6 +18,8 @@ JX.install('PHUIXActionView', {
     _handler: null,
     _selected: false,
     _divider: false,
+    _keyCommand: null,
+    _unresponsive: null,
 
     _iconNode: null,
     _nameNode: null,
@@ -34,6 +36,12 @@ JX.install('PHUIXActionView', {
       return this;
     },
 
+    setUnresponsive: function(unresponsive) {
+      this._unresponsive = unresponsive;
+      this._buildNameNode(true);
+      return this;
+    },
+
     getDisabled: function() {
       return this._disabled;
     },
@@ -75,6 +83,7 @@ JX.install('PHUIXActionView', {
     setHandler: function(handler) {
       this._handler = handler;
       this._buildNameNode(true);
+      this._rebuildClasses();
       return this;
     },
 
@@ -93,6 +102,19 @@ JX.install('PHUIXActionView', {
     setHref: function(href) {
       this._href = href;
       this._buildNameNode(true);
+      this._rebuildClasses();
+      return this;
+    },
+
+    setKeyCommand: function(command) {
+      this._keyCommand = command;
+
+      var key_node = this._buildKeyCommandNode();
+      JX.DOM.setContent(key_node, this._keyCommand);
+
+      var node = this.getNode();
+      JX.DOM.alterClass(node, 'has-key-command', !!this._keyCommand);
+
       return this;
     },
 
@@ -100,17 +122,14 @@ JX.install('PHUIXActionView', {
       if (!this._node) {
         var classes = ['phabricator-action-view'];
 
-        if (this._href || this._handler) {
-          classes.push('phabricator-action-view-href');
-        }
-
         if (this._icon) {
           classes.push('action-has-icon');
         }
 
         var content = [
           this._buildIconNode(),
-          this._buildNameNode()
+          this._buildNameNode(),
+          this._buildKeyCommandNode(),
         ];
 
         var attr = {
@@ -119,11 +138,20 @@ JX.install('PHUIXActionView', {
         this._node = JX.$N('li', attr, content);
 
         JX.Stratcom.addSigil(this._node, 'phuix-action-view');
+
+        this._rebuildClasses();
       }
 
       return this._node;
     },
 
+    _rebuildClasses: function() {
+      var node = this.getNode();
+
+      var is_href = !!(this._href || this._handler);
+      JX.DOM.alterClass(node, 'phabricator-action-view-href', is_href);
+    },
+
     _buildIconNode: function(dirty) {
       if (!this._iconNode || dirty) {
         var attr = {
@@ -155,6 +183,17 @@ JX.install('PHUIXActionView', {
       return this._iconNode;
     },
 
+    _buildKeyCommandNode: function() {
+      if (!this._keyCommandNode) {
+        var attrs = {
+          className: 'keyboard-shortcut-key'
+        };
+
+        this._keyCommandNode = JX.$N('div', attrs);
+      }
+      return this._keyCommandNode;
+    },
+
     _buildNameNode: function(dirty) {
       if (!this._nameNode || dirty) {
         var attr = {
@@ -162,12 +201,11 @@ JX.install('PHUIXActionView', {
         };
 
         var href = this._href;
-        if (!href && this._handler) {
+        if (!href && this._handler && !this._unresponsive) {
           href = '#';
         }
         if (href) {
           attr.href = href;
-
         }
 
         var tag = href ? 'a' : 'span';
@@ -185,6 +223,11 @@ JX.install('PHUIXActionView', {
     },
 
     _onclick: function(e) {
+      if (this._unresponsive) {
+        e.prevent();
+        return;
+      }
+
       if (this._handler) {
         this._handler(e);
       }