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.

Sign up for GitHub

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

♻️ Create plain <img> placeholders #34379

Merged
merged 3 commits into from
May 20, 2021
Merged

♻️ Create plain <img> placeholders #34379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2060ea
♻️ Create plain `<img>` placeholders
alanorozco May 13, 2021
c903b49
review
alanorozco May 14, 2021
f759370
tests
alanorozco May 17, 2021
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
9 changes: 4 additions & 5 deletions extensions/amp-apester-media/0.1/amp-apester-media.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import {
observeWithSharedInOb,
unobserveWithSharedInOb,
} from '../../../src/viewport-observer';
import {px, setStyles} from '../../../src/style';
import {removeElement} from '../../../src/dom';
import {setStyles} from '../../../src/style';

/** @const */
const TAG = 'amp-apester-media';
Expand Down Expand Up @@ -253,11 +253,10 @@ class AmpApesterMedia extends AMP.BaseElement {
* @return {!Element}
*/
constructLoaderImg_() {
const img = this.element.ownerDocument.createElement('amp-img');
const img = this.element.ownerDocument.createElement('img');
img.setAttribute('loading', 'lazy');
img.setAttribute('src', this.loaderUrl_);
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
img.setAttribute('layout', 'fixed');
img.setAttribute('width', '100');
img.setAttribute('height', '100');
setStyles(img, {'width': px(100), 'height': px(100)});
return img;
}

Expand Down
24 changes: 13 additions & 11 deletions extensions/amp-brid-player/0.1/amp-brid-player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,27 +242,29 @@ class AmpBridPlayer extends AMP.BaseElement {

const {partnerID_: partnerID, feedID_: feedID} = this;

const placeholder = htmlFor(element)`
<amp-img referrerpolicy=origin layout=fill placeholder>
<amp-img referrerpolicy=origin layout=fill fallback
src="https://cdn.brid.tv/live/default/defaultSnapshot.png">
</amp-img>
</amp-img>`;
const html = htmlFor(element);
const placeholder = html`
<img placeholder referrerpolicy="origin" loading="lazy" />
`;

this.propagateAttributes(['aria-label'], placeholder);
this.applyFillContent(placeholder);

const altText = placeholder.hasAttribute('aria-label')
? 'Loading video - ' + placeholder.getAttribute('aria-label')
: 'Loading video';

placeholder.setAttribute('alt', altText);

placeholder.setAttribute(
'src',
`https://cdn.brid.tv/live/partners/${encodeURIComponent(partnerID)}` +
`/snapshot/${encodeURIComponent(feedID)}.jpg`
);

const altText = placeholder.hasAttribute('aria-label')
? 'Loading video - ' + placeholder.getAttribute('aria-label')
: 'Loading video';

placeholder.setAttribute('alt', altText);
this.loadPromise(placeholder).catch(() => {
placeholder.src = 'https://cdn.brid.tv/live/default/defaultSnapshot.png';
});

return placeholder;
}
Expand Down
25 changes: 4 additions & 21 deletions extensions/amp-brid-player/0.1/test/test-amp-brid-player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ describes.realWin(
'data-player': '979',
'data-video': '13663',
}).then((brid) => {
const img = brid.querySelector('amp-img');
const img = brid.querySelector('img');
expect(img).to.not.be.null;
expect(img.getAttribute('src')).to.equal(
'https://cdn.brid.tv/live/partners/264/snapshot/13663.jpg'
);
expect(img.getAttribute('layout')).to.equal('fill');
expect(img.hasAttribute('placeholder')).to.be.true;
expect(img).to.have.class('i-amphtml-fill-content');
expect(img).to.have.attribute('placeholder');
expect(img.getAttribute('alt')).to.equal('Loading video');
expect(img.getAttribute('referrerpolicy')).to.equal('origin');
});
Expand All @@ -234,30 +234,13 @@ describes.realWin(
'data-video': '13663',
'aria-label': 'great video',
}).then((brid) => {
const img = brid.querySelector('amp-img');
const img = brid.querySelector('img');
expect(img).to.not.be.null;
expect(img.getAttribute('alt')).to.equal(
'Loading video - great video'
);
});
});
it('should create a fallback for default snapshot', () => {
return getBridPlayer({
'data-partner': '264',
'data-player': '979',
'data-video': '13663',
}).then((brid) => {
const img = brid.querySelector('amp-img');
const fallbackImg = img.querySelector('amp-img');
expect(fallbackImg).to.not.be.null;
expect(fallbackImg.getAttribute('src')).to.equal(
'https://cdn.brid.tv/live/default/defaultSnapshot.png'
);
expect(fallbackImg.getAttribute('layout')).to.equal('fill');
expect(fallbackImg.hasAttribute('fallback')).to.be.true;
expect(fallbackImg.getAttribute('referrerpolicy')).to.equal('origin');
});
});
});
}
);
4 changes: 0 additions & 4 deletions extensions/amp-delight-player/0.1/amp-delight-player.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@

