Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make createLoaderLogoCallback static #32534

Merged
merged 1 commit into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions extensions/amp-facebook-comments/0.1/amp-facebook-comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ import {removeElement} from '../../../src/dom';
import {tryParseJson} from '../../../src/json';

class AmpFacebookComments extends AMP.BaseElement {
/** @override @nocollapse */
Copy link
Member

@samouri samouri Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can we document why we need @nocollapse somewhere? Even if just giving each a TODO with a link to a GH issue. If the nocollapse were specified in AMP.BaseElement would it mean we don't need to repeat in each override?

static createLoaderLogoCallback(element) {
return createLoaderLogo(element);
}

/** @param {!AmpElement} element */
constructor(element) {
super(element);
Expand Down Expand Up @@ -120,11 +125,6 @@ class AmpFacebookComments extends AMP.BaseElement {
}
}

/** @override */
createLoaderLogoCallback() {
return createLoaderLogo(this.element);
}

/** @override */
unlayoutCallback() {
if (this.iframe_) {
Expand Down
10 changes: 5 additions & 5 deletions extensions/amp-facebook-page/0.1/amp-facebook-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ import {removeElement} from '../../../src/dom';
import {tryParseJson} from '../../../src/json';

class AmpFacebookPage extends AMP.BaseElement {
/** @override @nocollapse */
static createLoaderLogoCallback(element) {
return createLoaderLogo(element);
}

/** @param {!AmpElement} element */
constructor(element) {
super(element);
Expand Down Expand Up @@ -122,11 +127,6 @@ class AmpFacebookPage extends AMP.BaseElement {
}
}

/** @override */
createLoaderLogoCallback() {
return createLoaderLogo(this.element);
}

/** @override */
unlayoutCallback() {
if (this.iframe_) {
Expand Down
10 changes: 5 additions & 5 deletions extensions/amp-facebook/0.1/amp-facebook.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ import {removeElement} from '../../../src/dom';
import {tryParseJson} from '../../../src/json';

class AmpFacebook extends AMP.BaseElement {
/** @override @nocollapse */
static createLoaderLogoCallback(element) {
return createLoaderLogo(element);
}

/** @param {!AmpElement} element */
constructor(element) {
super(element);
Expand Down Expand Up @@ -133,11 +138,6 @@ class AmpFacebook extends AMP.BaseElement {
}
}

/** @override */
createLoaderLogoCallback() {
return createLoaderLogo(this.element);
}

/** @override */
unlayoutOnPause() {
return true;
Expand Down
53 changes: 27 additions & 26 deletions extensions/amp-pinterest/0.1/amp-pinterest.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,33 @@ import {user, userAssert} from '../../../src/log';
* - buttonFollow: User follow button
*/
class AmpPinterest extends AMP.BaseElement {
/** @override @nocollapse */
static createLoaderLogoCallback(element) {
const type = element.getAttribute('data-do');
if (type != 'embedPin') {
return {};
}

const html = htmlFor(element);
return {
color: '#E60019',
content: html`
<svg viewBox="0 0 72 72">
<path
fill="currentColor"
d="M36,26c-5.52,0-9.99,4.47-9.99,9.99c0,4.24,2.63,7.85,6.35,9.31c-0.09-0.79-0.16-2.01,0.03-2.87
c0.18-0.78,1.17-4.97,1.17-4.97s-0.3-0.6-0.3-1.48c0-1.39,0.81-2.43,1.81-2.43c0.86,0,1.27,0.64,1.27,1.41
c0,0.86-0.54,2.14-0.83,3.33c-0.24,1,0.5,1.81,1.48,1.81c1.78,0,3.14-1.88,3.14-4.57c0-2.39-1.72-4.06-4.18-4.06
c-2.85,0-4.51,2.13-4.51,4.33c0,0.86,0.33,1.78,0.74,2.28c0.08,0.1,0.09,0.19,0.07,0.29c-0.07,0.31-0.25,1-0.28,1.13
c-0.04,0.18-0.15,0.22-0.34,0.13c-1.25-0.58-2.03-2.4-2.03-3.87c0-3.15,2.29-6.04,6.6-6.04c3.46,0,6.16,2.47,6.16,5.77
c0,3.45-2.17,6.22-5.18,6.22c-1.01,0-1.97-0.53-2.29-1.15c0,0-0.5,1.91-0.62,2.38c-0.22,0.87-0.83,1.96-1.24,2.62
c0.94,0.29,1.92,0.44,2.96,0.44c5.52,0,9.99-4.47,9.99-9.99C45.99,30.47,41.52,26,36,26z"
/>
</svg>
`,
};
}

/** @param {!AmpElement} element */
constructor(element) {
super(element);
Expand Down Expand Up @@ -118,32 +145,6 @@ class AmpPinterest extends AMP.BaseElement {
}
});
}

/** @override */
createLoaderLogoCallback() {
if (this.type_ != 'embedPin') {
return {};
}

const html = htmlFor(this.element);
return {
color: '#E60019',
content: html`
<svg viewBox="0 0 72 72">
<path
fill="currentColor"
d="M36,26c-5.52,0-9.99,4.47-9.99,9.99c0,4.24,2.63,7.85,6.35,9.31c-0.09-0.79-0.16-2.01,0.03-2.87
c0.18-0.78,1.17-4.97,1.17-4.97s-0.3-0.6-0.3-1.48c0-1.39,0.81-2.43,1.81-2.43c0.86,0,1.27,0.64,1.27,1.41
c0,0.86-0.54,2.14-0.83,3.33c-0.24,1,0.5,1.81,1.48,1.81c1.78,0,3.14-1.88,3.14-4.57c0-2.39-1.72-4.06-4.18-4.06
c-2.85,0-4.51,2.13-4.51,4.33c0,0.86,0.33,1.78,0.74,2.28c0.08,0.1,0.09,0.19,0.07,0.29c-0.07,0.31-0.25,1-0.28,1.13
c-0.04,0.18-0.15,0.22-0.34,0.13c-1.25-0.58-2.03-2.4-2.03-3.87c0-3.15,2.29-6.04,6.6-6.04c3.46,0,6.16,2.47,6.16,5.77
c0,3.45-2.17,6.22-5.18,6.22c-1.01,0-1.97-0.53-2.29-1.15c0,0-0.5,1.91-0.62,2.38c-0.22,0.87-0.83,1.96-1.24,2.62
c0.94,0.29,1.92,0.44,2.96,0.44c5.52,0,9.99-4.47,9.99-9.99C45.99,30.47,41.52,26,36,26z"
/>
</svg>
`,
};
}
}

AMP.extension('amp-pinterest', '0.1', (AMP) => {
Expand Down
42 changes: 21 additions & 21 deletions extensions/amp-twitter/0.1/amp-twitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,27 @@ import {listenFor} from '../../../src/iframe-helper';
import {removeElement} from '../../../src/dom';

class AmpTwitter extends AMP.BaseElement {
/** @override @nocollapse */
static createLoaderLogoCallback(element) {
const html = htmlFor(element);
return {
color: '#1DA1F2',
content: html`
<svg viewBox="0 0 72 72">
<path
fill="currentColor"
d="M32.29,44.13c7.55,0,11.67-6.25,11.67-11.67c0-0.18,0-0.35-0.01-0.53c0.8-0.58,1.5-1.3,2.05-2.12
c-0.74,0.33-1.53,0.55-2.36,0.65c0.85-0.51,1.5-1.31,1.8-2.27c-0.79,0.47-1.67,0.81-2.61,1c-0.75-0.8-1.82-1.3-3-1.3
c-2.27,0-4.1,1.84-4.1,4.1c0,0.32,0.04,0.64,0.11,0.94c-3.41-0.17-6.43-1.8-8.46-4.29c-0.35,0.61-0.56,1.31-0.56,2.06
c0,1.42,0.72,2.68,1.83,3.42c-0.67-0.02-1.31-0.21-1.86-0.51c0,0.02,0,0.03,0,0.05c0,1.99,1.41,3.65,3.29,4.02
c-0.34,0.09-0.71,0.14-1.08,0.14c-0.26,0-0.52-0.03-0.77-0.07c0.52,1.63,2.04,2.82,3.83,2.85c-1.4,1.1-3.17,1.76-5.1,1.76
c-0.33,0-0.66-0.02-0.98-0.06C27.82,43.45,29.97,44.13,32.29,44.13"
/>
</svg>
`,
};
}

/** @param {!AmpElement} element */
constructor(element) {
super(element);
Expand Down Expand Up @@ -155,27 +176,6 @@ class AmpTwitter extends AMP.BaseElement {
});
}

/** @override */
createLoaderLogoCallback() {
const html = htmlFor(this.element);
return {
color: '#1DA1F2',
content: html`
<svg viewBox="0 0 72 72">
<path
fill="currentColor"
d="M32.29,44.13c7.55,0,11.67-6.25,11.67-11.67c0-0.18,0-0.35-0.01-0.53c0.8-0.58,1.5-1.3,2.05-2.12
c-0.74,0.33-1.53,0.55-2.36,0.65c0.85-0.51,1.5-1.31,1.8-2.27c-0.79,0.47-1.67,0.81-2.61,1c-0.75-0.8-1.82-1.3-3-1.3
c-2.27,0-4.1,1.84-4.1,4.1c0,0.32,0.04,0.64,0.11,0.94c-3.41-0.17-6.43-1.8-8.46-4.29c-0.35,0.61-0.56,1.31-0.56,2.06
c0,1.42,0.72,2.68,1.83,3.42c-0.67-0.02-1.31-0.21-1.86-0.51c0,0.02,0,0.03,0,0.05c0,1.99,1.41,3.65,3.29,4.02
c-0.34,0.09-0.71,0.14-1.08,0.14c-0.26,0-0.52-0.03-0.77-0.07c0.52,1.63,2.04,2.82,3.83,2.85c-1.4,1.1-3.17,1.76-5.1,1.76
c-0.33,0-0.66-0.02-0.98-0.06C27.82,43.45,29.97,44.13,32.29,44.13"
/>
</svg>
`,
};
}

/** @override */
unlayoutOnPause() {
return true;
Expand Down
27 changes: 15 additions & 12 deletions src/base-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,21 @@ export class BaseElement {
return false;
}

/**
* Subclasses can override this method to provide a svg logo that will be
* displayed as the loader.
*
* @param {!AmpElement} unusedElement
* @return {{
* content: (!Element|undefined),
* color: (string|undefined),
* }}
* @nocollapse
*/
static createLoaderLogoCallback(unusedElement) {
return {};
}

/** @param {!AmpElement} element */
constructor(element) {
/** @public @const {!Element} */
Expand Down Expand Up @@ -374,18 +389,6 @@ export class BaseElement {
return null;
}

/**
* Subclasses can override this method to provide a svg logo that will be
* displayed as the loader.
* @return {{
* content: (!Element|undefined),
* color: (string|undefined),
* }}
*/
createLoaderLogoCallback() {
return {};
}

/**
* Subclasses can override this method to opt-out of rendering the element
* when it is not currently visible.
Expand Down
4 changes: 3 additions & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,9 @@ function createBaseCustomElementClass(win) {
* @final
*/
createLoaderLogo() {
return this.impl_ ? this.impl_.createLoaderLogoCallback() : {};
return this.implClass_
? this.implClass_.createLoaderLogoCallback(this)
: {};
}

/**
Expand Down