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
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 132766b..976cf9b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -178,7 +178,9 @@
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>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 5219b4e..3257469 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/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) {
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
index 0e4d8f0..5b70769 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
@@ -161,42 +161,49 @@
<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 @@
</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 @@
<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>
@@ -259,14 +262,6 @@
<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>
<span class="key">←</span>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
index f58916e..5a41709 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
@@ -21,8 +21,9 @@
<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>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index 77b5b57..3f71892 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -53,6 +53,11 @@
return [];
},
},
+
+ foldOffsetTop: {
+ type: Number,
+ value: 0,
+ },
},
observers: [
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 7595bb2..e4603b8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -43,13 +43,15 @@
.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 @@
.canComment .lineNum[data-value] {
cursor: pointer;
}
- .canComment .lineNum[data-value]:before {
- text-decoration: underline;
- }
.canComment .lineNum[data-value]:hover:before {
background-color: #ccc;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
index b0a349c..e1990c4 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
+++ b/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
