opendev/gerrit
Code Issues Proposed changes

Set _loading flag in diff-view while awaiting API

Browse Source 
Previously, PolyGerrit relied on the default value of the loading flag
in the diff view to show/hide the appropriate diff panels (or loading
message). Because of this, stale information is displayed when a user
navigates between different files within the same diff view.

With this change, the `_loading` flag is set before the major API calls
are made, and the loading message is made more obvious.

Bug: Issue 7381
Change-Id: Id70d981a2c72fef9f44a32c5463dc43c3ce41cf6
(cherry picked from commit 29b2ab4a69)
This commit is contained in:
Kasper Nilsson
2017-10-09 11:36:19 -07:00
committed by Paladox none
parent 9fb181d7b8
commit 7d521fc71c
3 changed files with 14 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
View File

@@ -124,8 +124,11 @@ limitations under the License.
text-decoration: none;
}
.loading {
padding: 0 var(--default-horizontal-margin) 1em;
color: #666;
color: #777;
font-size: 2em;
height: 100%;
padding: 1em var(--default-horizontal-margin);
text-align: center;
}
.subHeader {
flex-wrap: wrap;

1
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -540,6 +540,7 @@
promises.push(this._getChangeEdit(this._changeNum));
this._loading = true;
Promise.all(promises).then(r => {
const edit = r[4];
if (edit) {

8
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
View File

@@ -100,23 +100,27 @@ limitations under the License.
'10', PARENT), 'Should navigate to /c/42/10/wheatley.md');
element._path = 'wheatley.md';
assert.equal(element.changeViewState.selectedFileIndex, 2);
assert.isTrue(element._loading);
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(diffNavStub.lastCall.calledWith(element._change, 'glados.txt',
'10', PARENT), 'Should navigate to /c/42/10/glados.txt');
element._path = 'glados.txt';
assert.equal(element.changeViewState.selectedFileIndex, 1);
assert.isTrue(element._loading);
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(diffNavStub.lastCall.calledWith(element._change, 'chell.go', '10',
PARENT), 'Should navigate to /c/42/10/chell.go');
element._path = 'chell.go';
assert.equal(element.changeViewState.selectedFileIndex, 0);
assert.isTrue(element._loading);
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(changeNavStub.lastCall.calledWith(element._change),
'Should navigate to /c/42/');
assert.equal(element.changeViewState.selectedFileIndex, 0);
assert.isTrue(element._loading);
const showPrefsStub =
sandbox.stub(element.$.diffPreferences.$.prefsOverlay, 'open',
@@ -190,24 +194,28 @@ limitations under the License.
'5'), 'Should navigate to /c/42/5..10');
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change,
'wheatley.md', '10', '5'),
'Should navigate to /c/42/5..10/wheatley.md');
element._path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change,
'glados.txt', '10', '5'),
'Should navigate to /c/42/5..10/glados.txt');
element._path = 'glados.txt';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change, 'chell.go',
'10', '5'),
'Should navigate to /c/42/5..10/chell.go');
element._path = 'chell.go';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert.isTrue(element._loading);
assert(changeNavStub.lastCall.calledWithExactly(element._change, '10',
'5'),
'Should navigate to /c/42/5..10');