Fix diff flashing when expand inline diffs
There was an issue where when a diff was expanded that had previously
been expanded, its old content was displayed, then cleared, then
reloaded.
Fix this by clearing diffs when they are collapsed.
Bug: Issue 7986
Change-Id: I3dc6460ff6fba4ff9f2cbd4b004b1e50d4b75814
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 c4120eb..44c7e2d 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
@@ -819,6 +819,12 @@
* @param {!Array} record The splice record in the expanded paths list.
*/
_expandedPathsChanged(record) {
+ // Clear content for any diffs that are not open so if they get re-opened
+ // the stale content does not flash before it is cleared and reloaded.
+ const collapsedDiffs = this.diffs.filter(diff =>
+ this._expandedFilePaths.indexOf(diff.path) === -1);
+ this._clearCollapsedDiffs(collapsedDiffs);
+
if (!record) { return; }
this.filesExpanded = this._computeExpandedFiles(
@@ -843,6 +849,12 @@
this.$.diffCursor.handleDiffUpdate();
},
+ _clearCollapsedDiffs(collapsedDiffs) {
+ for (const diff of collapsedDiffs) {
+ diff.clearDiffContent();
+ }
+ },
+
/**
* Given an array of paths and a NodeList of diff elements, render the diff
* for each path in order, awaiting the previous render to complete before
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 0a45fe3..4c1326a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -831,39 +831,44 @@
test('_togglePathExpanded', () => {
const path = 'path/to/my/file.txt';
- element.files = [{__path: path}];
- const renderStub = sandbox.stub(element, '_renderInOrder')
- .returns(Promise.resolve());
+ element._files = [{__path: path}];
+ const renderSpy = sandbox.spy(element, '_renderInOrder');
+ const collapseStub = sandbox.stub(element, '_clearCollapsedDiffs');
assert.equal(element._expandedFilePaths.length, 0);
element._togglePathExpanded(path);
flushAsynchronousOperations();
+ assert.equal(collapseStub.lastCall.args[0].length, 0);
- assert.equal(renderStub.callCount, 1);
+ assert.equal(renderSpy.callCount, 1);
assert.include(element._expandedFilePaths, path);
element._togglePathExpanded(path);
flushAsynchronousOperations();
- assert.equal(renderStub.callCount, 2);
+ assert.equal(renderSpy.callCount, 2);
assert.notInclude(element._expandedFilePaths, path);
+ assert.equal(collapseStub.lastCall.args[0].length, 1);
});
- test('collapseAllDiffs', () => {
- sandbox.stub(element, '_renderInOrder')
- .returns(Promise.resolve());
+ test('expandAllDiffs and collapseAllDiffs', () => {
+ const collapseStub = sandbox.stub(element, '_clearCollapsedDiffs');
const cursorUpdateStub = sandbox.stub(element.$.diffCursor,
'handleDiffUpdate');
const path = 'path/to/my/file.txt';
- element.files = [{__path: path}];
- element._expandedFilePaths = [path];
- element._showInlineDiffs = true;
+ element._files = [{__path: path}];
+ element.expandAllDiffs();
+ flushAsynchronousOperations();
+ assert.isTrue(element._showInlineDiffs);
+ assert.isTrue(cursorUpdateStub.calledOnce);
+ assert.equal(collapseStub.lastCall.args[0].length, 0);
element.collapseAllDiffs();
flushAsynchronousOperations();
assert.equal(element._expandedFilePaths.length, 0);
assert.isFalse(element._showInlineDiffs);
- assert.isTrue(cursorUpdateStub.calledOnce);
+ assert.isTrue(cursorUpdateStub.calledTwice);
+ assert.equal(collapseStub.lastCall.args[0].length, 1);
});
test('_expandedPathsChanged', done => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 5f3269a..75ed70c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -176,7 +176,7 @@
this.clearBlame();
this._safetyBypass = null;
this._showWarning = false;
- this._clearDiffContent();
+ this.clearDiffContent();
const promises = [];
@@ -615,7 +615,7 @@
return this.prefs;
},
- _clearDiffContent() {
+ clearDiffContent() {
this.$.diffTable.innerHTML = null;
},
