Tidies keyboard shortcut dialogs and updated cursor styles

Reorganizes the keyboard shortcut list for change view with smaller
groups of shortcuts. Also tidies up the dialog for the diff view a bit.

Updates the style for the selected line in the diff view and fixes the
focus when the diff expand/collapse buttons are activated. The scroll
behavior is changed slightly.

Change-Id: I8e520f725f4706aaad6e8bd8b99dd0e274d2f830

polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html

@@ -178,7 +178,9 @@ limitations under the License.
           view-mode="[[_userPrefs.diff_view]]"></gr-diff>
     </template>
     <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
-    <gr-diff-cursor id="cursor"></gr-diff-cursor>
+    <gr-diff-cursor
+        id="cursor"
+        fold-offset-top="[[topMargin]]"></gr-diff-cursor>
   </template>
   <script src="gr-file-list.js"></script>
 </dom-module>

polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js

@@ -127,20 +127,26 @@
       }
     },
 
-    _expandAllDiffs: function() {
+    _expandAllDiffs: function(e) {
       this._showInlineDiffs = true;
       this._forEachDiff(function(diff) {
         diff.hidden = false;
         diff.reload();
       });
+      if (e && e.target) {
+        e.target.blur();
+      }
     },
 
-    _collapseAllDiffs: function() {
+    _collapseAllDiffs: function(e) {
       this._showInlineDiffs = false;
       this._forEachDiff(function(diff) {
         diff.hidden = true;
       });
       this.$.cursor.handleDiffUpdate();
+      if (e && e.target) {
+        e.target.blur();
+      }
     },
 
     _computeCommentsString: function(comments, patchNum, path) {

polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html

@@ -161,42 +161,49 @@ limitations under the License.
             <td></td><td class="header">File list</td>
           </tr>
           <tr>
-            <td><span class="key">j</span></td>
-            <td>Select next item</td>
+            <td><span class="key">j</span> or <span class="key">↓</span></td>
+            <td>Select next file</td>
           </tr>
           <tr>
-            <td><span class="key">k</span></td>
-            <td>Select previous item</td>
+            <td><span class="key">k</span> or <span class="key">↑</span></td>
+            <td>Select previous file</td>
           </tr>
           <tr>
-            <td><span class="key">↓</span></td>
-            <td>Select next item</td>
+            <td><span class="key">Enter</span> or <span class="key">o</span></td>
+            <td>Show selected file</td>
           </tr>
           <tr>
-            <td><span class="key">↑</span></td>
-            <td>Select previous item</td>
+            <td></td><td class="header">Diffs</td>
+          </tr>
+          <tr>
+            <td><span class="key">j</span> or <span class="key">↓</span></td>
+            <td>Go to next line</td>
+          </tr>
+          <tr>
+            <td><span class="key">k</span> or <span class="key">↑</span></td>
+            <td>Go to previous line</td>
           </tr>
           <tr>
             <td><span class="key">n</span></td>
-            <td>Show next diff chunk</td>
+            <td>Go to next diff chunk</td>
           </tr>
           <tr>
             <td><span class="key">p</span></td>
-            <td>Show previous diff chunk</td>
+            <td>Go to previous diff chunk</td>
           </tr>
           <tr>
             <td>
               <span class="key modifier">Shift</span>
               <span class="key">n</span>
             </td>
-            <td>Show next comment thread</td>
+            <td>Go to next comment thread</td>
           </tr>
           <tr>
             <td>
               <span class="key modifier">Shift</span>
               <span class="key">p</span>
             </td>
-            <td>Show previous comment thread</td>
+            <td>Go to previous comment thread</td>
           </tr>
           <tr>
             <td>
@@ -218,10 +225,6 @@ limitations under the License.
             </td>
             <td>Draft new comment</td>
           </tr>
-          <tr>
-            <td><span class="key">Enter</span> or <span class="key">o</span></td>
-            <td>Show selected file</td>
-          </tr>
         </tbody>
         <!-- Diff View -->
         <tbody hidden$="[[!_computeInView(view, 'gr-diff-view')]]" hidden>
@@ -229,11 +232,11 @@ limitations under the License.
             <td></td><td class="header">Actions</td>
           </tr>
           <tr>
-            <td><span class="key">j</span></td>
+            <td><span class="key">j</span> or <span class="key">↓</span></td>
             <td>Show next line</td>
           </tr>
           <tr>
-            <td><span class="key">k</span></td>
+            <td><span class="key">k</span> or <span class="key">↑</span></td>
             <td>Show previous line</td>
           </tr>
           <tr>
@@ -258,14 +261,6 @@ limitations under the License.
             </td>
             <td>Show previous comment thread</td>
           </tr>
-          <tr>
-            <td><span class="key">↓</span></td>
-            <td>Show next line</td>
-          </tr>
-          <tr>
-            <td><span class="key">↑</span></td>
-            <td>Show previous line</td>
-          </tr>
           <tr>
             <td>
               <span class="key modifier">Shift</span>

polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html

@@ -21,8 +21,9 @@ limitations under the License.
   <template>
     <gr-cursor-manager
         id="cursorManager"
-        scroll
+        scroll="keep-visible"
         cursor-target-class="target-row"
+        fold-offset-top="[[foldOffsetTop]]"
         target="{{diffRow}}"></gr-cursor-manager>
   </template>
   <script src="gr-diff-cursor.js"></script>

polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js

@@ -53,6 +53,11 @@
           return [];
         },
       },
+
+      foldOffsetTop: {
+        type: Number,
+        value: 0,
+      },
     },
 
     observers: [

polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html

@@ -43,13 +43,15 @@ limitations under the License.
       .section {
         background-color: #eee;
       }
-      .diff-row.target-row {
-        outline: .2em solid #ddd;
-      }
       .diff-row.target-row.target-side-left .lineNum.left,
       .diff-row.target-row.target-side-right .lineNum.right,
       .diff-row.target-row.unified .lineNum {
-        background-color: #ccc;
+        background-color: #BBDEFB;
+      }
+      .diff-row.target-row.target-side-left .lineNum.left:before,
+      .diff-row.target-row.target-side-right .lineNum.right:before,
+      .diff-row.target-row.unified .lineNum:before {
+        color: #000;
       }
       .blank,
       .content {
@@ -72,9 +74,6 @@ limitations under the License.
       .canComment .lineNum[data-value] {
         cursor: pointer;
       }
-      .canComment .lineNum[data-value]:before {
-        text-decoration: underline;
-      }
       .canComment .lineNum[data-value]:hover:before {
         background-color: #ccc;
       }

polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js

@@ -14,6 +14,12 @@
 (function() {
   'use strict';
 
+  var ScrollBehavior = {
+    ALWAYS: 'always',
+    NEVER: 'never',
+    KEEP_VISIBLE: 'keep-visible',
+  };
+
   Polymer({
     is: 'gr-cursor-manager',
 
@@ -46,9 +52,24 @@
         type: String,
         value: null,
       },
+
+      /**
+       * The scroll behavior for the cursor. Values are 'never', 'always' and
+       * 'keep-visible'. 'keep-visible' will only scroll if the cursor is beyond
+       * the viewport.
+       */
       scroll: {
-        type: Boolean,
-        value: false,
+        type: String,
+        value: ScrollBehavior.NEVER,
+      },
+
+      /**
+       * When using the 'keep-visible' scroll behavior, set an offset to the top
+       * of the window for what is considered above the upper fold.
+       */
+      foldOffsetTop: {
+        type: Number,
+        value: 0,
       },
     },
 
@@ -181,7 +202,7 @@
     },
 
     _scrollToTarget: function() {
-      if (!this.target || !this.scroll) { return; }
+      if (!this.target || this.scroll === ScrollBehavior.NEVER) { return; }
 
       // Calculate where the element is relative to the window.
       var top = this.target.offsetTop;
@@ -191,6 +212,10 @@
         top += offsetParent.offsetTop;
       }
 
+      if (this.scroll === ScrollBehavior.KEEP_VISIBLE &&
+          top > window.pageYOffset + this.foldOffsetTop &&
+          top < window.pageYOffset + window.innerHeight) { return; }
+
       // Scroll the element to the middle of the window. Dividing by a third
       // instead of half the inner height feels a bit better otherwise the
       // element appears to be below the center of the window even when it