Make gr-diff own "loading" state

The loading state is only used in gr-diff, so it makes sense to be also
set in gr-diff.
This change slightly changes the behavior of loading to flip to false
when the content is rendered, not only after syntax highlighting is
done. That is not a problem though,
because the two places that read it only care about content, too.

Change-Id: I2b4bda1ff8f336bce4a7a763d1479b2474a2beb7
This commit is contained in:
Ole
2020-10-13 14:59:08 +02:00
parent b036826d6e
commit e3ea65da29
5 changed files with 17 additions and 18 deletions

View File

@@ -231,9 +231,6 @@ export class GrDiffHost extends GestureEventListeners(
@property({type: Boolean}) @property({type: Boolean})
_loggedIn = false; _loggedIn = false;
@property({type: Boolean})
_loading = false;
@property({type: String}) @property({type: String})
_errorMessage: string | null = null; _errorMessage: string | null = null;
@@ -331,7 +328,7 @@ export class GrDiffHost extends GestureEventListeners(
this.clear(); this.clear();
if (!this.path) throw new Error('Missing required "path" property.'); if (!this.path) throw new Error('Missing required "path" property.');
if (!this.changeNum) throw new Error('Missing required "changeNum" prop.'); if (!this.changeNum) throw new Error('Missing required "changeNum" prop.');
this._loading = true; this.diff = undefined;
this._errorMessage = null; this._errorMessage = null;
const whitespaceLevel = this._getIgnoreWhitespace(); const whitespaceLevel = this._getIgnoreWhitespace();
@@ -381,7 +378,6 @@ export class GrDiffHost extends GestureEventListeners(
} finally { } finally {
this.reporting.timeEnd(TimingLabel.TOTAL); this.reporting.timeEnd(TimingLabel.TOTAL);
} }
this._loading = false;
} }
private _getLayers(path: string, changeNum: NumericChangeId): DiffLayer[] { private _getLayers(path: string, changeNum: NumericChangeId): DiffLayer[] {

View File

@@ -32,7 +32,6 @@ export const htmlTemplate = html`
view-mode="[[viewMode]]" view-mode="[[viewMode]]"
line-of-interest="[[lineOfInterest]]" line-of-interest="[[lineOfInterest]]"
logged-in="[[_loggedIn]]" logged-in="[[_loggedIn]]"
loading="[[_loading]]"
error-message="[[_errorMessage]]" error-message="[[_errorMessage]]"
base-image="[[_baseImage]]" base-image="[[_baseImage]]"
revision-image="[[_revisionImage]]" revision-image="[[_revisionImage]]"

View File

@@ -388,6 +388,13 @@ suite('gr-diff-host tests', () => {
sinon.stub(element, '_getDiff').callsFake(() => new Promise(() => {})); sinon.stub(element, '_getDiff').callsFake(() => new Promise(() => {}));
element.patchRange = {}; element.patchRange = {};
// Needs to be set to something first for it to cancel.
element.diff = {
content: [{
a: ['foo'],
}],
};
element.reload(); element.reload();
assert.isTrue(cancelStub.called); assert.isTrue(cancelStub.called);
}); });
@@ -824,7 +831,7 @@ suite('gr-diff-host tests', () => {
test('delegates cancel()', () => { test('delegates cancel()', () => {
const stub = sinon.stub(element.$.diff, 'cancel'); const stub = sinon.stub(element.$.diff, 'cancel');
element.patchRange = {}; element.patchRange = {};
element.reload(); element.cancel();
assert.isTrue(stub.calledOnce); assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0); assert.equal(stub.lastCall.args.length, 0);
}); });

View File

@@ -191,8 +191,9 @@ export class GrDiff extends GestureEventListeners(
@property({type: Object}) @property({type: Object})
lineOfInterest?: LineOfInterest; lineOfInterest?: LineOfInterest;
@property({type: Boolean, observer: '_loadingChanged'}) /** True when diff is changed, until the content is done rendering. */
loading = false; @property({type: Boolean})
_loading = false;
@property({type: Boolean}) @property({type: Boolean})
loggedIn = false; loggedIn = false;
@@ -787,12 +788,6 @@ export class GrDiff extends GestureEventListeners(
this.clearDiffContent(); this.clearDiffContent();
} }
_loadingChanged(newValue?: boolean) {
if (newValue) {
this._cleanup();
}
}
_lineWrappingObserver() { _lineWrappingObserver() {
this._prefsChanged(this.prefs); this._prefsChanged(this.prefs);
} }
@@ -831,8 +826,9 @@ export class GrDiff extends GestureEventListeners(
} }
_diffChanged(newValue?: DiffInfo) { _diffChanged(newValue?: DiffInfo) {
this._loading = true;
this._cleanup();
if (newValue) { if (newValue) {
this._cleanup();
this._diffLength = this.getDiffLength(newValue); this._diffLength = this.getDiffLength(newValue);
this._debounceRenderDiffTable(); this._debounceRenderDiffTable();
} }
@@ -890,6 +886,7 @@ export class GrDiff extends GestureEventListeners(
} }
_handleRenderContent() { _handleRenderContent() {
this._loading = false;
this._unobserveIncrementalNodes(); this._unobserveIncrementalNodes();
this._incrementalNodeObserver = (dom( this._incrementalNodeObserver = (dom(
this this

View File

@@ -446,7 +446,7 @@ export const htmlTemplate = html`
<template <template
is="dom-if" is="dom-if"
if="[[showNoChangeMessage(loading, prefs, _diffLength, diff)]]" if="[[showNoChangeMessage(_loading, prefs, _diffLength, diff)]]"
> >
<div class="whitespace-change-only-message"> <div class="whitespace-change-only-message">
This file only contains whitespace changes. Modify the whitespace This file only contains whitespace changes. Modify the whitespace
@@ -457,7 +457,7 @@ export const htmlTemplate = html`
</gr-diff-highlight> </gr-diff-highlight>
</gr-diff-selection> </gr-diff-selection>
</div> </div>
<div class$="[[_computeNewlineWarningClass(_newlineWarning, loading)]]"> <div class$="[[_computeNewlineWarningClass(_newlineWarning, _loading)]]">
[[_newlineWarning]] [[_newlineWarning]]
</div> </div>
<div id="loadingError" class$="[[_computeErrorClass(errorMessage)]]"> <div id="loadingError" class$="[[_computeErrorClass(errorMessage)]]">