Allow keyboard navigation between individual changes

Summary:
Permit "j" and "k" to cycle through individual changeblocks, similar to how this
feature works in ReviewBoard. This still needs a bunch of refinement but it's
getting closer to being useful.

Also moved reticle underneath the table so you can click links through it (derp
derp).

Test Plan:
Used "j" and "k" to cycle through individual changes.

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: moskov, aran, epriestley
Differential Revision: 426

src/__celerity_resource_map__.php

@@ -63,7 +63,7 @@ celerity_register_resource_map(array(
   ),
   'aphront-headsup-action-list-view-css' =>
   array(
-    'uri' => '/res/b7c6bbc2/rsrc/css/aphront/headsup-action-list-view.css',
+    'uri' => '/res/c0ef93b6/rsrc/css/aphront/headsup-action-list-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -145,7 +145,7 @@ celerity_register_resource_map(array(
   ),
   'differential-changeset-view-css' =>
   array(
-    'uri' => '/res/2a59525c/rsrc/css/application/differential/changeset-view.css',
+    'uri' => '/res/d2923406/rsrc/css/application/differential/changeset-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -451,7 +451,7 @@ celerity_register_resource_map(array(
   ),
   'javelin-behavior-differential-keyboard-navigation' =>
   array(
-    'uri' => '/res/dfa0f979/rsrc/js/application/differential/behavior-keyboard-nav.js',
+    'uri' => '/res/08ad535c/rsrc/js/application/differential/behavior-keyboard-nav.js',
     'type' => 'js',
     'requires' =>
     array(
@@ -1024,7 +1024,7 @@ celerity_register_resource_map(array(
   ),
   'phabricator-keyboard-shortcut-manager' =>
   array(
-    'uri' => '/res/14f11fd3/rsrc/js/application/core/KeyboardShortcutManager.js',
+    'uri' => '/res/0454fd16/rsrc/js/application/core/KeyboardShortcutManager.js',
     'type' => 'js',
     'requires' =>
     array(
@@ -1038,7 +1038,7 @@ celerity_register_resource_map(array(
   ),
   'phabricator-object-selector-css' =>
   array(
-    'uri' => '/res/62a8fd92/rsrc/css/application/objectselector/object-selector.css',
+    'uri' => '/res/608461d2/rsrc/css/application/objectselector/object-selector.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -1078,7 +1078,7 @@ celerity_register_resource_map(array(
   ),
   'phabricator-standard-page-view' =>
   array(
-    'uri' => '/res/453bd261/rsrc/css/application/base/standard-page-view.css',
+    'uri' => '/res/f0022c27/rsrc/css/application/base/standard-page-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -1132,40 +1132,7 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/2892314d/typeahead.pkg.js',
       'type' => 'js',
     ),
-    '861d7db0' =>
+    'a452c449' =>
-    array (
-      'name' => 'workflow.pkg.js',
-      'symbols' =>
-      array (
-        0 => 'javelin-mask',
-        1 => 'javelin-workflow',
-        2 => 'javelin-behavior-workflow',
-        3 => 'javelin-behavior-aphront-form-disable-on-submit',
-        4 => 'phabricator-keyboard-shortcut-manager',
-        5 => 'phabricator-keyboard-shortcut',
-        6 => 'javelin-behavior-phabricator-keyboard-shortcuts',
-      ),
-      'uri' => '/res/pkg/861d7db0/workflow.pkg.js',
-      'type' => 'js',
-    ),
-    '8f3a55d1' =>
-    array (
-      'name' => 'differential.pkg.css',
-      'symbols' =>
-      array (
-        0 => 'differential-core-view-css',
-        1 => 'differential-changeset-view-css',
-        2 => 'differential-revision-detail-css',
-        3 => 'differential-revision-history-css',
-        4 => 'differential-table-of-contents-css',
-        5 => 'differential-revision-comment-css',
-        6 => 'differential-revision-add-comment-css',
-        7 => 'differential-revision-comment-list-css',
-      ),
-      'uri' => '/res/pkg/8f3a55d1/differential.pkg.css',
-      'type' => 'css',
-    ),
-    'acfddc38' =>
     array (
       'name' => 'core.pkg.css',
       'symbols' =>
@@ -1186,7 +1153,24 @@ celerity_register_resource_map(array(
         13 => 'phabricator-remarkup-css',
         14 => 'syntax-highlighting-css',
       ),
-      'uri' => '/res/pkg/acfddc38/core.pkg.css',
+      'uri' => '/res/pkg/a452c449/core.pkg.css',
+      'type' => 'css',
+    ),
+    'c9226a80' =>
+    array (
+      'name' => 'differential.pkg.css',
+      'symbols' =>
+      array (
+        0 => 'differential-core-view-css',
+        1 => 'differential-changeset-view-css',
+        2 => 'differential-revision-detail-css',
+        3 => 'differential-revision-history-css',
+        4 => 'differential-table-of-contents-css',
+        5 => 'differential-revision-comment-css',
+        6 => 'differential-revision-add-comment-css',
+        7 => 'differential-revision-comment-list-css',
+      ),
+      'uri' => '/res/pkg/c9226a80/differential.pkg.css',
       'type' => 'css',
     ),
     'da416e1c' =>
@@ -1222,42 +1206,58 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/db95a6d0/javelin.pkg.js',
       'type' => 'js',
     ),
+    'df91920b' =>
+    array (
+      'name' => 'workflow.pkg.js',
+      'symbols' =>
+      array (
+        0 => 'javelin-mask',
+        1 => 'javelin-workflow',
+        2 => 'javelin-behavior-workflow',
+        3 => 'javelin-behavior-aphront-form-disable-on-submit',
+        4 => 'phabricator-keyboard-shortcut-manager',
+        5 => 'phabricator-keyboard-shortcut',
+        6 => 'javelin-behavior-phabricator-keyboard-shortcuts',
+      ),
+      'uri' => '/res/pkg/df91920b/workflow.pkg.js',
+      'type' => 'js',
+    ),
   ),
   'reverse' =>
   array (
-    'aphront-crumbs-view-css' => 'acfddc38',
+    'aphront-crumbs-view-css' => 'a452c449',
-    'aphront-dialog-view-css' => 'acfddc38',
+    'aphront-dialog-view-css' => 'a452c449',
-    'aphront-form-view-css' => 'acfddc38',
+    'aphront-form-view-css' => 'a452c449',
-    'aphront-list-filter-view-css' => 'acfddc38',
+    'aphront-list-filter-view-css' => 'a452c449',
-    'aphront-panel-view-css' => 'acfddc38',
+    'aphront-panel-view-css' => 'a452c449',
-    'aphront-side-nav-view-css' => 'acfddc38',
+    'aphront-side-nav-view-css' => 'a452c449',
-    'aphront-table-view-css' => 'acfddc38',
+    'aphront-table-view-css' => 'a452c449',
-    'aphront-tokenizer-control-css' => 'acfddc38',
+    'aphront-tokenizer-control-css' => 'a452c449',
-    'aphront-typeahead-control-css' => 'acfddc38',
+    'aphront-typeahead-control-css' => 'a452c449',
-    'differential-changeset-view-css' => '8f3a55d1',
+    'differential-changeset-view-css' => 'c9226a80',
-    'differential-core-view-css' => '8f3a55d1',
+    'differential-core-view-css' => 'c9226a80',
-    'differential-revision-add-comment-css' => '8f3a55d1',
+    'differential-revision-add-comment-css' => 'c9226a80',
-    'differential-revision-comment-css' => '8f3a55d1',
+    'differential-revision-comment-css' => 'c9226a80',
-    'differential-revision-comment-list-css' => '8f3a55d1',
+    'differential-revision-comment-list-css' => 'c9226a80',
-    'differential-revision-detail-css' => '8f3a55d1',
+    'differential-revision-detail-css' => 'c9226a80',
-    'differential-revision-history-css' => '8f3a55d1',
+    'differential-revision-history-css' => 'c9226a80',
-    'differential-table-of-contents-css' => '8f3a55d1',
+    'differential-table-of-contents-css' => 'c9226a80',
     'diffusion-commit-view-css' => '03ef179e',
     'javelin-behavior' => 'db95a6d0',
     'javelin-behavior-aphront-basic-tokenizer' => '2892314d',
-    'javelin-behavior-aphront-form-disable-on-submit' => '861d7db0',
+    'javelin-behavior-aphront-form-disable-on-submit' => 'df91920b',
     'javelin-behavior-differential-diff-radios' => 'da416e1c',
     'javelin-behavior-differential-edit-inline-comments' => 'da416e1c',
     'javelin-behavior-differential-feedback-preview' => 'da416e1c',
     'javelin-behavior-differential-populate' => 'da416e1c',
     'javelin-behavior-differential-show-more' => 'da416e1c',
-    'javelin-behavior-phabricator-keyboard-shortcuts' => '861d7db0',
+    'javelin-behavior-phabricator-keyboard-shortcuts' => 'df91920b',
-    'javelin-behavior-workflow' => '861d7db0',
+    'javelin-behavior-workflow' => 'df91920b',
     'javelin-dom' => 'db95a6d0',
     'javelin-event' => 'db95a6d0',
     'javelin-install' => 'db95a6d0',
     'javelin-json' => 'db95a6d0',
-    'javelin-mask' => '861d7db0',
+    'javelin-mask' => 'df91920b',
     'javelin-request' => 'db95a6d0',
     'javelin-stratcom' => 'db95a6d0',
     'javelin-tokenizer' => '2892314d',
@@ -1269,14 +1269,14 @@ celerity_register_resource_map(array(
     'javelin-uri' => 'db95a6d0',
     'javelin-util' => 'db95a6d0',
     'javelin-vector' => 'db95a6d0',
-    'javelin-workflow' => '861d7db0',
+    'javelin-workflow' => 'df91920b',
-    'phabricator-core-buttons-css' => 'acfddc38',
+    'phabricator-core-buttons-css' => 'a452c449',
-    'phabricator-core-css' => 'acfddc38',
+    'phabricator-core-css' => 'a452c449',
-    'phabricator-directory-css' => 'acfddc38',
+    'phabricator-directory-css' => 'a452c449',
-    'phabricator-keyboard-shortcut' => '861d7db0',
+    'phabricator-keyboard-shortcut' => 'df91920b',
-    'phabricator-keyboard-shortcut-manager' => '861d7db0',
+    'phabricator-keyboard-shortcut-manager' => 'df91920b',
-    'phabricator-remarkup-css' => 'acfddc38',
+    'phabricator-remarkup-css' => 'a452c449',
-    'phabricator-standard-page-view' => 'acfddc38',
+    'phabricator-standard-page-view' => 'a452c449',
-    'syntax-highlighting-css' => 'acfddc38',
+    'syntax-highlighting-css' => 'a452c449',
   ),
 ));

webroot/rsrc/css/application/base/standard-page-view.css

@@ -134,13 +134,10 @@ td.phabricator-login-details {
 }
 
 .keyboard-focus-focus-reticle {
-  z-index: 32;
+  z-index: 1;
-  border: 1px solid #999999;
+  background: #fffff3;
-
   position: absolute;
-  -webkit-box-shadow: 0 0 3px #000;
+  border: 1px solid #eeeed3;
-  -mox-box-shadow:    0 0 3px #000;
-  box-shadow:         0 0 3px #000;
 }
 
 .keyboard-shortcuts-available {

webroot/rsrc/css/application/differential/changeset-view.css

@@ -2,8 +2,13 @@
  * @provides differential-changeset-view-css
  */
 
+.differential-changeset {
+  z-index: 2;
+  position: relative;
+}
+
 .differential-diff {
-  background:         #ffffff;
+  background:         transparent;
   font-family:        "Menlo", "Consolas", "Monaco", monospace;
   font-size:          10px;
   width:              100%;

webroot/rsrc/js/application/core/KeyboardShortcutManager.js

@@ -54,21 +54,40 @@ JX.install('KeyboardShortcutManager', {
      * Scroll an element into view.
      */
     scrollTo : function(node) {
-      window.scrollTo(0, JX.$V(node).y - 40);
+      window.scrollTo(0, JX.$V(node).y - 60);
     },
 
     /**
      * Move the keyboard shortcut focus to an element.
+     *
+     * @param Node Node to focus, or pass null to clear the focus.
+     * @param Node To focus multiple nodes (like rows in a table), specify the
+     *             top-left node as the first parameter and the bottom-right
+     *             node as the focus extension.
+     * @return void
      */
-    focusOn : function(node) {
+    focusOn : function(node, extended_node) {
       this._clearReticle();
 
+      if (!node) {
+        return;
+      }
+
       var r = JX.$N('div', {className : 'keyboard-focus-focus-reticle'});
 
-      // Outset the reticle 8 pixels away from the element, so there's some
+      extended_node = extended_node || node;
+
+      // Outset the reticle some pixels away from the element, so there's some
       // space between the focused element and the outline.
-      JX.Vector.getPos(node).add(-8, -8).setPos(r);
+      var p  = JX.Vector.getPos(node);
-      JX.Vector.getDim(node).add(16, 16).setDim(r);
+      p.add(-6, -6).setPos(r);
+      // Compute the size we need to extend to the full extent of the focused
+      // nodes.
+      JX.Vector.getPos(extended_node)
+        .add(-p.x, -p.y)
+        .add(JX.Vector.getDim(extended_node))
+        .add(12, 12)
+        .setDim(r);
       document.body.appendChild(r);
 
       this._focusReticle = r;

webroot/rsrc/js/application/differential/behavior-keyboard-nav.js

@@ -7,7 +7,8 @@
 
 JX.behavior('differential-keyboard-navigation', function(config) {
 
-  var cursor = null;
+  var cursor = -1;
+  var cursor_block = null;
   var changesets;
 
   function init() {
@@ -17,19 +18,104 @@ JX.behavior('differential-keyboard-navigation', function(config) {
     changesets = JX.DOM.scry(document.body, 'div', 'differential-changeset');
   }
 
+  function getBlocks(cursor) {
+    // TODO: This might not be terribly fast; we can't currently memoize it
+    // because it can change as ajax requests come in (e.g., content loads).
+
+    var rows = JX.DOM.scry(changesets[cursor], 'tr');
+    var blocks = [[changesets[cursor], changesets[cursor]]];
+    var start = null;
+    var type;
+    for (var ii = 0; ii < rows.length; ii++) {
+      type = getRowType(rows[ii]);
+      if (type == 'ignore') {
+        continue;
+      }
+      if (!type && start) {
+        blocks.push([start, rows[ii - 1]]);
+        start = null;
+      } else if (type && !start) {
+        start = rows[ii];
+      }
+    }
+    if (start) {
+      blocks.push([start, rows[ii - 1]]);
+    }
+
+    return blocks;
+  }
+
+  function getRowType(row) {
+    // NOTE: Being somewhat over-general here to allow other types of objects
+    // to be easily focused in the future (inline comments, 'show more..').
+
+    if (row.className.indexOf('inline') !== -1) {
+      return 'ignore';
+    }
+
+    var cells = JX.DOM.scry(row, 'td');
+
+    for (var ii = 0; ii < cells.length; ii++) {
+      // NOTE: The semantic use of classnames here is for performance; don't
+      // emulate this elsewhere since it's super terrible.
+      if (cells[ii].className.indexOf('old') !== -1 ||
+          cells[ii].className.indexOf('new') !== -1) {
+        return 'change';
+      }
+    }
+
+    return null;
+  }
+
   function jump(manager, delta) {
     init();
 
-    if (cursor === null) {
+    if (cursor < 0) {
-      cursor = -1;
+      if (delta < 0) {
+        // If the user goes "back" without a selection, just reject the action.
+        return;
+      } else {
+        cursor = 0;
+      }
     }
 
-    cursor = (cursor + changesets.length + delta) % changesets.length;
+    var selected;
+    var extent;
+    while (true) {
+      var blocks = getBlocks(cursor);
+      var focus;
+      if (delta < 0) {
+        focus = blocks.length;
+      } else {
+        focus = -1;
+      }
 
-    var selected = changesets[cursor];
+      for (var ii = 0; ii < blocks.length; ii++) {
+        if (blocks[ii][0] == cursor_block) {
+          focus = ii;
+          break;
+        }
+      }
+
+      focus += delta;
+
+      if (blocks[focus]) {
+        selected = blocks[focus][0];
+        extent = blocks[focus][1];
+        cursor_block = selected;
+        break;
+      } else {
+        var adjusted = (cursor + delta);
+        if (adjusted < 0 || adjusted >= changesets.length) {
+          // Stop cursor movement when the user reaches either end.
+          return;
+        }
+        cursor = adjusted;
+      }
+    }
 
     manager.scrollTo(selected);
-    manager.focusOn(selected);
+    manager.focusOn(selected, extent);
   }
 
   new JX.KeyboardShortcut('j', 'Jump to next change.')