Merge "Prevent scrolling when initializing the file list"
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>
