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

♻️ Consolidate players as .i-amphtml-media-component #26967

Merged
merged 3 commits into from
Feb 26, 2020
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
2 changes: 1 addition & 1 deletion css/video-autoplay.css
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ i-amphtml-video-mask {
}

amp-video[controls] .amp-video-eq,
.i-amphtml-video-component:not(amp-video) .amp-video-eq,
.i-amphtml-video-interface:not(amp-video) .amp-video-eq,
amp-story .amp-video-eq {
display: flex;
}
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-audio/0.1/amp-audio.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {closestAncestorElementBySelector} from '../../../src/dom';
import {dev, user} from '../../../src/log';
import {getMode} from '../../../src/mode';
import {listen} from '../../../src/event-helper';
import {setIsMediaComponent} from '../../../src/video-interface';
import {triggerAnalyticsEvent} from '../../../src/analytics';

const TAG = 'amp-audio';
Expand Down Expand Up @@ -63,6 +64,8 @@ export class AmpAudio extends AMP.BaseElement {
this.buildAudioElement();
}

setIsMediaComponent(this.element);

this.registerAction('play', this.play_.bind(this));
this.registerAction('pause', this.pause_.bind(this));
}
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-connatix-player/0.1/amp-connatix-player.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {dict} from '../../../src/utils/object';
import {getData} from '../../../src/event-helper';
import {isLayoutSizeDefined} from '../../../src/layout';
import {removeElement} from '../../../src/dom';
import {setIsMediaComponent} from '../../../src/video-interface';
import {userAssert} from '../../../src/log';

