From 333340b008bcf3510e015a0a077b97311ff8df7c Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:18:54 -0700
Subject: [PATCH 01/19] =?UTF-8?q?=F0=9F=93=96=20Guide=20to=20Building=20a?=
 =?UTF-8?q?=20Bento=20Video=20Player?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../building-a-bento-amp-extension.md         |   3 +-
 contributing/building-a-bento-video-player.md | 609 ++++++++++++++++++
 2 files changed, 611 insertions(+), 1 deletion(-)
 create mode 100644 contributing/building-a-bento-video-player.md

diff --git a/contributing/building-a-bento-amp-extension.md b/contributing/building-a-bento-amp-extension.md
index dc14a7b024f2..3d5d807d9c87 100644
--- a/contributing/building-a-bento-amp-extension.md
+++ b/contributing/building-a-bento-amp-extension.md
@@ -670,8 +670,9 @@ $ amp check-types
     -   [amp-timeago](https://github.com/ampproject/amphtml/pull/26507)
     -   [amp-base-carousel](https://github.com/ampproject/amphtml/pull/29303)
     -   [amp-stream-gallery](https://github.com/ampproject/amphtml/pull/30597)
--   Adding video components which use `VideoWrapper`
+-   Adding video components which use `VideoWrapper`. [You may also follow the guide to Building a Bento Video Component.](./building-a-bento-video-player.md)
     -   [amp-video](https://github.com/ampproject/amphtml/pull/30280)
     -   [amp-youtube](https://github.com/ampproject/amphtml/pull/30444)
+    -   [amp-vimeo](https://github.com/ampproject/amphtml/pull/33971)
 -   Adding iframe based embeds
     -   [amp-instagram](https://github.com/ampproject/amphtml/pull/30230)
diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
new file mode 100644
index 000000000000..bdbc444d5e04
--- /dev/null
+++ b/contributing/building-a-bento-video-player.md
@@ -0,0 +1,609 @@
+# Building a Bento Video Player
+
+> **You should first read through the [guide to Building a Bento AMP Component](./building-a-bento-amp-extension.md).** Do not follow the steps to generate an extension, since they're specified here. Once you're familiar with the concepts related to AMP extensions and Bento components, follow this guide instead.
+
+<!--
+  (Do not remove or edit this comment.)
+
+  This table-of-contents is automatically generated. To generate it, run:
+    amp markdown-toc --fix
+-->
+
+<!-- {"maxdepth": 3} -->
+
+-   [How Video Player Components Work](#how-video-player-components-work)
+-   [Getting Started](#getting-started)
+-   [Directory Structure](#directory-structure)
+-   [Extend `VideoBaseElement` for AMP](#extend-videobaseelement-for-amp)
+    -   [Pre-upgrade CSS](#pre-upgrade-css)
+    -   [`props`](#props)
+-   [Define a Preact component](#define-a-preact-component)
+    -   [Forwarding `ref`](#forwarding-ref)
+    -   [Loading an iframe with `VideoIframe`](#loading-an-iframe-with-videoiframe)
+        -   [`src`](#src)
+        -   [`origin`](#origin)
+        -   [Playback methods with `makeMethodMessage`](#playback-methods-with-makemethodmessage)
+        -   [Handling events with `onMessage`](#handling-events-with-onmessage)
+    -   [Use `VideoWrapper` directly](#use-videowrapper-directly)
+        -   [Specifying `component`](#specifying-component)
+        -   [Passing or overriding props](#passing-or-overriding-props)
+        -   [Imperative handle](#imperative-handle)
+-   [Completing your extension](#completing-your-extension)
+-   [Example Pull Requests](#example-pull-requests)
+
+## How Video Player Components Work
+
+AMP and Bento provide [default video player capabilities](https://amp.dev/documentation/guides-and-tutorials/learn/spec/amp-video-interface/) in order to create a uniform experience. For example, videos only autoplay while they're visible and muted, and they consistently unmute when clicked. They send the same event signals to [`amp-analytics`](https://go.amp.dev/c/amp-analytics), or may be pinned to a corner through the same User Experience in combination with [`amp-video-docking`](https://go.amp.dev/c/amp-video-docking).
+
+On a host document, player components must dispatch the same events and implement the same methods so that playback and user interface can be coordinated successfully.
+
+**Preact components** can support this behavior by using a **`VideoWrapper`** that will use a specified `component`:
+
+```js
+return <VideoWrapper component="video" {...props} />
+```
+
+The `component` prop can be a string to specify a `<video>` element or an id reference to a component whose interface is similar to [`HTMLMediaElement`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement).
+
+However, most video players are embedded through an iframe so they should use **`VideoIframe`** instead. This is a specialized `VideoWrapper` that doesn't require an underlying `component`:
+
+```js
+return <VideoIframe {...props}>
+```
+
+To enable component support for **AMP documents**, our video player element must extend from a base class `VideoBaseElement`. This enables [actions](https://amp.dev/documentation/guides-and-tutorials/learn/amp-actions-and-events/#amp-video-and-other-video-elements_1) and [analytics](https://github.com/ampproject/amphtml/blob/main/extensions/amp-analytics/amp-video-analytics.md), and allows us to define further behavior specific to the AMP layer, like parsing element attributes into Preact props.
+
+This guide covers how to implement video player components that are internally implemented using these Preact and AMP components.
+
+## Getting Started
+
+Start by generating an extension specifying `--bento` and `--nojss`. We name our extension **`amp-fantastic-player`**, ending with `-player` according to our [guidelines for naming a third-party component](../spec/amp-3p-naming.md).
+
+```shell
+amp make-extension --bento --nojss --name=amp-fantastic-player
+```
+
+## Directory Structure
+
+A [full directory for a Bento component](./building-a-bento-amp-extension.md#directory-structure) is generated, but this guide will cover the following files in particular:
+
+```shell
+/extensions/amp-fantastic-player/1.0/
+ ├── base-element.js               # Preact base element
+ ├── component.js                  # Preact implementation
+ ├── amp-my-fantastic-player.js    # Element's implementation
+ └── amp-my-fantastic-player.css   # Custom CSS
+```
+
+## Extend `VideoBaseElement` for AMP
+
+Our `BaseElement` should be a superclass of `VideoBaseElement`. In **`base-element.js`**, we change:
+
+```diff
+  import {MyFantasticPlayer} from './component';
+- import {PreactBaseElement} from '../../../src/preact/base-element';
++ import {VideoBaseElement} from '../../amp-video/1.0/base-element';
+
+- export class BaseElement extends PreactBaseElement {}
++ export class BaseElement extends VideoBaseElement {}
+```
+
+into:
+
+```js
+import {MyFantasticPlayer} from './component';
+import {VideoBaseElement} from '../../amp-video/1.0/base-element';
+
+export class BaseElement extends VideoBaseElement {}
+```
+
+This enables support for AMP actions and analytics, once we map attributes to their prop counterparts in `BaseElement['props']`, and we implement the Preact component.
+
+**Adding this import statement will cause CI to fail**. You should solve this by explicitly allowing the cross-extension import, which is ok for this target file. We add to **`build-system/test-configs/dep-check-config.js`**:
+
+```diff
+{
+  // Extensions can't depend on other extensions.
+  filesMatching: 'extensions/**/*.js',
+  mustNotDependOn: 'extensions/**/*.js',
+  allowlist: [
+    ...
++   // Bento MyFantasticPlayer, amp-my-fantastic-player
++   'extensions/amp-my-fantastic-player/1.0/base-element.js->extensions/amp-video/1.0/base-element.js',
+  ],
+},
+```
+
+### Pre-upgrade CSS
+
+Bento components must specify certain layout properties in order to prevent CLS. Video extensions must include the following in the generated **`amp-fantastic-player.css`**:
+
+```css
+/*
+ * Pre-upgrade:
+ * - display:block element
+ * - size-defined element
+ */
+amp-fantastic-player {
+  display: block;
+  overflow: hidden;
+  position: relative;
+}
+
+/* Pre-upgrade: size-defining element - hide children. */
+amp-fantastic-player:not(.i-amphtml-built) > :not([placeholder]):not(.i-amphtml-svc) {
+  display: none;
+  content-visibility: hidden;
+}
+```
+
+### `props`
+
+[**`props`**](https://github.com/ampproject/amphtml/blob/main/contributing/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`VideoBaseElement`](extensions/amp-video/1.0/base-element.js) for how most video properties are mapped. On your own `base-element.js`, you should specify any of them you support.
+
+```js
+/** @override */
+BaseElement['props'] = {
+  'autoplay': {attr: 'autoplay', type: 'boolean'},
+};
+```
+
+## Define a Preact component
+
+If you need to directly insert nodes to the document, like a `<video>` element, you need to use a `<VideoWrapper>`.
+
+However, it's more likely that you load a third-party iframe and you communicate with the host via `postMessage`. In this case you should use a `<VideoIframe>` as opposed to a `<VideoWrapper>`.
+
+> Components may **not** embed scripts from a third-party location into host documents. If a third-party script is absolutely required, like on `<amp-ima-video>`, it must be inserted in an intermediate iframe, which we call a **proxy frame**.
+>
+> Proxy frames on Bento have not yet been tested as video player components, so they're not covered in this guide. If you wish to use one, please get in touch with `@alanorozco` via a Github issue or on Slack.
+
+### Forwarding `ref`
+
+To enable AMP actions (`my-element.play`) and the Preact component's imperative handle (`myPlayerRef.current.play()`), you'll have to [`forwardRef`](https://preactjs.com/guide/v10/switching-to-preact/#forwardref). Rename `FantasticPlayer` to `FantasticPlayerWithRef`, and export a `FantasticPlayer` that forwards a `ref` into the former.
+
+```diff
++ import {forwardRef} from '../../../src/preact/compat';
+
+- export function FantasticPlayer({...rest}) {
++ function FantasticPlayerWithRef({...rest}, ref) {
+    ...
+  }
+
++ const FantasticPlayer = forwardRef(FantasticPlayerWithRef);
++ FantasticPlayer.displayName = 'FantasticPlayer'; // Make findable for tests.
++ export {FantasticPlayer};
+```
+
+So the outer structure looks like:
+
+```js
+// ...
+import {forwardRef} from '../../../src/preact/compat';
+
+// ...
+function FantasticPlayerWithRef({...rest}, ref) {
+  // ...
+}
+
+//...
+const FantasticPlayer = forwardRef(FantasticPlayerWithRef);
+FantasticPlayer.displayName = 'FantasticPlayer'; // Make findable for tests.
+export {FantasticPlayer};
+```
+
+### Loading an iframe with `VideoIframe`
+
+Your `FantasticPlayer` component should return a `VideoIframe` that's configured to a corresponding `postMessage` API. To start, we update the implementation in **`component.js`**:
+
+```diff
+- import {ContainWrapper} from '../../../src/preact/component';
++ import {VideoIframe} from '../../amp-video/1.0/video-iframe';
+
+  function FantasticPlayerWithRef({...rest}, ref) {
+-   ...
++   const src = useMemo(
++     () => 'https://example.com/fantastic',
++     []
++   );
++   const makeMethodMessage = useCallback(() => '{}', []);
++   const onMessage = useCallback((e) => {
++     console.log(e);
++   }, []);
+    return (
+-     <ContainWrapper layout size paint {...rest} >
+-       ...
+-     </ContainWrapper>
++     <VideoIframe
++       {...rest}
++       src={src}
++       makeMethodMessage={makeMethodMessage}
++       onMessage={onMessage}
++     />
+    );
+  }
+```
+
+into:
+
+```js
+import {VideoIframe} from '../../amp-video/1.0/video-iframe';
+
+// ...
+
+function FantasticPlayerWithRef({...rest}, ref) {
+  const src = useMemo(
+    () => 'https://example.com/fantastic',
+    []
+  );
+  const makeMethodMessage = useCallback(() => '{}', []);
+  const onMessage = useCallback((e) => {
+    console.log(e);
+  }, []);
+  return (
+    <VideoIframe
+      {...rest}
+      src={src}
+      makeMethodMessage={makeMethodMessage}
+      onMessage={onMessage}
+    />
+  );
+}
+```
+
+We're rendering an iframe that always loads `https://example.com/fantastic`, but we'll specify a dynamic URL later; hence `useMemo()`. Likewise, we'll need to define implementations for the communication functions `makeMethodMessage` and `onMessage`.
+
+You should add allow one more cross-extension import for `VideoIframe`. Following the previous location on **`dep-check-config.js`**, we add:
+
+```diff
+  allowlist: [
+    ...
+    // Bento MyFantasticPlayer, amp-my-fantastic-player
+    'extensions/amp-my-fantastic-player/1.0/base-element.js->extensions/amp-video/1.0/base-element.js',
++   'extensions/amp-my-fantastic-player/1.0/component.js->extensions/amp-video/1.0/video-iframe.js',
+  ],
+```
+
+#### `src`
+
+You may use props to construct the `src`, like using a `videoId` to load `https://example.com/fantastic/${videoId}/`.
+
+We employ the `useMemo()` hook so that the `src` is generated only when the `videoId` changes:
+
+```js
+function FantasticPlayerWithRef(
+  {videoId, ...rest},
+  ref
+) {
+  // ...
+  const src = useMemo(
+    () =>
+      `https://example.com/fantastic/${encodeURIComponent(videoId)}/`,
+    [videoId]
+  );
+  // ...
+  return (
+    <VideoIframe
+      {...rest}
+      src={src}
+      ...
+    />
+  );
+}
+```
+
+#### `origin`
+
+#### Playback methods with `makeMethodMessage`
+
+We need the ability to tell the iframe to execute certain actions, for example `play`, `mute` or `hideControls`. When using an iframe to load a player, this is done by sending a `postMessage` downstream.
+
+`makeMethodMessage` takes an action to execute as a `string`, and returns another `string` corresponding to a message to send.
+
+```ts
+type MakeMethodMessageFunction = (method: string) => string;
+```
+
+We implement this function with `useCallback()` so that it's created only when required as specified by hook dependencies. It's recommended that you also create a higher-level `makeMessage` function that creates and serializes messages as your iframe's interface needs it. In this case, we `JSON.stringify` a `videoId` and a `method`:
+
+```js
+function makeMessage(videoId, method) {
+  return JSON.stringify({
+    'videoId': videoId,
+    'method': method,
+  });
+}
+
+function FantasticPlayerWithRef(
+  {videoId, ...rest},
+  ref
+) {
+  // ...
+  const makeMethodMessage = useCallback(
+    (method) => makeMessage(videoId, method),
+    [videoId]
+  );
+
+  // ...
+  return (
+    <VideoIframe
+      {...rest}
+      makeMethodMessage={makeMethodMessage}
+      ...
+    />
+  );
+}
+```
+
+#### Handling events with `onMessage`
+
+```ts
+type OnMessageFunction = (event: MessageEvent) => void;
+```
+
+> **This is an incomplete [`MessageEvent`](https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent).** It's a copy that contains `currentTarget` (the originating `<iframe>`), `target` (same as `currentTarget`) and `data` (the data sent by the iframe with `postMessage`). If you need to copy other properties, add them to the appropriate place in [`video-iframe.js`](../extensions/amp-video/1.0/video-iframe.js) and add `@alanorozco` as a reviewer on your pull request.
+
+```js
+function onMessage(event) {
+  const {data, currentTarget} = event;
+  switch (data?.event) {
+    case 'canplay':
+    case 'play':
+    case 'pause':
+      dispatchCustomEvent(currentTarget, data.event, null, {
+        bubbles: true,
+        cancelable: false,
+      });
+      break;
+  }
+}
+
+function FantasticPlayerWithRef(
+  {videoId, ...rest},
+  ref
+) {
+  // ...
+  return (
+    <VideoIframe
+      {...rest}
+      onMessage={onMessage}
+      ...
+    />
+  );
+}
+```
+
+### Use `VideoWrapper` directly
+
+> **If you need an iframe, you should ignore this section and use `VideoIframe` instead.** It requires less work and likely provides all you need. Read on if you're sure that you need to write to a host document directly to create `<video>` elements, or otherwise manage frames manually (like creating proxy frames).
+
+Your `FantasticPlayer` component should return a `VideoWrapper` that's configured to a corresponding `postMessage` API. To start, we update the implementation in **`component.js`**.
+
+```diff
+- import {ContainWrapper} from '../../../src/preact/component';
++ import {VideoWrapper} from '../../amp-video/1.0/video-wrapper';
+
+  function FantasticPlayerWithRef({...rest}, ref) {
+-   ...
+    return (
+-     <ContainWrapper layout size paint {...rest} >
+-       ...
+-     </ContainWrapper>
++     <VideoWrapper component="video" />
+    );
+  }
+```
+
+So that our component returns a `<VideoWrapper>`:
+
+```js
+import {VideoWrapper} from '../../amp-video/1.0/video-wrapper';
+
+// ...
+function FantasticPlayerWithRef({...rest}, ref) {
+  return <VideoWrapper component="video" />;
+}
+```
+
+We're specifying `"video"` as the element to render, which is also the default. We'll llater change this into our own component implementation.
+
+You should add allow one more cross-extension import for `VideoWrapper`. Following the previous location on **`dep-check-config.js`**, we add:
+
+```diff
+  allowlist: [
+    ...
+    // Bento MyFantasticPlayer, amp-my-fantastic-player
+    'extensions/amp-my-fantastic-player/1.0/base-element.js->extensions/amp-video/1.0/base-element.js',
++   'extensions/amp-my-fantastic-player/1.0/component.js->extensions/amp-video/1.0/video-wrapper.js',
+  ],
+```
+
+#### Specifying `component`
+
+The `VideoWrapper` must interact with an element that implements the `HTMLMediaInterface` like `<video>`, or a Preact component that emulates the same interface.
+
+For example, we may set `component={RandomVideo}`, where the specified function renders its own video from a randomly selected `src`.
+
+By passing the `ref` through, we're able to call methods like `play()` from `FantasticPlayer` on the `<video>` element itself. By passing the `...rest` of the props we make sure that `src` is set, in addition to listening to playback events through props like `onPlay`.
+
+```js
+function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
+  return (
+    <div>
+      <video ref={ref} {...rest}>
+        {sources}
+      </video>
+    </div>
+  );
+}
+
+const FantasticPlayerInternal = forwardRef(FantasticPlayerInternalWithRef);
+
+// ...
+
+function FantasticPlayerWithRef({...rest}, ref) {
+  return <VideoWrapper {...rest} component={RandomVideo} />;
+}
+```
+
+#### Passing or overriding props
+
+In the previous example, props received from the `VideoWrapper` are implicitly set through `...rest`. If we set each explicitly, we see the `HTMLMediaInterface` attributes and events handled.
+
+```js
+function FantasticPlayerInternalWithRef(
+  {
+    muted,
+    loop,
+    controls,
+    onCanPlay,
+    onLoadedMetadata,
+    onPlaying,
+    onPause,
+    onEnded,
+    onError,
+    src,
+    poster,
+    style,
+    sources,
+  },
+  ref
+) {
+  return (
+    <div>
+      <video
+        ref={ref}
+        muted={muted}
+        loop={loop}
+        controls={controls}
+        onCanPlay={onCanPlay}
+        onLoadedMetadata={onLoadedMetadata}
+        onPlaying={onPlaying}
+        onPause={onPause}
+        onEnded={onEnded}
+        onError={onError}
+        src={src}
+        poster={poster}
+        style={style}
+      >
+        {sources}
+      </video>
+    </div>
+  );
+}
+```
+
+We can wrap playback events set on these props to dispatch them. For example, by wrapping `onCanPlay` we may mediate the `canplay` event by delaying it by 500 milliseconds:
+
+```js
+const onVideoCanPlay = useCallback((e) => {
+  setTimeout(() => {
+    onCanPlay(e);
+  }, 500);
+}, [onCanPlay]);
+```
+
+We set the wrapped method as the `<video>`'s actual event handler:
+
+```diff
+  <video
+    ref={ref}
+-   onCanPlay={onCanPlay}
++   onCanPlay={onVideoCanPlay}
+```
+
+You may similarly choose to pass or override properties at the higher level, passed from `FantasticPlayer` into the `VideoWrapper` we instantiate. For a list of these properties [see `video-wrapper.type.js`](../extensions/amp-video/1.0/video-wrapper.type.js)
+
+#### Imperative handle
+
+AMP actions execute methods on the Preact component because they `forwardRef` to an element that defines them, or because they define them with `useImperativeHandle`. Methods executed down the Preact component chain cascade the same way.
+
+When we click the following button on an AMP document:
+
+```html
+<button on="tap: my-fantastic-player.play">
+  Play
+</button>
+```
+
+We call the corresponding function `play`:
+
+```
+-> FantasticPlayer.play()
+```
+
+> The AMP action `my-element.play` is declared to be forwarded to the Preact component's method. See the [`init()` method on `VideoBaseElement`](../extensions/amp-video/1.0/base-element.js) for a list of the supported actions.
+
+Since we don't call `useImperativeHandle` at this layer, its `ref` forwards to `VideoWrapper`:
+
+```
+-> FantasticPlayer.play()
+-> (forwardRef) VideoWrapper.play()
+```
+
+`VideoWrapper` sets an imperative handle that explicitly calls `component.play()`:
+
+```
+-> FantasticPlayer.play()
+-> (forwardRef) VideoWrapper.play()
+  -> (imperativeHandle) component.play()
+```
+
+If `component` is a `<video>`, then the method is called direclty. If it's our `FantasticPlayerInternal` as we defined earlier, it may either `forwardRef` to a `<video>` or implement its own imperative handle.
+
+```
+-> FantasticPlayer.play()
+-> (forwardRef) VideoWrapper.play()
+  -> (imperativeHandle) component.play()
+  -> (forwardRef) <video>.play()
+```
+
+Methods can be defined with `useImperativeHandle` at the `component` implementation. We no longer forward our `ref` to the `<video>`, we use it to define the imperative handle instead. Downstream methods on the `<video>` element are executed explicitly through a local `videoRef`.
+
+```js
+function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
+  const videoRef = useRef(null);
+
+  useImperativeHandle(() => {
+    return {
+      play() {
+        videoRef.current.play();
+      },
+      pause() {
+        videoRef.current.pause();
+      },
+      requestFullscreen() {
+        videoRef.current.requestFullscreen();
+      },
+      get readyState() {
+        return videoRef.current.readyState;
+      },
+      get currentTime() {
+        return videoRef.current.currentTime;
+      },
+      get duration() {
+        return videoRef.current.duration;
+      },
+    }
+  }, []);
+
+  return (<video ref={videoRef} {...rest}>{sources}</video>)
+}
+```
+
+The methods and getters listed are the current requirements from `VideoWrapper`. Note that video players on Bento are in active development, so this list might expand in the future.
+
+## Completing your extension
+
+Follow the [guide to Building a Bento AMP Component](./building-a-bento-amp-extension.md) for other instructions that you should complete, including:
+
+-   **Documentation** that describes the component.
+-   **Tests** that verify the component's functionality.
+-   **Validator rules** to embed the component in an AMP document.
+-   **An example** to our Storybook or to be published on [amp.dev](https://amp.dev/)
+
+## Example Pull Requests
+
+-   [amp-video](https://github.com/ampproject/amphtml/pull/30280)
+-   [amp-youtube](https://github.com/ampproject/amphtml/pull/30444)
+-   [amp-vimeo](https://github.com/ampproject/amphtml/pull/33971)

From b67e38046c9d8cd961bfd0777d7b8b56b6c405b4 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:21:05 -0700
Subject: [PATCH 02/19] links

---
 contributing/building-a-bento-video-player.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index bdbc444d5e04..5a5fbcecc80a 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -160,7 +160,7 @@ However, it's more likely that you load a third-party iframe and you communicate
 
 ### Forwarding `ref`
 
-To enable AMP actions (`my-element.play`) and the Preact component's imperative handle (`myPlayerRef.current.play()`), you'll have to [`forwardRef`](https://preactjs.com/guide/v10/switching-to-preact/#forwardref). Rename `FantasticPlayer` to `FantasticPlayerWithRef`, and export a `FantasticPlayer` that forwards a `ref` into the former.
+To enable AMP actions (`my-element.play`) and the Preact component's imperative handle (`myPlayerRef.current.play()`), you'll have to [`forwardRef`](https://reactjs.org/docs/forwarding-refs.html). Rename `FantasticPlayer` to `FantasticPlayerWithRef`, and export a `FantasticPlayer` that forwards a `ref` into the former.
 
 ```diff
 + import {forwardRef} from '../../../src/preact/compat';
@@ -516,7 +516,7 @@ You may similarly choose to pass or override properties at the higher level, pas
 
 #### Imperative handle
 
-AMP actions execute methods on the Preact component because they `forwardRef` to an element that defines them, or because they define them with `useImperativeHandle`. Methods executed down the Preact component chain cascade the same way.
+AMP actions execute methods on the Preact component because they `forwardRef` to an element that defines them, or because they define them with [`useImperativeHandle`](https://reactjs.org/docs/hooks-reference.html#useimperativehandle). Methods executed down the Preact component chain cascade the same way.
 
 When we click the following button on an AMP document:
 

From 068dfef6c74bcb2aa8e8123f31eaf68257f5c6ef Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:23:13 -0700
Subject: [PATCH 03/19] link/acronym

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 5a5fbcecc80a..d1c03163a8c1 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -116,7 +116,7 @@ This enables support for AMP actions and analytics, once we map attributes to th
 
 ### Pre-upgrade CSS
 
-Bento components must specify certain layout properties in order to prevent CLS. Video extensions must include the following in the generated **`amp-fantastic-player.css`**:
+Bento components must specify certain layout properties in order to prevent [Cumulative Layout Shift (CLS)](https://web.dev/cls). Video extensions must include the following in the generated **`amp-fantastic-player.css`**:
 
 ```css
 /*

From 77873978286003295c3ad2496f0b391fb0d598cf Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:24:39 -0700
Subject: [PATCH 04/19] =?UTF-8?q?=E2=9A=A0=EF=B8=8F?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 contributing/building-a-bento-video-player.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index d1c03163a8c1..16fcf4ee9e64 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -154,7 +154,7 @@ If you need to directly insert nodes to the document, like a `<video>` element,
 
 However, it's more likely that you load a third-party iframe and you communicate with the host via `postMessage`. In this case you should use a `<VideoIframe>` as opposed to a `<VideoWrapper>`.
 
-> Components may **not** embed scripts from a third-party location into host documents. If a third-party script is absolutely required, like on `<amp-ima-video>`, it must be inserted in an intermediate iframe, which we call a **proxy frame**.
+> ⚠️ Components may **not** embed scripts from a third-party location into host documents. If a third-party script is absolutely required, like on `<amp-ima-video>`, it must be inserted in an intermediate iframe, which we call a **proxy frame**.
 >
 > Proxy frames on Bento have not yet been tested as video player components, so they're not covered in this guide. If you wish to use one, please get in touch with `@alanorozco` via a Github issue or on Slack.
 
@@ -341,7 +341,7 @@ function FantasticPlayerWithRef(
 type OnMessageFunction = (event: MessageEvent) => void;
 ```
 
-> **This is an incomplete [`MessageEvent`](https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent).** It's a copy that contains `currentTarget` (the originating `<iframe>`), `target` (same as `currentTarget`) and `data` (the data sent by the iframe with `postMessage`). If you need to copy other properties, add them to the appropriate place in [`video-iframe.js`](../extensions/amp-video/1.0/video-iframe.js) and add `@alanorozco` as a reviewer on your pull request.
+> ⚠️ **This is an incomplete [`MessageEvent`](https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent).** It's a copy that contains `currentTarget` (the originating `<iframe>`), `target` (same as `currentTarget`) and `data` (the data sent by the iframe with `postMessage`). If you need to copy other properties, add them to the appropriate place in [`video-iframe.js`](../extensions/amp-video/1.0/video-iframe.js) and add `@alanorozco` as a reviewer on your pull request.
 
 ```js
 function onMessage(event) {
@@ -560,6 +560,8 @@ If `component` is a `<video>`, then the method is called direclty. If it's our `
 
 Methods can be defined with `useImperativeHandle` at the `component` implementation. We no longer forward our `ref` to the `<video>`, we use it to define the imperative handle instead. Downstream methods on the `<video>` element are executed explicitly through a local `videoRef`.
 
+> The methods and getters listed are the current requirements from `VideoWrapper`. Note that video players on Bento are in active development, so this list might expand in the future.
+
 ```js
 function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
   const videoRef = useRef(null);
@@ -591,8 +593,6 @@ function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
 }
 ```
 
-The methods and getters listed are the current requirements from `VideoWrapper`. Note that video players on Bento are in active development, so this list might expand in the future.
-
 ## Completing your extension
 
 Follow the [guide to Building a Bento AMP Component](./building-a-bento-amp-extension.md) for other instructions that you should complete, including:

From c51e2173522f24f7aeeb080b09805d91f8e642c5 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:25:50 -0700
Subject: [PATCH 05/19] wording

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 16fcf4ee9e64..cdc76cbb912d 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -33,7 +33,7 @@
 
 ## How Video Player Components Work
 
-AMP and Bento provide [default video player capabilities](https://amp.dev/documentation/guides-and-tutorials/learn/spec/amp-video-interface/) in order to create a uniform experience. For example, videos only autoplay while they're visible and muted, and they consistently unmute when clicked. They send the same event signals to [`amp-analytics`](https://go.amp.dev/c/amp-analytics), or may be pinned to a corner through the same User Experience in combination with [`amp-video-docking`](https://go.amp.dev/c/amp-video-docking).
+AMP and Bento provide [default video player capabilities](https://amp.dev/documentation/guides-and-tutorials/learn/spec/amp-video-interface/) in order to create a uniform experience. For example, videos only autoplay while they're visible and muted, and they consistently unmute when clicked. They send the same event signals to [`amp-analytics`](https://go.amp.dev/c/amp-analytics), or may be pinned to a corner in the same way in combination with [`amp-video-docking`](https://go.amp.dev/c/amp-video-docking).
 
 On a host document, player components must dispatch the same events and implement the same methods so that playback and user interface can be coordinated successfully.
 

From 6b3ded6325911c7f154dda789ecfff6feaa08d5f Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:34:35 -0700
Subject: [PATCH 06/19] missing blurb for onMessage

---
 contributing/building-a-bento-video-player.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index cdc76cbb912d..38eb9166ac14 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -337,12 +337,16 @@ function FantasticPlayerWithRef(
 
 #### Handling events with `onMessage`
 
+Upstream events originated by the iframe are received as messages. You should define a function that interprets these messages and dispatches [`HTMLMediaElement` events](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement#events).
+
 ```ts
 type OnMessageFunction = (event: MessageEvent) => void;
 ```
 
 > ⚠️ **This is an incomplete [`MessageEvent`](https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent).** It's a copy that contains `currentTarget` (the originating `<iframe>`), `target` (same as `currentTarget`) and `data` (the data sent by the iframe with `postMessage`). If you need to copy other properties, add them to the appropriate place in [`video-iframe.js`](../extensions/amp-video/1.0/video-iframe.js) and add `@alanorozco` as a reviewer on your pull request.
 
+Here we implement the `canplay`, `play` and `pause` events for an iframe that posts them as the message `{"event": "play"}`.
+
 ```js
 function onMessage(event) {
   const {data, currentTarget} = event;
@@ -373,6 +377,8 @@ function FantasticPlayerWithRef(
 }
 ```
 
+The API that your iframe's document uses to post messages is likely different, but it should always dispatch [`HTMLMediaElement` events](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement#events) upstream.
+
 ### Use `VideoWrapper` directly
 
 > **If you need an iframe, you should ignore this section and use `VideoIframe` instead.** It requires less work and likely provides all you need. Read on if you're sure that you need to write to a host document directly to create `<video>` elements, or otherwise manage frames manually (like creating proxy frames).

From f0224619723dfa8f4f4344128526a0f9017e5ec5 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:35:20 -0700
Subject: [PATCH 07/19] codeblock types

---
 contributing/building-a-bento-video-player.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 38eb9166ac14..f838b9e9b117 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -59,7 +59,7 @@ This guide covers how to implement video player components that are internally i
 
 Start by generating an extension specifying `--bento` and `--nojss`. We name our extension **`amp-fantastic-player`**, ending with `-player` according to our [guidelines for naming a third-party component](../spec/amp-3p-naming.md).
 
-```shell
+```console
 amp make-extension --bento --nojss --name=amp-fantastic-player
 ```
 
@@ -67,7 +67,7 @@ amp make-extension --bento --nojss --name=amp-fantastic-player
 
 A [full directory for a Bento component](./building-a-bento-amp-extension.md#directory-structure) is generated, but this guide will cover the following files in particular:
 
-```shell
+```
 /extensions/amp-fantastic-player/1.0/
  ├── base-element.js               # Preact base element
  ├── component.js                  # Preact implementation

From 88219dd143ae725d5956df16a97f7f5ddf2b2015 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 08:38:54 -0700
Subject: [PATCH 08/19] details for 'origin'

---
 contributing/building-a-bento-video-player.md | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index f838b9e9b117..072bb1110569 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -294,6 +294,18 @@ function FantasticPlayerWithRef(
 
 #### `origin`
 
+By default, messages from the iframe are only verified by comparing their `contentWindow`. You should define a regular expression that verifies their origin:
+
+```js
+return (
+  <VideoIframe
+    {...rest}
+    origin={new RegExp('^https://((player|www)\.)?example\.com/?$')}
+    ...
+  />
+);
+```
+
 #### Playback methods with `makeMethodMessage`
 
 We need the ability to tell the iframe to execute certain actions, for example `play`, `mute` or `hideControls`. When using an iframe to load a player, this is done by sending a `postMessage` downstream.

From de0f286b863b831d2f9021d226805ffb57bdc6dd Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:31:17 -0700
Subject: [PATCH 09/19] address review

---
 contributing/building-a-bento-video-player.md | 24 +++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 072bb1110569..ecdcedc6a40f 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -253,7 +253,7 @@ function FantasticPlayerWithRef({...rest}, ref) {
 
 We're rendering an iframe that always loads `https://example.com/fantastic`, but we'll specify a dynamic URL later; hence `useMemo()`. Likewise, we'll need to define implementations for the communication functions `makeMethodMessage` and `onMessage`.
 
-You should add allow one more cross-extension import for `VideoIframe`. Following the previous location on **`dep-check-config.js`**, we add:
+You should list one more allowed cross-extension import for `VideoIframe`. Following the previous location on **`dep-check-config.js`**, we add:
 
 ```diff
   allowlist: [
@@ -407,7 +407,7 @@ Your `FantasticPlayer` component should return a `VideoWrapper` that's configure
 -     <ContainWrapper layout size paint {...rest} >
 -       ...
 -     </ContainWrapper>
-+     <VideoWrapper component="video" />
++     <VideoWrapper ref={ref} {...rest} component="video" />
     );
   }
 ```
@@ -419,13 +419,13 @@ import {VideoWrapper} from '../../amp-video/1.0/video-wrapper';
 
 // ...
 function FantasticPlayerWithRef({...rest}, ref) {
-  return <VideoWrapper component="video" />;
+  return <VideoWrapper ref={ref} {...rest} component="video" />;
 }
 ```
 
 We're specifying `"video"` as the element to render, which is also the default. We'll llater change this into our own component implementation.
 
-You should add allow one more cross-extension import for `VideoWrapper`. Following the previous location on **`dep-check-config.js`**, we add:
+You should list one more allowed cross-extension import for `VideoWrapper`. Following the previous location on **`dep-check-config.js`**, we add:
 
 ```diff
   allowlist: [
@@ -440,7 +440,7 @@ You should add allow one more cross-extension import for `VideoWrapper`. Followi
 
 The `VideoWrapper` must interact with an element that implements the `HTMLMediaInterface` like `<video>`, or a Preact component that emulates the same interface.
 
-For example, we may set `component={RandomVideo}`, where the specified function renders its own video from a randomly selected `src`.
+For example, we may set a `component={FantasticPlayerInternal}`, where the specified function controls how we render and interact with the `<video>` element.
 
 By passing the `ref` through, we're able to call methods like `play()` from `FantasticPlayer` on the `<video>` element itself. By passing the `...rest` of the props we make sure that `src` is set, in addition to listening to playback events through props like `onPlay`.
 
@@ -460,7 +460,13 @@ const FantasticPlayerInternal = forwardRef(FantasticPlayerInternalWithRef);
 // ...
 
 function FantasticPlayerWithRef({...rest}, ref) {
-  return <VideoWrapper {...rest} component={RandomVideo} />;
+  return (
+    <VideoWrapper
+      ref={ref}
+      {...rest}
+      component={FantasticPlayerInternal}
+    />
+  );
 }
 ```
 
@@ -607,7 +613,11 @@ function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
     }
   }, []);
 
-  return (<video ref={videoRef} {...rest}>{sources}</video>)
+  return (
+    <video ref={videoRef} {...rest}>
+      {sources}
+    </video>
+  );
 }
 ```
 

From dd80d11c787fc431e32e2f069c2b7688af060005 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:34:12 -0700
Subject: [PATCH 10/19] slack link

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index ecdcedc6a40f..97f9445679a1 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -156,7 +156,7 @@ However, it's more likely that you load a third-party iframe and you communicate
 
 > ⚠️ Components may **not** embed scripts from a third-party location into host documents. If a third-party script is absolutely required, like on `<amp-ima-video>`, it must be inserted in an intermediate iframe, which we call a **proxy frame**.
 >
-> Proxy frames on Bento have not yet been tested as video player components, so they're not covered in this guide. If you wish to use one, please get in touch with `@alanorozco` via a Github issue or on Slack.
+> Proxy frames on Bento have not yet been tested as video player components, so they're not covered in this guide. If you wish to use one, please get in touch with `@alanorozco` via a Github issue or on on [Slack](https://bit.ly/amp-slack-signup) in the [`#contributing` channel](https://amphtml.slack.com/messages/C9HRJ1GPN/).
 
 ### Forwarding `ref`
 

From 6f7f08c48ecef3451688d6c6b508218e954e630b Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:41:05 -0700
Subject: [PATCH 11/19] assorted changes

---
 contributing/building-a-bento-video-player.md | 43 +++++++++++++------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 97f9445679a1..d9dc284b8686 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -91,6 +91,8 @@ Our `BaseElement` should be a superclass of `VideoBaseElement`. In **`base-eleme
 into:
 
 ```js
+// base-element.js
+// ...
 import {MyFantasticPlayer} from './component';
 import {VideoBaseElement} from '../../amp-video/1.0/base-element';
 
@@ -119,6 +121,8 @@ This enables support for AMP actions and analytics, once we map attributes to th
 Bento components must specify certain layout properties in order to prevent [Cumulative Layout Shift (CLS)](https://web.dev/cls). Video extensions must include the following in the generated **`amp-fantastic-player.css`**:
 
 ```css
+/* amp-fantastic-player.css */
+
 /*
  * Pre-upgrade:
  * - display:block element
@@ -142,6 +146,8 @@ amp-fantastic-player:not(.i-amphtml-built) > :not([placeholder]):not(.i-amphtml-
 [**`props`**](https://github.com/ampproject/amphtml/blob/main/contributing/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`VideoBaseElement`](extensions/amp-video/1.0/base-element.js) for how most video properties are mapped. On your own `base-element.js`, you should specify any of them you support.
 
 ```js
+// base-element.js
+// ...
 /** @override */
 BaseElement['props'] = {
   'autoplay': {attr: 'autoplay', type: 'boolean'},
@@ -178,14 +184,13 @@ To enable AMP actions (`my-element.play`) and the Preact component's imperative
 So the outer structure looks like:
 
 ```js
+// component.js
 // ...
 import {forwardRef} from '../../../src/preact/compat';
-
 // ...
 function FantasticPlayerWithRef({...rest}, ref) {
   // ...
 }
-
 //...
 const FantasticPlayer = forwardRef(FantasticPlayerWithRef);
 FantasticPlayer.displayName = 'FantasticPlayer'; // Make findable for tests.
@@ -224,18 +229,15 @@ Your `FantasticPlayer` component should return a `VideoIframe` that's configured
   }
 ```
 
-into:
+So that our component returns a `<VideoIframe>`:
 
 ```js
+// component.js
+// ...
 import {VideoIframe} from '../../amp-video/1.0/video-iframe';
-
 // ...
-
 function FantasticPlayerWithRef({...rest}, ref) {
-  const src = useMemo(
-    () => 'https://example.com/fantastic',
-    []
-  );
+  const src = 'https://example.com/fantastic';
   const makeMethodMessage = useCallback(() => '{}', []);
   const onMessage = useCallback((e) => {
     console.log(e);
@@ -251,7 +253,7 @@ function FantasticPlayerWithRef({...rest}, ref) {
 }
 ```
 
-We're rendering an iframe that always loads `https://example.com/fantastic`, but we'll specify a dynamic URL later; hence `useMemo()`. Likewise, we'll need to define implementations for the communication functions `makeMethodMessage` and `onMessage`.
+We're rendering an iframe that always loads `https://example.com/fantastic`, but we'll specify a dynamic URL later. Likewise, we'll need to define implementations for the communication functions `makeMethodMessage` and `onMessage`.
 
 You should list one more allowed cross-extension import for `VideoIframe`. Following the previous location on **`dep-check-config.js`**, we add:
 
@@ -271,6 +273,8 @@ You may use props to construct the `src`, like using a `videoId` to load `https:
 We employ the `useMemo()` hook so that the `src` is generated only when the `videoId` changes:
 
 ```js
+// component.js
+// ...
 function FantasticPlayerWithRef(
   {videoId, ...rest},
   ref
@@ -294,9 +298,11 @@ function FantasticPlayerWithRef(
 
 #### `origin`
 
-By default, messages from the iframe are only verified by comparing their `contentWindow`. You should define a regular expression that verifies their origin:
+By default, messages from the iframe are only verified by comparing their `contentWindow`. You should define a regular expression that verifies their [origin](https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage#the_dispatched_event):
 
 ```js
+// component.js
+// ...
 return (
   <VideoIframe
     {...rest}
@@ -319,6 +325,8 @@ type MakeMethodMessageFunction = (method: string) => string;
 We implement this function with `useCallback()` so that it's created only when required as specified by hook dependencies. It's recommended that you also create a higher-level `makeMessage` function that creates and serializes messages as your iframe's interface needs it. In this case, we `JSON.stringify` a `videoId` and a `method`:
 
 ```js
+// component.js
+// ...
 function makeMessage(videoId, method) {
   return JSON.stringify({
     'videoId': videoId,
@@ -360,6 +368,8 @@ type OnMessageFunction = (event: MessageEvent) => void;
 Here we implement the `canplay`, `play` and `pause` events for an iframe that posts them as the message `{"event": "play"}`.
 
 ```js
+// component.js
+// ...
 function onMessage(event) {
   const {data, currentTarget} = event;
   switch (data?.event) {
@@ -415,6 +425,8 @@ Your `FantasticPlayer` component should return a `VideoWrapper` that's configure
 So that our component returns a `<VideoWrapper>`:
 
 ```js
+// component.js
+// ...
 import {VideoWrapper} from '../../amp-video/1.0/video-wrapper';
 
 // ...
@@ -423,7 +435,7 @@ function FantasticPlayerWithRef({...rest}, ref) {
 }
 ```
 
-We're specifying `"video"` as the element to render, which is also the default. We'll llater change this into our own component implementation.
+We're specifying `"video"` as the element to render, which is also the default. We'll later change this into our own component implementation.
 
 You should list one more allowed cross-extension import for `VideoWrapper`. Following the previous location on **`dep-check-config.js`**, we add:
 
@@ -445,6 +457,8 @@ For example, we may set a `component={FantasticPlayerInternal}`, where the speci
 By passing the `ref` through, we're able to call methods like `play()` from `FantasticPlayer` on the `<video>` element itself. By passing the `...rest` of the props we make sure that `src` is set, in addition to listening to playback events through props like `onPlay`.
 
 ```js
+// component.js
+// ...
 function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
   return (
     <div>
@@ -458,7 +472,6 @@ function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
 const FantasticPlayerInternal = forwardRef(FantasticPlayerInternalWithRef);
 
 // ...
-
 function FantasticPlayerWithRef({...rest}, ref) {
   return (
     <VideoWrapper
@@ -475,6 +488,8 @@ function FantasticPlayerWithRef({...rest}, ref) {
 In the previous example, props received from the `VideoWrapper` are implicitly set through `...rest`. If we set each explicitly, we see the `HTMLMediaInterface` attributes and events handled.
 
 ```js
+// component.js
+// ...
 function FantasticPlayerInternalWithRef(
   {
     muted,
@@ -587,6 +602,8 @@ Methods can be defined with `useImperativeHandle` at the `component` implementat
 > The methods and getters listed are the current requirements from `VideoWrapper`. Note that video players on Bento are in active development, so this list might expand in the future.
 
 ```js
+// component.js
+// ...
 function FantasticPlayerInternalWithRef({sources, ...rest}, ref) {
   const videoRef = useRef(null);
 

From 2a6365d6565911077f357bb649bfc53442698dd8 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:47:44 -0700
Subject: [PATCH 12/19] mismatched diff

---
 contributing/building-a-bento-video-player.md | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index d9dc284b8686..ee431ce69f7b 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -207,10 +207,7 @@ Your `FantasticPlayer` component should return a `VideoIframe` that's configured
 
   function FantasticPlayerWithRef({...rest}, ref) {
 -   ...
-+   const src = useMemo(
-+     () => 'https://example.com/fantastic',
-+     []
-+   );
++   const src = 'https://example.com/fantastic';
 +   const makeMethodMessage = useCallback(() => '{}', []);
 +   const onMessage = useCallback((e) => {
 +     console.log(e);

From 25a55878357d1ed84c2d156f05358387326d158b Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:48:14 -0700
Subject: [PATCH 13/19] nit

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index ee431ce69f7b..0ea7694c6753 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -37,7 +37,7 @@ AMP and Bento provide [default video player capabilities](https://amp.dev/docume
 
 On a host document, player components must dispatch the same events and implement the same methods so that playback and user interface can be coordinated successfully.
 
-**Preact components** can support this behavior by using a **`VideoWrapper`** that will use a specified `component`:
+**Preact components** can support this behavior by using a **`VideoWrapper`** that renders a specified `component`:
 
 ```js
 return <VideoWrapper component="video" {...props} />

From 09a133f8e37142fb6dd4ff3bfdb96fce4daeb8bb Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:49:33 -0700
Subject: [PATCH 14/19] ref

---
 contributing/building-a-bento-video-player.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 0ea7694c6753..4fb18aa18f56 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -217,6 +217,7 @@ Your `FantasticPlayer` component should return a `VideoIframe` that's configured
 -       ...
 -     </ContainWrapper>
 +     <VideoIframe
++       ref={ref}
 +       {...rest}
 +       src={src}
 +       makeMethodMessage={makeMethodMessage}
@@ -241,6 +242,7 @@ function FantasticPlayerWithRef({...rest}, ref) {
   }, []);
   return (
     <VideoIframe
+      ref={ref}
       {...rest}
       src={src}
       makeMethodMessage={makeMethodMessage}

From bd23e5fd53f3cd0d6af282ba60fb1f2bed3328f5 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:53:50 -0700
Subject: [PATCH 15/19] alphabetical order

---
 contributing/building-a-bento-amp-extension.md | 2 +-
 contributing/building-a-bento-video-player.md  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contributing/building-a-bento-amp-extension.md b/contributing/building-a-bento-amp-extension.md
index 3d5d807d9c87..e4217dcd8de8 100644
--- a/contributing/building-a-bento-amp-extension.md
+++ b/contributing/building-a-bento-amp-extension.md
@@ -672,7 +672,7 @@ $ amp check-types
     -   [amp-stream-gallery](https://github.com/ampproject/amphtml/pull/30597)
 -   Adding video components which use `VideoWrapper`. [You may also follow the guide to Building a Bento Video Component.](./building-a-bento-video-player.md)
     -   [amp-video](https://github.com/ampproject/amphtml/pull/30280)
-    -   [amp-youtube](https://github.com/ampproject/amphtml/pull/30444)
     -   [amp-vimeo](https://github.com/ampproject/amphtml/pull/33971)
+    -   [amp-youtube](https://github.com/ampproject/amphtml/pull/30444)
 -   Adding iframe based embeds
     -   [amp-instagram](https://github.com/ampproject/amphtml/pull/30230)
diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 4fb18aa18f56..6bf80144421f 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -649,5 +649,5 @@ Follow the [guide to Building a Bento AMP Component](./building-a-bento-amp-exte
 ## Example Pull Requests
 
 -   [amp-video](https://github.com/ampproject/amphtml/pull/30280)
--   [amp-youtube](https://github.com/ampproject/amphtml/pull/30444)
 -   [amp-vimeo](https://github.com/ampproject/amphtml/pull/33971)
+-   [amp-youtube](https://github.com/ampproject/amphtml/pull/30444)

From 155b69c7d7d99bfe0cf1b766731aef69732888a7 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:55:26 -0700
Subject: [PATCH 16/19] clearer order

---
 contributing/building-a-bento-video-player.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 6bf80144421f..51e5396449e6 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -566,12 +566,12 @@ When we click the following button on an AMP document:
 
 We call the corresponding function `play`:
 
+> The AMP action `my-element.play` is declared to be forwarded to the Preact component's method. See the [`init()` method on `VideoBaseElement`](../extensions/amp-video/1.0/base-element.js) for a list of the supported actions.
+
 ```
 -> FantasticPlayer.play()
 ```
 
-> The AMP action `my-element.play` is declared to be forwarded to the Preact component's method. See the [`init()` method on `VideoBaseElement`](../extensions/amp-video/1.0/base-element.js) for a list of the supported actions.
-
 Since we don't call `useImperativeHandle` at this layer, its `ref` forwards to `VideoWrapper`:
 
 ```

From bd47b62cdafae28d13762c27946c8aaa073a60e3 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 13:59:06 -0700
Subject: [PATCH 17/19] clearer

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index 51e5396449e6..f73c813249eb 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -398,7 +398,7 @@ function FantasticPlayerWithRef(
 }
 ```
 
-The API that your iframe's document uses to post messages is likely different, but it should always dispatch [`HTMLMediaElement` events](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement#events) upstream.
+Your iframe's interface to post messages is likely different, but your component should always dispatch [`HTMLMediaElement` events](https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement#events) upstream.
 
 ### Use `VideoWrapper` directly
 

From 5e0a5d09f2242c6f96179571968839151a42ac89 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 14:23:52 -0700
Subject: [PATCH 18/19] fix link

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index f73c813249eb..edddaa64d24d 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -143,7 +143,7 @@ amp-fantastic-player:not(.i-amphtml-built) > :not([placeholder]):not(.i-amphtml-
 
 ### `props`
 
-[**`props`**](https://github.com/ampproject/amphtml/blob/main/contributing/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`VideoBaseElement`](extensions/amp-video/1.0/base-element.js) for how most video properties are mapped. On your own `base-element.js`, you should specify any of them you support.
+[**`props`**](https://github.com/ampproject/amphtml/blob/main/contributing/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`VideoBaseElement`](./extensions/amp-video/1.0/base-element.js) for how most video properties are mapped. On your own `base-element.js`, you should specify any of them you support.
 
 ```js
 // base-element.js

From 6b146255b37e8d3978eaccf81090667d0198b6d5 Mon Sep 17 00:00:00 2001
From: Alan Orozco <alan@orozco.xyz>
Date: Tue, 4 May 2021 15:04:04 -0700
Subject: [PATCH 19/19] fix link for real

---
 contributing/building-a-bento-video-player.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contributing/building-a-bento-video-player.md b/contributing/building-a-bento-video-player.md
index edddaa64d24d..859b5c8b072e 100644
--- a/contributing/building-a-bento-video-player.md
+++ b/contributing/building-a-bento-video-player.md
@@ -143,7 +143,7 @@ amp-fantastic-player:not(.i-amphtml-built) > :not([placeholder]):not(.i-amphtml-
 
 ### `props`
 
-[**`props`**](https://github.com/ampproject/amphtml/blob/main/contributing/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`VideoBaseElement`](./extensions/amp-video/1.0/base-element.js) for how most video properties are mapped. On your own `base-element.js`, you should specify any of them you support.
+[**`props`**](https://github.com/ampproject/amphtml/blob/main/contributing/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`VideoBaseElement`](../extensions/amp-video/1.0/base-element.js) for how most video properties are mapped. On your own `base-element.js`, you should specify any of them you support.
 
 ```js
 // base-element.js