Skip to content

Commit

Permalink
♿ Adds Alt & aria-label to Placeholder Images (#14391)
Browse files Browse the repository at this point in the history 
* adds alt & aria label to brid-player & tests

* adds alt & aria-label to amp-jwplayer & tests to cover

* adds alt & aria-label to amp-kaltura-player & tests

* adds alt & aria-label to placeholder img for amp-springboard-player

* adds alt & aria-label attributes to img placeholder for amp-youtube

* fixes lint errors

* removes duplicated line
  • Loading branch information
@gulliverhan @glevitzky
gulliverhan authored and glevitzky committed Apr 27, 2018
1 parent 5e8e0f3 commit 9cd79a9
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 21 deletions.
40 changes: 24 additions & 16 deletions extensions/amp-brid-player/0.1/amp-brid-player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ class AmpBridPlayer extends AMP.BaseElement {
}

/**
* @param {boolean=} opt_onLayout
* @override
*/
* @param {boolean=} opt_onLayout
* @override
*/
preconnectCallback(opt_onLayout) {
this.preconnect.url('https://services.brid.tv', opt_onLayout);
this.preconnect.url('https://cdn.brid.tv', opt_onLayout);
Expand Down Expand Up @@ -122,8 +122,8 @@ class AmpBridPlayer extends AMP.BaseElement {

this.feedID_ = user().assert(
(this.element.getAttribute('data-video') ||
this.element.getAttribute('data-playlist') ||
this.element.getAttribute('data-outstream')),
this.element.getAttribute('data-playlist') ||
this.element.getAttribute('data-outstream')),
'Either the data-video or the data-playlist or the data-outstream ' +
'attributes must be specified for <amp-brid-player> %s',
this.element);
Expand All @@ -149,7 +149,7 @@ class AmpBridPlayer extends AMP.BaseElement {
this.unlistenMessage_ = listen(
this.win,
'message',
this. handleBridMessages_.bind(this)
this.handleBridMessages_.bind(this)
);

this.element.appendChild(iframe);
Expand Down Expand Up @@ -185,24 +185,32 @@ class AmpBridPlayer extends AMP.BaseElement {
const partnerID = this.partnerID_;
const feedID = this.feedID_;

this.propagateAttributes(['aria-label'], placeholder);
if (this.element.hasAttribute('data-video') ||
this.element.hasAttribute('data-playlist')) {
this.element.hasAttribute('data-playlist')) {

const placeholderFallback = this.win.document.createElement('amp-img');
placeholderFallback.setAttribute('src',
'https://cdn.brid.tv/live/default/defaultSnapshot.png');
'https://cdn.brid.tv/live/default/defaultSnapshot.png');
placeholderFallback.setAttribute('referrerpolicy', 'origin');
placeholderFallback.setAttribute('layout', 'fill');
placeholderFallback.setAttribute('fallback', '');
placeholder.appendChild(placeholderFallback);

placeholder.setAttribute('src',
'https://cdn.brid.tv/live/partners/' +
encodeURIComponent(partnerID) + '/snapshot/' +
encodeURIComponent(feedID) + '.jpg');
'https://cdn.brid.tv/live/partners/' +
encodeURIComponent(partnerID) + '/snapshot/' +
encodeURIComponent(feedID) + '.jpg');
placeholder.setAttribute('layout', 'fill');
placeholder.setAttribute('placeholder', '');
placeholder.setAttribute('referrerpolicy', 'origin');
if (placeholder.hasAttribute('aria-label')) {
placeholder.setAttribute('alt',
'Loading video - ' + placeholder.getAttribute('aria-label')
);
} else {
placeholder.setAttribute('alt', 'Loading video');
}
this.applyFillContent(placeholder);

return placeholder;
Expand All @@ -214,11 +222,11 @@ class AmpBridPlayer extends AMP.BaseElement {
}

/**
* Sends a command to the player through postMessage.
* @param {string} command
* @param {*=} opt_arg
* @private
* */
* Sends a command to the player through postMessage.
* @param {string} command
* @param {*=} opt_arg
* @private
* */
sendCommand_(command, opt_arg) {

this.playerReadyPromise_.then(() => {
Expand Down
14 changes: 13 additions & 1 deletion extensions/amp-brid-player/0.1/test/test-amp-brid-player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,22 @@ describes.realWin('amp-brid-player', {
'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.getAttribute('alt')).to.equal('Loading video');
expect(img.getAttribute('referrerpolicy')).to.equal('origin');
});
});

it('should propagate aria label for placeholder image', () => {
return getBridPlayer({
'data-partner': '264',
'data-player': '979',
'data-video': '13663',
'aria-label': 'great video',
}).then(brid => {
const img = brid.querySelector('amp-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',
Expand Down
8 changes: 8 additions & 0 deletions extensions/amp-jwplayer/0.1/amp-jwplayer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,19 @@ class AmpJWPlayer extends AMP.BaseElement {
return;
}
const placeholder = this.win.document.createElement('amp-img');
this.propagateAttributes(['aria-label'], placeholder);
placeholder.setAttribute('src', 'https://content.jwplatform.com/thumbs/' +
encodeURIComponent(this.contentid_) + '-720.jpg');
placeholder.setAttribute('layout', 'fill');
placeholder.setAttribute('placeholder', '');
placeholder.setAttribute('referrerpolicy', 'origin');
if (placeholder.hasAttribute('aria-label')) {
placeholder.setAttribute('alt',
'Loading video - ' + placeholder.getAttribute('aria-label')
);
} else {
placeholder.setAttribute('alt', 'Loading video');
}
return placeholder;
}
}
Expand Down
15 changes: 14 additions & 1 deletion extensions/amp-jwplayer/0.1/test/test-amp-jwplayer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,22 @@ describes.realWin('amp-jwplayer', {
expect(img.getAttribute('layout')).to.equal('fill');
expect(img.hasAttribute('placeholder')).to.be.true;
expect(img.getAttribute('referrerpolicy')).to.equal('origin');
expect(img.getAttribute('alt')).to.equal('Loading video');
});
});
it('should propagate aria-label to placeholder', () => {
return getjwplayer({
'data-media-id': 'Wferorsv',
'data-player-id': 'sDZEo0ea',
'aria-label': 'interesting video',
}).then(jwp => {
const img = jwp.querySelector('amp-img');
expect(img).to.not.be.null;
expect(img.getAttribute('aria-label')).to.equal('interesting video');
expect(img.getAttribute('alt'))
.to.equal('Loading video - interesting video');
});
});

it('should not create a placeholder for playlists', () => {
return getjwplayer({
'data-playlist-id': 'Wferorsv',
Expand Down
8 changes: 8 additions & 0 deletions extensions/amp-kaltura-player/0.1/amp-kaltura-player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class AmpKaltura extends AMP.BaseElement {
/** @override */
createPlaceholderCallback() {
const placeholder = this.win.document.createElement('amp-img');
this.propagateAttributes(['aria-label'], placeholder);
const width = this.element.getAttribute('width');
const height = this.element.getAttribute('height');
let src = `https://cdnapisec.kaltura.com/p/${encodeURIComponent(this.partnerId_)}/thumbnail/entry_id/${encodeURIComponent(this.entryId_)}`;
Expand All @@ -93,6 +94,13 @@ class AmpKaltura extends AMP.BaseElement {
placeholder.setAttribute('layout', 'fill');
placeholder.setAttribute('placeholder', '');
placeholder.setAttribute('referrerpolicy', 'origin');
if (placeholder.hasAttribute('aria-label')) {
placeholder.setAttribute('alt',
'Loading video - ' + placeholder.getAttribute('aria-label')
);
} else {
placeholder.setAttribute('alt', 'Loading video');
}
return placeholder;
}

Expand Down
15 changes: 15 additions & 0 deletions extensions/amp-kaltura-player/0.1/test/test-amp-kaltura-player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ describes.realWin('amp-kaltura-player', {
expect(img.getAttribute('layout')).to.equal('fill');
expect(img.hasAttribute('placeholder')).to.be.true;
expect(img.getAttribute('referrerpolicy')).to.equal('origin');
expect(img.getAttribute('alt')).to.equal('Loading video');
});
});
it('should propagate aria label to placeholder image', () => {
return getKaltura({
'data-partner': '1281471',
'data-entryid': '1_3ts1ms9c',
'data-uiconf': '33502051',
'aria-label': 'great video',
}).then(kp => {
const img = kp.querySelector('amp-img');
expect(img).to.not.be.null;
expect(img.hasAttribute('placeholder')).to.be.true;
expect(img.getAttribute('aria-label')).to.equal('great video');
expect(img.getAttribute('alt')).to.equal('Loading video - great video');
});
});
});
Expand Down
9 changes: 8 additions & 1 deletion extensions/amp-springboard-player/0.1/amp-springboard-player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class AmpSpringboardPlayer extends AMP.BaseElement {
/** @override */
createPlaceholderCallback() {
const placeholder = this.win.document.createElement('amp-img');

this.propagateAttributes(['aria-label'], placeholder);
placeholder.setAttribute('src',
'https://www.springboardplatform.com/storage/' +
encodeURIComponent(this.domain_) + '/snapshots/' +
Expand All @@ -122,6 +122,13 @@ class AmpSpringboardPlayer extends AMP.BaseElement {
placeholder.setAttribute('placeholder', '');
placeholder.setAttribute('referrerpolicy', 'origin');
placeholder.setAttribute('layout', 'fill');
if (placeholder.hasAttribute('aria-label')) {
placeholder.setAttribute('alt',
'Loading video - ' + placeholder.getAttribute('aria-label')
);
} else {
placeholder.setAttribute('alt', 'Loading video');
}
return placeholder;
}
}
Expand Down
24 changes: 24 additions & 0 deletions extensions/amp-springboard-player/0.1/test/test-amp-springboard-player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,30 @@ describes.realWin('amp-springboard-player', {
expect(img.getAttribute('layout')).to.equal('fill');
expect(img.hasAttribute('placeholder')).to.be.true;
expect(img.getAttribute('referrerpolicy')).to.equal('origin');
expect(img.getAttribute('alt')).to.equal('Loading video');
});
});
it('should propagate aria-label for placeholder image', () => {
return getSpringboardPlayer({
'data-site-id': '261',
'data-mode': 'video',
'data-content-id': '1578473',
'data-player-id': 'test401',
'data-domain': 'test.com',
'data-items': '10',
'aria-label': 'sporty video',
}).then(kp => {
const img = kp.querySelector('amp-img');
expect(img).to.not.be.null;
expect(img.getAttribute('src')).to.equal(
'https://www.springboardplatform.com/storage/test.com' +
'/snapshots/1578473.jpg');
expect(img.getAttribute('layout')).to.equal('fill');
expect(img.hasAttribute('placeholder')).to.be.true;
expect(img.getAttribute('referrerpolicy')).to.equal('origin');
expect(img.getAttribute('aria-label')).to.equal('sporty video');
expect(img.getAttribute('alt'))
.to.equal('Loading video - sporty video');
});
});

Expand Down
10 changes: 8 additions & 2 deletions extensions/amp-youtube/0.1/amp-youtube.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,15 +407,21 @@ class AmpYoutube extends AMP.BaseElement {
// the object-fit: cover.
'visibility': 'hidden',
});

this.propagateAttributes(['aria-label'], imgPlaceholder);
// TODO(mkhatib): Maybe add srcset to allow the browser to
// load the needed size or even better match YTPlayer logic for loading
// player thumbnails for different screen sizes for a cache win!
imgPlaceholder.src = 'https://i.ytimg.com/vi/' +
encodeURIComponent(videoid) + '/sddefault.jpg#404_is_fine';
imgPlaceholder.setAttribute('placeholder', '');
imgPlaceholder.setAttribute('referrerpolicy', 'origin');

if (imgPlaceholder.hasAttribute('aria-label')) {
imgPlaceholder.setAttribute('alt',
'Loading video - ' + imgPlaceholder.getAttribute('aria-label')
);
} else {
imgPlaceholder.setAttribute('alt', 'Loading video');
}
this.applyFillContent(imgPlaceholder);
this.element.appendChild(imgPlaceholder);

Expand Down
16 changes: 16 additions & 0 deletions extensions/amp-youtube/0.1/test/test-amp-youtube.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ describes.realWin('amp-youtube', {
expect(imgPlaceholder.src).to.be.equal(
`https://i.ytimg.com/vi/${EXAMPLE_VIDEOID}/sddefault.jpg#404_is_fine`);
expect(imgPlaceholder.getAttribute('referrerpolicy')).to.equal('origin');
expect(imgPlaceholder.getAttribute('alt')).to.equal('Loading video');
}).then(yt => {
const iframe = yt.querySelector('iframe');
expect(iframe).to.not.be.null;
Expand All @@ -275,6 +276,21 @@ describes.realWin('amp-youtube', {
});
});

it('propagates aria-label to img placeholder', () => {
return getYt({
'data-videoid': EXAMPLE_VIDEOID,
'aria-label': 'kind video',
}, true, function(yt) {
const iframe = yt.querySelector('iframe');
expect(iframe).to.be.null;
const imgPlaceholder = yt.querySelector('img[placeholder]');
expect(imgPlaceholder).to.not.be.null;
expect(imgPlaceholder.getAttribute('aria-label')).to.equal('kind video');
expect(imgPlaceholder.getAttribute('alt'))
.to.equal('Loading video - kind video');
});
});

it('loads only sddefault when it exists if source is videoid', () => {
return getYt({'data-videoid': EXAMPLE_VIDEOID}, true, function(yt) {
const iframe = yt.querySelector('iframe');
Expand Down

0 comments on commit 9cd79a9

Please sign in to comment.