Shortcut for moving cursor to visible code
Page-up/page-down keys don't move the cursor. The subsequent up/down
keys scroll window back to the cursor position.
This behavior makes it harder to navigate through a diff view via
keyboard. To make behavior more consistent, the shortcut is added to
move cursor to currently visible code.
Bug: Issue 12221
Change-Id: I4f01ed3d3a5db983a8f85ec7a64f80280048c984
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
index 8a1ed87..495f8ab 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
@@ -148,6 +148,7 @@
NEXT_LINE: 'NEXT_LINE',
PREV_LINE: 'PREV_LINE',
+ VISIBLE_LINE: 'VISIBLE_LINE',
NEXT_CHUNK: 'NEXT_CHUNK',
PREV_CHUNK: 'PREV_CHUNK',
EXPAND_ALL_DIFF_CONTEXT: 'EXPAND_ALL_DIFF_CONTEXT',
@@ -237,6 +238,8 @@
_describe(Shortcut.NEXT_LINE, ShortcutSection.DIFFS, 'Go to next line');
_describe(Shortcut.PREV_LINE, ShortcutSection.DIFFS, 'Go to previous line');
+ _describe(Shortcut.VISIBLE_LINE, ShortcutSection.DIFFS,
+ 'Move cursor to currently visible code');
_describe(Shortcut.NEXT_CHUNK, ShortcutSection.DIFFS,
'Go to next diff chunk');
_describe(Shortcut.PREV_CHUNK, ShortcutSection.DIFFS,
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 e8d629c..87152d8 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
@@ -180,6 +180,15 @@
}
}
+ moveToVisibleArea() {
+ if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
+ this.$.cursorManager.moveToVisibleArea(
+ this._rowHasSide.bind(this));
+ } else {
+ this.$.cursorManager.moveToVisibleArea();
+ }
+ }
+
moveToNextChunk(opt_clipToTop) {
this.$.cursorManager.next(this._isFirstRowOfChunk.bind(this),
target => target.parentNode.scrollHeight, opt_clipToTop);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index d49b562..d7eaabf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -229,6 +229,7 @@
[this.Shortcut.RIGHT_PANE]: '_handleRightPane',
[this.Shortcut.NEXT_LINE]: '_handleNextLineOrFileWithComments',
[this.Shortcut.PREV_LINE]: '_handlePrevLineOrFileWithComments',
+ [this.Shortcut.VISIBLE_LINE]: '_handleVisibleLine',
[this.Shortcut.NEXT_FILE_WITH_COMMENTS]:
'_handleNextLineOrFileWithComments',
[this.Shortcut.PREV_FILE_WITH_COMMENTS]:
@@ -382,6 +383,13 @@
this.$.cursor.moveUp();
}
+ _handleVisibleLine(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this.$.cursor.moveToVisibleArea();
+ }
+
_onOpenFixPreview(e) {
this.$.applyFixDialog.open(e);
}
diff --git a/polygerrit-ui/app/elements/gr-app-element.js b/polygerrit-ui/app/elements/gr-app-element.js
index d39ba58..c1e317a 100644
--- a/polygerrit-ui/app/elements/gr-app-element.js
+++ b/polygerrit-ui/app/elements/gr-app-element.js
@@ -240,6 +240,10 @@
this.Shortcut.NEXT_LINE, 'j', 'down');
this.bindShortcut(
this.Shortcut.PREV_LINE, 'k', 'up');
+ if (this._isCursorManagerSupportMoveToVisibleLine()) {
+ this.bindShortcut(
+ this.Shortcut.VISIBLE_LINE, '.');
+ }
this.bindShortcut(
this.Shortcut.NEXT_CHUNK, 'n');
this.bindShortcut(
@@ -303,6 +307,14 @@
this.Shortcut.SEARCH, '/');
}
+ _isCursorManagerSupportMoveToVisibleLine() {
+ // This method is a copy-paste from the
+ // method _isIntersectionObserverSupported of gr-cursor-manager.js
+ // It is better share this method with gr-cursor-manager,
+ // but doing it require a lot if changes instead of 1-line copied code
+ return 'IntersectionObserver' in window;
+ }
+
_accountChanged(account) {
if (!account) { return; }
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 37c7147..4f98e88 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
@@ -131,6 +131,75 @@
}
/**
+ * Move the cursor to the row which is the closest to the viewport center
+ * in vertical direction.
+ * The method uses IntersectionObservers API. If browser
+ * doesn't support this API the method does nothing
+ *
+ * @param {!Function=} opt_condition Optional condition. If a condition
+ * is passed only stops which meet conditions are taken into account.
+ */
+ moveToVisibleArea(opt_condition) {
+ if (!this.stops || !this._isIntersectionObserverSupported()) {
+ return;
+ }
+ const filteredStops = opt_condition ? this.stops.filter(opt_condition)
+ : this.stops;
+ const dims = this._getWindowDims();
+ const windowCenter =
+ Math.round((dims.innerHeight + this.scrollTopMargin) / 2);
+
+ let closestToTheCenter = null;
+ let minDistanceToCenter = null;
+ let unobservedCount = filteredStops.length;
+
+ const observer = new IntersectionObserver(entries => {
+ // This callback is called for the first time immediately.
+ // Typically it gets all observed stops at once, but
+ // sometimes can get them in several chunks.
+ entries.forEach(entry => {
+ observer.unobserve(entry.target);
+
+ // In Edge it is recommended to use intersectionRatio instead of
+ // isIntersecting.
+ const isInsideViewport =
+ entry.isIntersecting || entry.intersectionRatio > 0;
+ if (!isInsideViewport) {
+ return;
+ }
+ const center = entry.boundingClientRect.top + Math.round(
+ entry.boundingClientRect.height / 2);
+ const distanceToWindowCenter = Math.abs(center - windowCenter);
+ if (minDistanceToCenter === null ||
+ distanceToWindowCenter < minDistanceToCenter) {
+ closestToTheCenter = entry.target;
+ minDistanceToCenter = distanceToWindowCenter;
+ }
+ });
+ unobservedCount -= entries.length;
+ if (unobservedCount == 0 && closestToTheCenter) {
+ // set cursor when all stops were observed.
+ // In most cases the target is visible, so scroll is not
+ // needed. But in rare cases the target can become invisible
+ // at this point (due to some scrolling in window).
+ // To avoid jumps set noScroll options.
+ this.setCursor(closestToTheCenter, true);
+ }
+ });
+ filteredStops.forEach(stop => {
+ observer.observe(stop);
+ });
+ }
+
+ _isIntersectionObserverSupported() {
+ // The copy of this method exists in gr-app-element.js under the
+ // name _isCursorManagerSupportMoveToVisibleLine
+ // If you update this method, you must update gr-app-element.js
+ // as well.
+ return 'IntersectionObserver' in window;
+ }
+
+ /**
* Set the cursor to an arbitrary element.
*
* @param {!HTMLElement} element
