Fix jumping scroll position when diff view loads
Previously, there was an issue where if you started scrolling on a diff view page before everything was loaded, the page would jump back to the top after loading finished. This change temporarily adjusts scroll behavior when scrolling is detected mid-diff load. The scroll behavior is then restored after loading completes. Bug: Issue 4411 Change-Id: I8175d433632fd8710f1353f7ba5635b826151ce0
This commit is contained in:
committed by
Andrew Bonventre
parent
5e4491886b
commit
55a1eec37d
@@ -61,6 +61,12 @@ limitations under the License.
|
||||
Polymer({
|
||||
is: 'gr-diff-builder',
|
||||
|
||||
/**
|
||||
* Fired when the diff begins rendering.
|
||||
*
|
||||
* @event render-start
|
||||
*/
|
||||
|
||||
/**
|
||||
* Fired when the diff is rendered.
|
||||
*
|
||||
@@ -121,6 +127,7 @@ limitations under the License.
|
||||
|
||||
reporting.time(TimingLabel.TOTAL);
|
||||
reporting.time(TimingLabel.CONTENT);
|
||||
this.fire('render-start');
|
||||
return this.$.processor.process(this.diff.content).then(function() {
|
||||
if (this.isImageDiff) {
|
||||
this._builder.renderDiffImages();
|
||||
|
||||
@@ -613,6 +613,16 @@ limitations under the License.
|
||||
assert.strictEqual(sections[0], section[0]);
|
||||
assert.strictEqual(sections[1], section[1]);
|
||||
});
|
||||
|
||||
test('render-start and render are fired', function() {
|
||||
var fireStub = sinon.stub(element, 'fire');
|
||||
element.render({left: [], right: []}, {});
|
||||
flush(function() {
|
||||
assert.isTrue(fireStub.calledWithExactly('render-start'));
|
||||
assert.isTrue(fireStub.calledWithExactly('render'));
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
suite('mock-diff', function() {
|
||||
|
||||
@@ -21,7 +21,7 @@ limitations under the License.
|
||||
<template>
|
||||
<gr-cursor-manager
|
||||
id="cursorManager"
|
||||
scroll="keep-visible"
|
||||
scroll="[[_scrollBehavior]]"
|
||||
cursor-target-class="target-row"
|
||||
target="{{diffRow}}"></gr-cursor-manager>
|
||||
</template>
|
||||
|
||||
@@ -24,6 +24,11 @@
|
||||
UNIFIED: 'UNIFIED_DIFF',
|
||||
};
|
||||
|
||||
var ScrollBehavior = {
|
||||
KEEP_VISIBLE: 'keep-visible',
|
||||
NEVER: 'never',
|
||||
};
|
||||
|
||||
var LEFT_SIDE_CLASS = 'target-side-left';
|
||||
var RIGHT_SIDE_CLASS = 'target-side-right';
|
||||
|
||||
@@ -63,6 +68,18 @@
|
||||
type: Number,
|
||||
value: null,
|
||||
},
|
||||
|
||||
/**
|
||||
* The scroll behavior for the cursor. Values are 'never' and
|
||||
* 'keep-visible'. 'keep-visible' will only scroll if the cursor is beyond
|
||||
* the viewport.
|
||||
*/
|
||||
_scrollBehavior: {
|
||||
type: String,
|
||||
value: ScrollBehavior.KEEP_VISIBLE,
|
||||
},
|
||||
|
||||
_listeningForScroll: Boolean,
|
||||
},
|
||||
|
||||
observers: [
|
||||
@@ -70,6 +87,15 @@
|
||||
'_diffsChanged(diffs.splices)',
|
||||
],
|
||||
|
||||
attached: function() {
|
||||
// Catch when users are scrolling as the view loads.
|
||||
this.listen(window, 'scroll', '_handleWindowScroll');
|
||||
},
|
||||
|
||||
detached: function() {
|
||||
this.unlisten(window, 'scroll', '_handleWindowScroll');
|
||||
},
|
||||
|
||||
moveLeft: function() {
|
||||
this.side = DiffSides.LEFT;
|
||||
if (this._isTargetBlank()) {
|
||||
@@ -169,12 +195,25 @@
|
||||
}
|
||||
},
|
||||
|
||||
_handleWindowScroll: function() {
|
||||
if (this._listeningForScroll) {
|
||||
this._scrollBehavior = ScrollBehavior.NEVER;
|
||||
this._listeningForScroll = false;
|
||||
}
|
||||
},
|
||||
|
||||
handleDiffUpdate: function() {
|
||||
this._updateStops();
|
||||
|
||||
if (!this.diffRow) {
|
||||
this.reInitCursor();
|
||||
}
|
||||
this._scrollBehavior = ScrollBehavior.KEEP_VISIBLE;
|
||||
this._listeningForScroll = false;
|
||||
},
|
||||
|
||||
_handleDiffRenderStart: function() {
|
||||
this._listeningForScroll = true;
|
||||
},
|
||||
|
||||
/**
|
||||
@@ -320,12 +359,15 @@
|
||||
for (i = splice.index;
|
||||
i < splice.index + splice.addedCount;
|
||||
i++) {
|
||||
this.listen(this.diffs[i], 'render-start', '_handleDiffRenderStart');
|
||||
this.listen(this.diffs[i], 'render', 'handleDiffUpdate');
|
||||
}
|
||||
|
||||
for (i = 0;
|
||||
i < splice.removed && splice.removed.length;
|
||||
i++) {
|
||||
this.unlisten(splice.removed[i],
|
||||
'render-start', '_handleDiffRenderStart');
|
||||
this.unlisten(splice.removed[i], 'render', 'handleDiffUpdate');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,6 +98,17 @@ limitations under the License.
|
||||
assert.equal(cursorElement.diffRow, firstDeltaRow);
|
||||
});
|
||||
|
||||
test('cursor scroll behavior', function() {
|
||||
cursorElement._handleDiffRenderStart();
|
||||
assert.equal(cursorElement._scrollBehavior, 'keep-visible');
|
||||
|
||||
cursorElement._handleWindowScroll();
|
||||
assert.equal(cursorElement._scrollBehavior, 'never');
|
||||
|
||||
cursorElement.handleDiffUpdate();
|
||||
assert.equal(cursorElement._scrollBehavior, 'keep-visible');
|
||||
});
|
||||
|
||||
suite('unified diff', function() {
|
||||
|
||||
setup(function(done) {
|
||||
|
||||
Reference in New Issue
Block a user