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

✨ Bento amp-video-iframe #31055

Merged
merged 30 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1bfb403
✨ Bento amp-video-iframe
alanorozco Nov 9, 2020
5ca54e7
Props are specified per component
alanorozco Nov 9, 2020
56c427c
intersection
alanorozco Nov 10, 2020
c22e6ff
dep-check
alanorozco Nov 10, 2020
8cf5d0b
review
alanorozco Nov 19, 2020
69e719e
resolve window
alanorozco Nov 19, 2020
462bfc6
Merge branch 'main' of github.com:ampproject/amphtml into amp-video-i…
alanorozco Apr 22, 2021
62b0d2d
update/review
alanorozco Apr 22, 2021
881d744
optional chaining
alanorozco Apr 22, 2021
e59f3bc
experiment
alanorozco Apr 22, 2021
c32d64e
test
alanorozco Apr 22, 2021
a4db13c
local url
alanorozco Apr 22, 2021
5bf4d3a
Merge branch 'main' of github.com:ampproject/amphtml into amp-video-i…
alanorozco Apr 27, 2021
b0e1046
requires css
alanorozco Apr 27, 2021
8ab71b8
children is a prop
alanorozco Apr 27, 2021
f0e4fc2
VideoIframeWrapper
alanorozco Apr 27, 2021
7af564b
extensions/amp-video-iframe/1.0/component.js
alanorozco Apr 27, 2021
8898fd2
Merge branch 'main' of github.com:ampproject/amphtml into amp-video-i…
alanorozco Apr 29, 2021
30bd4b1
named function
alanorozco Apr 29, 2021
da6862f
add missing css file
alanorozco Apr 29, 2021
1a72b4e
use util
alanorozco Apr 29, 2021
a292b1a
update component hierarchy
alanorozco Apr 29, 2021
b3bb751
dep-check-config
alanorozco Apr 29, 2021
7c2dc3a
remove unused
alanorozco Apr 29, 2021
b51023c
alphabetical
alanorozco Apr 29, 2021
83d4745
Merge branch 'main' of github.com:ampproject/amphtml into amp-video-i…
alanorozco May 3, 2021
8b95817
Sources should be part of base-element
alanorozco May 3, 2021
33dd37a
Revert to props in base element so we can take <source> by default
alanorozco May 3, 2021
e3b341f
share definitions
alanorozco May 3, 2021
806a2f4
correct dep-check-config
alanorozco May 3, 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
6 changes: 6 additions & 0 deletions build-system/compile/bundles.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,12 @@ exports.extensionBundles = [
latestVersion: '0.1',
type: TYPES.MEDIA,
},
{
name: 'amp-video-iframe',
version: '1.0',
latestVersion: '0.1',
type: TYPES.MEDIA,
},
{
name: 'amp-viqeo-player',
version: '0.1',
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-video-iframe/0.1/amp-video-iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const SANDBOX = [
* Events allowed to be dispatched from messages.
* @private @const
*/
const ALLOWED_EVENTS = [
export const BUBBLE_MESSAGE_EVENTS = [
VideoEvents.PLAYING,
VideoEvents.PAUSE,
VideoEvents.ENDED,
Expand Down Expand Up @@ -326,7 +326,7 @@ class AmpVideoIframe extends AMP.BaseElement {
return;
}

if (ALLOWED_EVENTS.indexOf(eventReceived) > -1) {
if (BUBBLE_MESSAGE_EVENTS.indexOf(eventReceived) > -1) {
this.element.dispatchCustomEvent(eventReceived);
return;
}
Expand Down
93 changes: 93 additions & 0 deletions extensions/amp-video-iframe/1.0/amp-video-iframe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright 2020 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {BUBBLE_MESSAGE_EVENTS} from '../0.1/amp-video-iframe';
import {VideoBaseElement} from '../../amp-video/1.0/base-element';
import {VideoIframe} from '../../amp-video/1.0/video-iframe';
import {createCustomEvent} from '../../../src/event-helper';
import {dict} from '../../../src/utils/object';

/** @const {string} */
const TAG = 'amp-video-iframe';

class AmpVideoIframe extends VideoBaseElement {}

/**
* @param {!MessageEvent} e
*/
const onMessage = (e) => {
const method = e.data?.method;
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
if (method) {
if (method == 'getIntersection') {
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
// TODO
return;
}
throw new Error(`Unknown method ${method}`);
}
const event = e.data?.event;
if (!event) {
return;
}
if (event === 'analytics') {
// TODO
return;
}
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
if (event === 'error') {
// TODO
return;
}
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
if (BUBBLE_MESSAGE_EVENTS.indexOf(event) > -1) {
e.currentTarget.dispatchEvent(
createCustomEvent(window, event, /* detail */ null, {
bubbles: true,
cancelable: true,
})
);
return;
}
};

/**
* @param {string} method
* @return {method}
alanorozco marked this conversation as resolved.
Show resolved Hide resolved
*/
const makeMethodMessage = (method) =>
JSON.stringify({
'event': 'method',
'method': method.toLowerCase(),
});

AmpVideoIframe['staticProps'] = dict({
'component': VideoIframe,
'onMessage': onMessage,
'makeMethodMessage': makeMethodMessage,
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't need a specific component, but only specific props.

This can be the case for 3p players like amp-youtube too, where we can specify its behavior only by changing props.

In a future world where CE get compiled away, for other components this can be expressed in one of two ways.

From:

<amp-youtube ...></amp-youtube>

Into:

A.

<Youtube ... />

or B.

<VideoIframe makeMethodMessage={YOUTUBE_MAKE_METHOD_MESSAGE} ... />

This is irrelevant for this PR, but it might be interesting to note.

});

/** @override */
AmpVideoIframe['props'] = {
'autoplay': {attr: 'autoplay', type: 'boolean'},
'referrerpolicy': {attr: 'referrerpolicy'},
'implements-media-session': {attr: 'mediasession', type: 'boolean'},
'poster': {attr: 'poster'},
'src': {attr: 'src'},

// TODO(alanorozco): These props have no internal implementation yet.
'dock': {attr: 'dock'},
'rotate-to-fullscreen': {attr: 'rotate-to-fullscreen', type: 'boolean'},
};

AMP.extension(TAG, '1.0', (AMP) => {
AMP.registerElement(TAG, AmpVideoIframe);
});
136 changes: 136 additions & 0 deletions extensions/amp-video-iframe/1.0/storybook/Basic.amp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/**
* Copyright 2020 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as Preact from '../../../../src/preact';
import {boolean, number, text, withKnobs} from '@storybook/addon-knobs';
import {withA11y} from '@storybook/addon-a11y';
import {withAmp} from '@ampproject/storybook-addon';

export default {
title: 'amp-video-iframe-1_0',
decorators: [withA11y, withKnobs, withAmp],
parameters: {extensions: [{name: 'amp-video-iframe', version: '1.0'}]},
};

const AmpVideoIframeWithKnobs = ({i, ...rest}) => {
const group = i ? `Player ${i + 1}` : undefined;

const width = text('width', '640px', group);
const height = text('height', '360px', group);

const ariaLabel = text('aria-label', 'Video Player');
const autoplay = boolean('autoplay', true);
const controls = boolean('controls', true);
const mediasession = boolean('mediasession', true);
const noaudio = boolean('noaudio', false);
const loop = boolean('loop', false);
const poster = text(
'poster',
'https://amp.dev/static/samples/img/amp-video-iframe-sample-placeholder.jpg'
);

const artist = text('artist', '');
const album = text('album', '');
const artwork = text('artwork', '');
const title = text('title', '');

const src = text(
'src',
'https://amp.dev/static/samples/files/amp-video-iframe-videojs.html'
);

return (
<amp-video-iframe
{...rest}
ariaLabel={ariaLabel}
autoplay={autoplay}
controls={controls}
mediasession={mediasession}
noaudio={noaudio}
loop={loop}
poster={poster}
artist={artist}
album={album}
artwork={artwork}
title={title}
layout="responsive"
width={width}
height={height}
src={src}
></amp-video-iframe>
);
};

const Spacer = ({height}) => {
return (
<div
style={{
height,
background: `linear-gradient(to bottom, #bbb, #bbb 10%, #fff 10%, #fff)`,
backgroundSize: '100% 10px',
}}
></div>
);
};

export const Default = () => {
const amount = number('Amount', 1, {}, 'Page');
const spacerHeight = text('Space', '80vh', 'Page');
const spaceAbove = boolean('Space above', false, 'Page');
const spaceBelow = boolean('Space below', false, 'Page');

const players = [];
for (let i = 0; i < amount; i++) {
players.push(<AmpVideoIframeWithKnobs key={i} i={i} />);
if (i < amount - 1) {
players.push(<Spacer height={spacerHeight} />);
}
}

return (
<>
{spaceAbove && <Spacer height={spacerHeight} />}
{players}
{spaceBelow && <Spacer height={spacerHeight} />}
</>
);
};

const ActionButton = ({children, ...props}) => (
<button style={{flex: 1, margin: '0 4px'}} {...props}>
{children}
</button>
);

export const Actions = () => {
return (
<div style="max-width: 800px">
<AmpVideoIframeWithKnobs id="player" />
<div
style={{
margin: '12px 0',
display: 'flex',
}}
>
<ActionButton on="tap:player.play">Play</ActionButton>
<ActionButton on="tap:player.pause">Pause</ActionButton>
<ActionButton on="tap:player.mute">Mute</ActionButton>
<ActionButton on="tap:player.unmute">Unmute</ActionButton>
<ActionButton on="tap:player.fullscreen">Fullscreen</ActionButton>
</div>
</div>
);
};
23 changes: 23 additions & 0 deletions extensions/amp-video/1.0/amp-video.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,29 @@ const TAG = 'amp-video';

class AmpVideo extends VideoBaseElement {}

/** @override */
AmpVideo['props'] = {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved since props vary by component, so I think they should be specified individually.

'album': {attr: 'album'},
'alt': {attr: 'alt'},
'artist': {attr: 'artist'},
'artwork': {attr: 'artwork'},
'attribution': {attr: 'attribution'},
'autoplay': {attr: 'autoplay', type: 'boolean'},
'controls': {attr: 'controls', type: 'boolean'},
'controlslist': {attr: 'controlslist'},
'crossorigin': {attr: 'crossorigin'},
'disableremoteplayback': {attr: 'disableremoteplayback'},
'loop': {attr: 'loop', type: 'boolean'},
'noaudio': {attr: 'noaudio', type: 'boolean'},
'poster': {attr: 'poster'},
'src': {attr: 'src'},
'title': {attr: 'title'},

// TODO(alanorozco): These props have no internal implementation yet.
'dock': {attr: 'dock'},
'rotate-to-fullscreen': {attr: 'rotate-to-fullscreen', type: 'boolean'},
};

AMP.extension(TAG, '1.0', (AMP) => {
AMP.registerElement(TAG, AmpVideo);
});
1 change: 1 addition & 0 deletions extensions/amp-video/1.0/autoplay.jss.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const autoplayMaskButton = {
appearance: 'none',
background: 'transparent',
border: 'none',
width: '100%',
};

const JSS = {
Expand Down
23 changes: 0 additions & 23 deletions extensions/amp-video/1.0/base-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,29 +83,6 @@ VideoBaseElement['layoutSizeDefined'] = true;
*/
VideoBaseElement['staticProps'];

/** @override */
VideoBaseElement['props'] = {
'album': {attr: 'album'},
'alt': {attr: 'alt'},
'artist': {attr: 'artist'},
'artwork': {attr: 'artwork'},
'attribution': {attr: 'attribution'},
'autoplay': {attr: 'autoplay', type: 'boolean'},
'controls': {attr: 'controls', type: 'boolean'},
'controlslist': {attr: 'controlslist'},
'crossorigin': {attr: 'crossorigin'},
'disableremoteplayback': {attr: 'disableremoteplayback'},
'loop': {attr: 'loop', type: 'boolean'},
'noaudio': {attr: 'noaudio', type: 'boolean'},
'poster': {attr: 'poster'},
'src': {attr: 'src'},
'title': {attr: 'title'},

// TODO(alanorozco): These props have no internal implementation yet.
'dock': {attr: 'dock'},
'rotate-to-fullscreen': {attr: 'rotate-to-fullscreen', type: 'boolean'},
};

/** @override */
VideoBaseElement['children'] = {
'sources': {
Expand Down