Skip to content

Commit

Permalink
toggleLoading: remove force paramater (#27508)
Browse files Browse the repository at this point in the history
* toggleLoading: remove force parameter

* Oops.
  • Loading branch information
samouri committed Apr 16, 2020
1 parent ef4fd12 commit cdbb496
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 33 deletions.
2 changes: 1 addition & 1 deletion extensions/amp-list/0.1/amp-list.js
Expand Up @@ -539,7 +539,7 @@ export class AmpList extends AMP.BaseElement {
) {
// Placeholder and loading don't need a mutate context.
this.togglePlaceholder(true);
this.toggleLoading(true, /* opt_force */ true);
this.toggleLoading(true);
this.mutateElement(() => {
this.toggleFallback_(false);
// Clean up bindings in children before removing them from DOM.
Expand Down
7 changes: 2 additions & 5 deletions extensions/amp-list/0.1/test/test-amp-list.js
Expand Up @@ -160,7 +160,7 @@ describes.repeated(
// If "reset-on-refresh" is set, show loading/placeholder before fetch.
if (opts.resetOnRefresh) {
listMock.expects('togglePlaceholder').withExactArgs(true).once();
listMock.expects('toggleLoading').withExactArgs(true, true).once();
listMock.expects('toggleLoading').withExactArgs(true).once();
}

// Stub the rendering of the template.
Expand Down Expand Up @@ -1048,10 +1048,7 @@ describes.repeated(
listMock.expects('fetchList_').never();
// Expect display of placeholder/loading before render.
listMock.expects('togglePlaceholder').withExactArgs(true).once();
listMock
.expects('toggleLoading')
.withExactArgs(true, true)
.once();
listMock.expects('toggleLoading').withExactArgs(true).once();
// Expect hiding of placeholder/loading after render.
listMock.expects('togglePlaceholder').withExactArgs(false).once();
listMock.expects('toggleLoading').withExactArgs(false).once();
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-twitter/0.1/amp-twitter.js
Expand Up @@ -201,7 +201,7 @@ class AmpTwitter extends AMP.BaseElement {
mutatedAttributesCallback(mutations) {
if (this.iframe_ && mutations['data-tweetid'] != null) {
this.unlayoutCallback();
this.toggleLoading(true, /* opt_force */ true);
this.toggleLoading(true);
this.layoutCallback();
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/base-element.js
Expand Up @@ -722,14 +722,12 @@ export class BaseElement {
}

/**
* Hides or shows the loading indicator. This function must only
* be called inside a mutate context.
* Hides or shows the loading indicator.
* @param {boolean} state
* @param {boolean=} opt_force
* @public @final
*/
toggleLoading(state, opt_force) {
this.element.toggleLoading(state, {force: !!opt_force});
toggleLoading(state) {
this.element.toggleLoading(state);
}

/**
Expand Down
36 changes: 15 additions & 21 deletions src/custom-element.js
Expand Up @@ -1608,25 +1608,27 @@ function createBaseCustomElementClass(win) {
isLoadingEnabled_() {
// No loading indicator will be shown if either one of these conditions
// true:
// 1. `noloading` attribute is specified;
// 2. The element has not been whitelisted;
// 3. The element is too small or has not yet been measured;
// 4. The element has already been laid out (include having loading
// 1. The document is A4A.
// 2. `noloading` attribute is specified;
// 3. The element has already been laid out, and does not support reshowing the indicator (include having loading
// error);
// 5. The element is a `placeholder` or a `fallback`;
// 6. The element's layout is not a size-defining layout.
// 7. The document is A4A.
// 4. The element is too small or has not yet been measured;
// 5. The element has not been whitelisted;
// 6. The element is an internal node (e.g. `placeholder` or `fallback`);
// 7. The element's layout is not a size-defining layout.
if (this.isInA4A()) {
return false;
}
if (this.loadingDisabled_ === undefined) {
this.loadingDisabled_ = this.hasAttribute('noloading');
}

const laidOut =
this.layoutCount_ > 0 || this.signals_.get(CommonSignals.RENDER_START);
if (
this.layoutCount_ > 0 ||
this.layoutWidth_ <= 0 || // Layout is not ready or invisible
this.loadingDisabled_ ||
(laidOut && !this.implementation_.isLoadingReused()) ||
this.layoutWidth_ <= 0 || // Layout is not ready or invisible
!isLoadingAllowed(this) ||
isInternalOrServiceNode(this) ||
!isLayoutSizeDefined(this.layout_)
Expand Down Expand Up @@ -1686,22 +1688,14 @@ function createBaseCustomElementClass(win) {
/**
* Turns the loading indicator on or off.
* @param {boolean} state
* @param {{cleanup:(boolean|undefined), force:(boolean|undefined), startTime:(number|undefined)}=} opt_options
* @param {{cleanup:(boolean|undefined), startTime:(number|undefined)}=} opt_options
* @public @final
*/
toggleLoading(state, opt_options) {
const cleanup = opt_options && opt_options.cleanup;
const force = opt_options && opt_options.force;
const startTime = opt_options && opt_options.startTime;
assertNotTemplate(this);
if (
state &&
!this.implementation_.isLoadingReused() &&
(this.layoutCount_ > 0 || this.signals_.get(CommonSignals.RENDER_START))
) {
// Loading has already been canceled. Ignore.
return;
}

if (state === this.loadingState_ && !opt_options) {
// Loading state is the same.
return;
Expand All @@ -1712,7 +1706,7 @@ function createBaseCustomElementClass(win) {
}

// Check if loading should be shown.
if (state && !force && !this.isLoadingEnabled_()) {
if (state && !this.isLoadingEnabled_()) {
this.loadingState_ = false;
return;
}
Expand All @@ -1722,7 +1716,7 @@ function createBaseCustomElementClass(win) {
let state = this.loadingState_;
// Repeat "loading enabled" check because it could have changed while
// waiting for vsync.
if (state && !force && !this.isLoadingEnabled_()) {
if (state && !this.isLoadingEnabled_()) {
state = false;
}
if (state) {
Expand Down

0 comments on commit cdbb496

Please sign in to comment.