Prevent scrolling when initializing the file list
Provide an optional parameter to the cursor manager's target setter
methods to prevent scrolling, allowing the cursor to be programmatically
initialized without needlessly scrolling the page.
Because the ScrollBehavior.ALWAYS option was actually non-functional and
was neither tested nor used in the PG code, it has been removed from the
enum and JSDocs.
Bug: Issue 5273
Change-Id: Ia299dca1ca178a8a4d60b64932b1f291e11b79de
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 f3384f2..a1f88b6 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
@@ -589,7 +589,7 @@
var files = Polymer.dom(this.root).querySelectorAll('.file-row');
this.$.fileCursor.stops = files;
- this.$.fileCursor.setCursorAtIndex(this.selectedIndex);
+ this.$.fileCursor.setCursorAtIndex(this.selectedIndex, true);
}.bind(this), 1);
},
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 81f5186..704cef2 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
@@ -15,7 +15,6 @@
'use strict';
var ScrollBehavior = {
- ALWAYS: 'always',
NEVER: 'never',
KEEP_VISIBLE: 'keep-visible',
};
@@ -55,7 +54,7 @@
},
/**
- * The scroll behavior for the cursor. Values are 'never', 'always' and
+ * The scroll behavior for the cursor. Values are 'never' and
* 'keep-visible'. 'keep-visible' will only scroll if the cursor is beyond
* the viewport.
*/
@@ -80,12 +79,22 @@
/**
* Set the cursor to an arbitrary element.
* @param {DOMElement} element
+ * @param {boolean} opt_noScroll prevent any potential scrolling in response
+ * setting the cursor.
*/
- setCursor: function(element) {
+ setCursor: function(element, opt_noScroll) {
+ var behavior;
+ if (opt_noScroll) {
+ behavior = this.scrollBehavior;
+ this.scrollBehavior = ScrollBehavior.NEVER;
+ }
+
this.unsetCursor();
this.target = element;
this._updateIndex();
this._decorateTarget();
+
+ if (opt_noScroll) { this.scrollBehavior = behavior; }
},
unsetCursor: function() {
@@ -108,8 +117,8 @@
}
},
- setCursorAtIndex: function(index) {
- this.setCursor(this.stops[index]);
+ setCursorAtIndex: function(index, opt_noScroll) {
+ this.setCursor(this.stops[index], opt_noScroll);
},
/**
@@ -197,11 +206,10 @@
}
},
- _scrollToTarget: function() {
- if (!this.target || this.scrollBehavior === ScrollBehavior.NEVER) {
- return;
- }
-
+ /**
+ * @return {boolean}
+ */
+ _targetIsVisible: function() {
// Calculate where the element is relative to the window.
var top = this.target.offsetTop;
for (var offsetParent = this.target.offsetParent;
@@ -210,9 +218,17 @@
top += offsetParent.offsetTop;
}
- if (this.scrollBehavior === ScrollBehavior.KEEP_VISIBLE &&
+ return this.scrollBehavior === ScrollBehavior.KEEP_VISIBLE &&
top > window.pageYOffset &&
- top < window.pageYOffset + window.innerHeight) { return; }
+ top < window.pageYOffset + window.innerHeight;
+ },
+
+ _scrollToTarget: function() {
+ if (!this.target ||
+ this.scrollBehavior === ScrollBehavior.NEVER ||
+ this._targetIsVisible()) {
+ 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
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html
index 1ad014d..884c01d 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html
@@ -26,9 +26,7 @@
<test-fixture id="basic">
<template>
- <gr-cursor-manager
- cursor-stop-selector="li"
- cursor-target-class="targeted"></gr-cursor-manager>
+ <gr-cursor-manager cursor-target-class="targeted"></gr-cursor-manager>
<ul>
<li>A</li>
<li>B</li>
@@ -40,15 +38,21 @@
<script>
suite('gr-cursor-manager tests', function() {
+ var sandbox;
var element;
var list;
setup(function() {
+ sandbox = sinon.sandbox.create();
var fixtureElements = fixture('basic');
element = fixtureElements[0];
list = fixtureElements[1];
});
+ teardown(function() {
+ sandbox.restore();
+ });
+
test('core cursor functionality', function() {
// The element is initialized into the proper state.
assert.isArray(element.stops);
@@ -120,5 +124,18 @@
assert.isNotOk(element.target);
assert.equal(element.index, -1);
});
+
+ test('opt_noScroll', function() {
+ sandbox.stub(element, '_targetIsVisible', function() { return false; });
+ var scrollStub = sandbox.stub(window, 'scrollTo');
+ element.stops = list.querySelectorAll('li');
+ element.scrollBehavior = 'keep-visible';
+
+ element.setCursorAtIndex(1, true);
+ assert.isFalse(scrollStub.called);
+
+ element.setCursorAtIndex(2);
+ assert.isTrue(scrollStub.called);
+ });
});
</script>
