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

Extract AMPPreactBaseElement #37309

Merged
merged 23 commits into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d5310bf
extract fn to register custom element
kvchari Jan 4, 2022
4508f14
create (empty) amp pbe
kvchari Jan 5, 2022
7c2a005
inherit amp-accordion from amp-pbe
kvchari Jan 5, 2022
39941d8
fix custom-element definitions for iframes (fixes tests)
kvchari Jan 5, 2022
0704b8c
workarounds to properly test bento components
kvchari Jan 5, 2022
61ae6f5
split out generic bento testing from amp tests
kvchari Jan 5, 2022
6d4765c
Revert "workarounds to properly test bento components"
kvchari Jan 5, 2022
fa48a80
inherit all non-video bento-amp components from amp-pbe
kvchari Jan 5, 2022
dd0bdbd
rename VideoBaseElement -> AmpVideoBaseElement
kvchari Jan 5, 2022
4e68348
refactor inheritance hierarchy for video components to remove amp cod…
kvchari Jan 5, 2022
e265420
fix amp-facebook hierarchy to preserve AmpFbBase functionality
kvchari Jan 6, 2022
5c4358e
update docs to demonstrate new way to scaffold amp-* components
kvchari Jan 6, 2022
93260f3
move (some) amp-specific methods into amp-pbe
kvchari Jan 6, 2022
66c65e1
update dependencies, rename BentoVideoBaseElement
kvchari Jan 11, 2022
8d0c950
add doc
kvchari Jan 12, 2022
1c77c72
refactor dailymotion
kvchari Jan 12, 2022
bb3472e
ampvideobaseelement docs
kvchari Jan 12, 2022
aba612d
lint
kvchari Jan 12, 2022
6fa1130
fix typings and lint
kvchari Jan 13, 2022
1f74058
remove file
kvchari Jan 13, 2022
21f7876
remove dict() based on ongoign discussions to deprecate
kvchari Jan 13, 2022
1fc0c59
disable local/restrict-this-access for the whole file
kvchari Jan 13, 2022
d334290
remove more references to dict()
kvchari Jan 18, 2022
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: 2 additions & 4 deletions build-system/tasks/extension-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,10 @@ async function getBentoBuildFilename(dir, name, mode, options) {
function generateBentoEntryPointSource(name, toExport) {
return dedent(`
import {BaseElement} from '../base-element';
import {defineBentoElement} from '../../../../src/preact/bento-ce';

function defineElement() {
customElements.define(
__name__,
BaseElement.CustomElement(BaseElement)
);
defineBentoElement(__name__, BaseElement);
}

${toExport ? 'export {defineElement};' : 'defineElement();'}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {BaseElement} from './base-element';
__css_import__;
import {isExperimentOn} from '#experiments';
import {AmpPreactBaseElement, setSuperClass} from '#preact/amp-base-element';
import {userAssert} from '#utils/log';

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

class Amp__component_name_pascalcase__ extends BaseElement {
class Amp__component_name_pascalcase__ extends setSuperClass(BaseElement, AmpPreactBaseElement) {
/** @override */
init() {
// __do_not_submit__: This is example code only.
Expand Down
3 changes: 2 additions & 1 deletion build-system/test-configs/dep-check-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ exports.rules = [
'extensions/amp-facebook-page/0.1/amp-facebook-page.js->extensions/amp-facebook/0.1/facebook-loader.js',
'extensions/amp-facebook-comments/0.1/amp-facebook-comments.js->extensions/amp-facebook/0.1/facebook-loader.js',

// VideoBaseElement, VideoIframe and VideoWrapper are meant to be shared.
// AmpVideoBaseElement, BentoVideoBaseElement, VideoIframe and VideoWrapper are meant to be shared.
'extensions/**->extensions/amp-video/1.0/video-base-element.js',
'extensions/**->extensions/amp-video/1.0/base-element.js',
'extensions/**->extensions/amp-video/1.0/video-iframe.js',

// <amp-video-iframe> versions share this message API definition.
Expand Down
12 changes: 7 additions & 5 deletions docs/building-a-bento-amp-extension.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ actionServiceForDoc(doc.documentElement).trigger(
createCustomEvent(
win,
`amp-base-carousel.${name}`,
dict({'index': index})
{'index': index}
),
ActionTrust.DEFAULT
);
Expand Down Expand Up @@ -351,6 +351,8 @@ You must document your element's actions and events in its own reference documen
The following shows the overall structure of your element implementation file (`extensions/amp-my-element/1.0/amp-my-element.js`). See [Experiments](#experiments) to make sure your component is experimentally gated if necessary.

```js
import {AmpPreactBaseElement, setSuperClass} from '#preact/amp-base-element';

import {func1, func2} from '../../../src/module';
import {BaseElement} from './base-element'; // Preact base element.
import {CSS} from '../../../build/amp-my-element-1.0.css';
Expand All @@ -362,7 +364,7 @@ const EXPERIMENT = 'amp-my-element';
/** @const */
const TAG = 'amp-my-element';

class AmpMyElement extends BaseElement {
class AmpMyElement extends setSuperClass(BaseElement, AmpPreactBaseElement) {
/** @override */
init() {
// Perform any additional processing of prop values that are not
Expand All @@ -371,10 +373,10 @@ class AmpMyElement extends BaseElement {
this.registerApiAction('close', (api) => api.close());

const processedProp = parseInt(element.getAttribute('data-binary'), 2);
return dict({
return {
'processedProp': processedProp,
'onClose': (event) => fireAmpEvent(event)}
);
'onClose': (event) => fireAmpEvent(event)
};
}

/** @override */
Expand Down
4 changes: 2 additions & 2 deletions docs/building-a-bento-iframe-component.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ In your AMP element implementation, you will use `requestResize` to pass in the
class AmpFantasticEmbed extends BaseElement {
/** @override */
init() {
return dict({
return {
'requestResize': (height) => this.attemptChangeHeight(height),
});
};
}
}
```
Expand Down
41 changes: 31 additions & 10 deletions docs/building-a-bento-video-player.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
- [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)
- [Extend `AmpVideoBaseElement` for AMP](#extend-ampvideobaseelement-for-amp)
- [`props`](#props)
- [Define a Preact component](#define-a-preact-component)
- [Forwarding `ref`](#forwarding-ref)
Expand Down Expand Up @@ -51,7 +51,7 @@ However, most video players are embedded through an iframe so they should use **
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.
To enable component support for **AMP documents**, our video player element must extend from a base class `AmpVideoBaseElement`. 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.

Expand All @@ -75,17 +75,17 @@ A [full directory for a Bento component](./building-a-bento-amp-extension.md#dir
└── amp-my-fantastic-player.css # Custom CSS
```

## Extend `VideoBaseElement` for AMP
## Extend `AmpVideoBaseElement` for AMP

Our `BaseElement` should be a superclass of `VideoBaseElement`. In **`base-element.js`**, we change:
Our `BaseElement` should be a superclass of `BentoVideoBaseElement`. In **`base-element.js`**, we change:

```diff
import {MyFantasticPlayer} from './component';
- import {PreactBaseElement} from '#preact/base-element';
+ import {VideoBaseElement} from '../../amp-video/1.0/video-base-element';
+ import {BentoVideoBaseElement} from '../../amp-video/1.0/base-element';

- export class BaseElement extends PreactBaseElement {}
+ export class BaseElement extends VideoBaseElement {}
+ export class BaseElement extends BentoVideoBaseElement {}
```

into:
Expand All @@ -94,16 +94,37 @@ into:
// base-element.js
// ...
import {MyFantasticPlayer} from './component';
import {VideoBaseElement} from '../../amp-video/1.0/base-element';
import {BentoVideoBaseElement} from '../../amp-video/1.0/base-element';

export class BaseElement extends VideoBaseElement {}
export class BaseElement extends BentoVideoBaseElement {}
```

Our `AmpFantasticPlayer` should be a superclass of `AmpVideoBaseElement`. in amp-fantastic-player.js, we chagne:

```diff
+ import {setSuperClass} from '#preact/amp-base-element';

+ import {AmpVideoBaseElement} from '../../amp-video/1.0/video-base-element';

- class AmpFantasticPlayer extends BaseElement {}
+ class AmpFantasticPlayer extends setSuperClass(BaseElement, AmpVideoBaseElement) {}
```

into:

```js
import {setSuperClass} from '#preact/amp-base-element';

import {AmpVideoBaseElement} from '../../amp-video/1.0/video-base-element';

class AmpFantasticPlayer extends setSuperClass(BaseElement, AmpVideoBaseElement) {}
```

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.

### `props`

[**`props`**](https://github.com/ampproject/amphtml/blob/main/docs/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/docs/building-a-bento-amp-extension.md#preactbaseelementprops) map the AMP element's attributes to the Preact component props. Take a look at [`AmpVideoBaseElement`](../extensions/amp-video/1.0/video-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
Expand Down Expand Up @@ -510,7 +531,7 @@ 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.
> The AMP action `my-element.play` is declared to be forwarded to the Preact component's method. See the [`init()` method on `AmpVideoBaseElement`](../extensions/amp-video/1.0/video-base-element.js) for a list of the supported actions.
```
-> FantasticPlayer.play()
Expand Down
4 changes: 3 additions & 1 deletion extensions/amp-accordion/1.0/amp-accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {getWin} from '#core/window';

import {isExperimentOn} from '#experiments';

import {AmpPreactBaseElement, setSuperClass} from '#preact/amp-base-element';

import {Services} from '#service';

import {createCustomEvent} from '#utils/event-helper';
Expand All @@ -16,7 +18,7 @@ import {CSS} from '../../../build/amp-accordion-1.0.css';
const TAG = 'amp-accordion';

/** @extends {PreactBaseElement<BentoAccordionDef.AccordionApi>} */
class AmpAccordion extends BaseElement {
class AmpAccordion extends setSuperClass(BaseElement, AmpPreactBaseElement) {
/** @override */
init() {
this.registerApiAction('toggle', (api, invocation) =>
Expand Down
Loading