export class AmpConnatixPlayer extends AMP.BaseElement {
Expand Down Expand Up @@ -93,6 +94,8 @@ export class AmpConnatixPlayer extends AMP.BaseElement {
buildCallback() {
const {element} = this;

setIsMediaComponent(element);

// Player id is mandatory
this.playerId_ = userAssert(
element.getAttribute('data-player-id'),
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-hulu/0.1/amp-hulu.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {Services} from '../../../src/services';
import {devAssert, userAssert} from '../../../src/log';
import {isLayoutSizeDefined} from '../../../src/layout';
import {removeElement} from '../../../src/dom';
import {setIsMediaComponent} from '../../../src/video-interface';

class AmpHulu extends AMP.BaseElement {
/** @param {!AmpElement} element */
Expand Down Expand Up @@ -74,6 +75,8 @@ class AmpHulu extends AMP.BaseElement {

/** @override */
buildCallback() {
setIsMediaComponent(this.element);

this.eid_ = userAssert(
this.element.getAttribute('data-eid'),
'The data-eid attribute is required for <amp-hulu> %s',
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-izlesene/0.1/amp-izlesene.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {devAssert, userAssert} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {getDataParamsFromAttributes} from '../../../src/dom';
import {isLayoutSizeDefined} from '../../../src/layout';
import {setIsMediaComponent} from '../../../src/video-interface';

class AmpIzlesene extends AMP.BaseElement {
/**
Expand Down Expand Up @@ -60,6 +61,8 @@ class AmpIzlesene extends AMP.BaseElement {

/** @override */
buildCallback() {
setIsMediaComponent(this.element);

this.videoid_ = userAssert(
this.element.getAttribute('data-videoid'),
'The data-videoid attribute is required for <amp-izlesene> %s',
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-jwplayer/0.1/amp-jwplayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {addParamsToUrl} from '../../../src/url';
import {dict} from '../../../src/utils/object';
import {isLayoutSizeDefined} from '../../../src/layout';
import {removeElement} from '../../../src/dom';
import {setIsMediaComponent} from '../../../src/video-interface';
import {userAssert} from '../../../src/log';

class AmpJWPlayer extends AMP.BaseElement {
Expand Down Expand Up @@ -74,6 +75,8 @@ class AmpJWPlayer extends AMP.BaseElement {

/** @override */
buildCallback() {
setIsMediaComponent(this.element);

const {element} = this;
this.contentid_ = userAssert(
element.getAttribute('data-playlist-id') ||
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-kaltura-player/0.1/amp-kaltura-player.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {addParamsToUrl} from '../../../src/url';
import {dict} from '../../../src/utils/object';
import {getDataParamsFromAttributes} from '../../../src/dom';
import {isLayoutSizeDefined} from '../../../src/layout';
import {setIsMediaComponent} from '../../../src/video-interface';
import {userAssert} from '../../../src/log';

class AmpKaltura extends AMP.BaseElement {
Expand Down Expand Up @@ -61,6 +62,8 @@ class AmpKaltura extends AMP.BaseElement {
this.element
);

setIsMediaComponent(this.element);

this.entryId_ = this.element.getAttribute('data-entryid') || 'default';
}

Expand Down
2 changes: 2 additions & 0 deletions extensions/amp-megaphone/0.1/amp-megaphone.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {getData, listen} from '../../../src/event-helper';
import {isLayoutSizeFixed} from '../../../src/layout';
import {isObject} from '../../../src/types';
import {removeElement} from '../../../src/dom';
import {setIsMediaComponent} from '../../../src/video-interface';
import {startsWith} from '../../../src/string';
import {tryParseJson} from '../../../src/json';
import {userAssert} from '../../../src/log';
Expand Down Expand Up @@ -78,6 +79,7 @@ class AmpMegaphone extends AMP.BaseElement {

/** @override */
buildCallback() {
setIsMediaComponent(this.element);
this.updateBaseUrl_();
}

Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-o2-player/0.1/amp-o2-player.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {Services} from '../../../src/services';
import {dict} from '../../../src/utils/object';
import {isLayoutSizeDefined} from '../../../src/layout';
import {setIsMediaComponent} from '../../../src/video-interface';
import {userAssert} from '../../../src/log';

class AmpO2Player extends AMP.BaseElement {
Expand Down Expand Up @@ -59,6 +60,8 @@ class AmpO2Player extends AMP.BaseElement {

/** @override */
buildCallback() {
setIsMediaComponent(this.element);

this.pid_ = userAssert(
this.element.getAttribute('data-pid'),
'data-pid attribute is required for <amp-o2-player> %s',
Expand Down
6 changes: 6 additions & 0 deletions extensions/amp-reach-player/0.1/amp-reach-player.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import {Services} from '../../../src/services';
import {isLayoutSizeDefined} from '../../../src/layout';
import {setIsMediaComponent} from '../../../src/video-interface';

class AmpReachPlayer extends AMP.BaseElement {
/** @param {!AmpElement} element */
Expand Down Expand Up @@ -43,6 +44,11 @@ class AmpReachPlayer extends AMP.BaseElement {
return isLayoutSizeDefined(layout);
}

/** @override */
buildCallback() {
setIsMediaComponent(this.element);
}

/** @override */
layoutCallback() {
const embedId = this.element.getAttribute('data-embed-id') || 'default';
Expand Down
6 changes: 6 additions & 0 deletions extensions/amp-soundcloud/0.1/amp-soundcloud.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import {Services} from '../../../src/services';
import {dict} from '../../../src/utils/object';
import {isLayoutSizeDefined} from '../../../src/layout';
import {setIsMediaComponent} from '../../../src/video-interface';
import {userAssert} from '../../../src/log';

class AmpSoundcloud extends AMP.BaseElement {
Expand Down Expand Up @@ -58,6 +59,11 @@ class AmpSoundcloud extends AMP.BaseElement {
return isLayoutSizeDefined(layout);
}

/** @override */
buildCallback() {
setIsMediaComponent(this.element);
}

/**@override*/
layoutCallback() {
const height = this.element.getAttribute('height');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import {Services} from '../../../src/services';
import {isLayoutSizeDefined} from '../../../src/layout';
import {setIsMediaComponent} from '../../../src/video-interface';
import {userAssert} from '../../../src/log';

class AmpSpringboardPlayer extends AMP.BaseElement {
Expand Down Expand Up @@ -66,6 +67,8 @@ class AmpSpringboardPlayer extends AMP.BaseElement {

/** @override */
buildCallback() {
setIsMediaComponent(this.element);

this.mode_ = userAssert(
this.element.getAttribute('data-mode'),
'The data-mode attribute is required for <amp-springboard-player> %s',
Expand Down
3 changes: 2 additions & 1 deletion src/service/video-manager-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
VideoAttributes,
VideoEvents,
VideoServiceSignals,
setIsMediaComponent,
userInteractedWith,
videoAnalyticsCustomEventTypeKey,
} from '../video-interface';
Expand Down Expand Up @@ -192,7 +193,7 @@ export class VideoManager {
const {element} = entry.video;
element.dispatchCustomEvent(VideoEvents.REGISTERED);

element.classList.add('i-amphtml-video-component');
setIsMediaComponent(element);

// Unlike events, signals are permanent. We can wait for `REGISTERED` at any
// moment in the element's lifecycle and the promise will resolve
Expand Down
23 changes: 23 additions & 0 deletions src/video-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,26 @@ export function delegateAutoplay(video) {
export function userInteractedWith(video) {
video.signals().signal(VideoServiceSignals.USER_INTERACTED);
}

/**
* Classname that media components should annotate themselves with.
* This applies to all video and audio playback components, regardless of
* whether they implement a common interface or not.
*
* TODO(go.amp.dev/issue/26984): This isn't exclusive to video, but there's no
* better place to put this now due to OWNERShip. Move.
*/
export const MEDIA_COMPONENT_CLASSNAME = 'i-amphtml-media-component';

/**
* Annotates media component element with a common classname.
* This applies to all video and audio playback components, regardless of
* whether they implement a common interface or not.
* @param {!Element} element
*
* TODO(go.amp.dev/issue/26984): This isn't exclusive to video, but there's no
* better place to put this now due to OWNERShip. Move.
*/
export function setIsMediaComponent(element) {
element.classList.add(MEDIA_COMPONENT_CLASSNAME);
}
6 changes: 3 additions & 3 deletions test/visual-diff/visual-tests
Original file line number Diff line number Diff line change
Expand Up @@ -772,21 +772,21 @@
"name": "amp-video-docking (left-to-right)",
"viewport": {"width": 800, "height": 600},
"interactive_tests": "examples/visual-tests/amp-video-docking/amp-video-docking.js",
"loading_complete_selectors": [".i-amphtml-video-component"],
"loading_complete_selectors": [".i-amphtml-video-interface"],
},
{
"url": "examples/visual-tests/amp-video-docking/rtl.html",
"name": "amp-video-docking (right-to-left)",
"viewport": {"width": 800, "height": 600},
"interactive_tests": "examples/visual-tests/amp-video-docking/amp-video-docking.js",
"loading_complete_selectors": [".i-amphtml-video-component"],
"loading_complete_selectors": [".i-amphtml-video-interface"],
},
{
"url": "examples/visual-tests/amp-video-docking/slot.html",
"name": "amp-video-docking (slot)",
"viewport": {"width": 800, "height": 600},
"interactive_tests": "examples/visual-tests/amp-video-docking/amp-video-docking.js",
"loading_complete_selectors": [".i-amphtml-video-component"],
"loading_complete_selectors": [".i-amphtml-video-interface"],
},
]
}