amp-delight-player [placeholder] {
transition: opacity 0.5s ease-out;
background: no-repeat 50%;
background-size: cover;
width: 100%;
height: 100%;
}

amp-delight-player [placeholder].i-amphtml-delight-player-faded {
Expand Down
7 changes: 4 additions & 3 deletions extensions/amp-delight-player/0.1/amp-delight-player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,13 @@ class AmpDelightPlayer extends AMP.BaseElement {
createPlaceholderCallback() {
const html = htmlFor(this.element);
const placeholder = html`
<div placeholder><amp-img layout="fill"></amp-img></div>
<img placeholder referrerpolicy="origin" loading="lazy" />
`;

const src = `${this.baseURL_}/poster/${this.contentID_}`;
this.applyFillContent(placeholder);

placeholder.firstElementChild.setAttribute('src', src);
const src = `${this.baseURL_}/poster/${this.contentID_}`;
placeholder.setAttribute('src', src);

this.placeholderEl_ = /** @type {HTMLElement} */ (placeholder);

Expand Down
14 changes: 7 additions & 7 deletions extensions/amp-gfycat/0.1/amp-gfycat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,11 @@ class AmpGfycat extends AMP.BaseElement {

/** @override */
createPlaceholderCallback() {
const placeholder = this.win.document.createElement('amp-img');
const placeholder = this.win.document.createElement('img');
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
const videoid = dev().assertString(this.videoid_);
this.applyFillContent(placeholder);
this.propagateAttributes(['alt', 'aria-label'], placeholder);
placeholder.setAttribute(
'src',
'https://thumbs.gfycat.com/' + encodeURIComponent(videoid) + '-poster.jpg'
);
placeholder.setAttribute('layout', 'fill');
placeholder.setAttribute('loading', 'lazy');
placeholder.setAttribute('placeholder', '');
placeholder.setAttribute('referrerpolicy', 'origin');
if (this.element.hasAttribute('aria-label')) {
Expand All @@ -110,7 +107,10 @@ class AmpGfycat extends AMP.BaseElement {
} else {
placeholder.setAttribute('alt', 'Loading gif');
}
this.applyFillContent(placeholder);
placeholder.setAttribute(
'src',
'https://thumbs.gfycat.com/' + encodeURIComponent(videoid) + '-poster.jpg'
);

return placeholder;
}
Expand Down
7 changes: 5 additions & 2 deletions extensions/amp-gfycat/0.1/test/test-amp-gfycat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,11 @@ describes.realWin(
return getGfycat('LeanMediocreBeardeddragon', {
withAlt: true,
}).then((gfycat) => {
const placeHolder = gfycat.querySelector('amp-img');
const placeHolder = gfycat.querySelector('img');
expect(placeHolder).to.not.be.null;
expect(placeHolder).to.have.attribute('placeholder');
expect(placeHolder).to.have.class('i-amphtml-fill-content');
expect(placeHolder.getAttribute('loading')).to.equal('lazy');
expect(placeHolder.getAttribute('alt')).to.equal(
'Loading gif test alt label'
);
Expand All @@ -140,7 +143,7 @@ describes.realWin(
return getGfycat('LeanMediocreBeardeddragon', {
withAria: true,
}).then((gfycat) => {
const placeHolder = gfycat.querySelector('amp-img');
const placeHolder = gfycat.querySelector('img');
expect(placeHolder).to.not.be.null;
expect(placeHolder.getAttribute('alt')).to.equal(
'Loading gif test aria label'
Expand Down
8 changes: 5 additions & 3 deletions extensions/amp-iframely/0.1/amp-iframely.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,17 @@ export class AmpIframely extends AMP.BaseElement {
this.constructSrc_('/thumbnail'),
this.options_
);
return createElementWithAttributes(
const element = createElementWithAttributes(
this.element.ownerDocument,
'amp-img',
'img',
{
'src': src,
'loading': 'lazy',
'placeholder': '',
'layout': 'fill',
}
);
this.applyFillContent(element);
return element;
}
return null;
}
Expand Down
36 changes: 17 additions & 19 deletions extensions/amp-iframely/0.1/test/test-amp-iframely.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,16 @@ describes.realWin(

it('renders image placeholder', () => {
return renderIframely(paramsID).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(image.tagName).to.equal('AMP-IMG');
expect(image.className).to.match(
/i-amphtml-element i-amphtml-notbuilt amp-notbuilt i-amphtml-layout-fill i-amphtml-layout-size-defined/
);
expect(image).to.have.class('i-amphtml-fill-content');
expect(image.getAttribute('loading')).to.equal('lazy');
});
});

it('renders image placeholder with proper URL for ID version', () => {
return renderIframely(paramsID).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(image.getAttribute('src')).to.equal(
`https://cdn.iframe.ly/${TestID}/thumbnail?amp=1`
Expand All @@ -162,7 +160,7 @@ describes.realWin(

it('renders image placeholder with proper URL for Key-URL version', () => {
return renderIframely(paramsKU).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(image.getAttribute('src')).to.equal(
`https://cdn.iframe.ly/api/thumbnail?url=${encodeURIComponent(
Expand All @@ -183,7 +181,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(image.getAttribute('src')).to.equal(
`https://${properDomain}/${TestID}/thumbnail?amp=1`
Expand All @@ -203,7 +201,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(image.getAttribute('src')).to.equal(
`https://${domain}/${TestID}/thumbnail?amp=1`
Expand All @@ -220,7 +218,7 @@ describes.realWin(
'layout': 'fill',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -235,7 +233,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -251,7 +249,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -265,7 +263,7 @@ describes.realWin(
'layout': 'fixed',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -280,7 +278,7 @@ describes.realWin(
'layout': 'fixed',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -296,7 +294,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -311,7 +309,7 @@ describes.realWin(
'layout': 'fixed',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.not.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -325,7 +323,7 @@ describes.realWin(
'resizable': '',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand All @@ -340,7 +338,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
expect(image).to.be.null;
expect(iframely.querySelector('iframe')).to.not.be.null;
});
Expand Down Expand Up @@ -372,7 +370,7 @@ describes.realWin(
'layout': 'responsive',
};
return renderIframely(data).then((iframely) => {
const image = iframely.querySelector('amp-img');
const image = iframely.querySelector('img');
const iframe = iframely.querySelector('iframe');
expect(image).to.not.be.null;
expect(iframe).to.not.be.null;
Expand Down
17 changes: 9 additions & 8 deletions extensions/amp-jwplayer/0.1/amp-jwplayer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,9 @@ class AmpJWPlayer extends AMP.BaseElement {
if (!this.element.hasAttribute('data-media-id')) {
return;
}
const placeholder = this.win.document.createElement('amp-img');
const placeholder = this.win.document.createElement('img');
this.propagateAttributes(['aria-label'], placeholder);
placeholder.setAttribute(
'src',
'https://content.jwplatform.com/thumbs/' +
encodeURIComponent(this.contentid_) +
'-720.jpg'
);
placeholder.setAttribute('layout', 'fill');
this.applyFillContent(placeholder);
placeholder.setAttribute('placeholder', '');
placeholder.setAttribute('referrerpolicy', 'origin');
if (placeholder.hasAttribute('aria-label')) {
Expand All @@ -367,6 +361,13 @@ class AmpJWPlayer extends AMP.BaseElement {
} else {
placeholder.setAttribute('alt', 'Loading video');
}
placeholder.setAttribute('loading', 'lazy');
placeholder.setAttribute(
'src',
'https://content.jwplatform.com/thumbs/' +
encodeURIComponent(this.contentid_) +
'-720.jpg'
);
return placeholder;
}

Expand Down
Loading