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-iframe] changes to publish to npm #36190

Merged
merged 10 commits into from
Oct 1, 2021
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: 2 additions & 0 deletions build-system/compile/bundles.config.extensions.json
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@
"version": "1.0",
"latestVersion": "0.1",
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
}
},
Expand Down
22 changes: 22 additions & 0 deletions extensions/amp-iframe/1.0/amp-iframe.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Pre-upgrade:
* - display:block element
* - size-defined element
*/
amp-iframe {
display: block;
overflow: hidden;
position: relative;
}

/* Pre-upgrade: size-defining element - hide text. */
amp-iframe:not(.i-amphtml-built) {
color: transparent !important;
}

/* Pre-upgrade: size-defining element - hide children. */
amp-iframe:not(.i-amphtml-built)
> :not([placeholder]):not([slot='i-amphtml-svc']) {
display: none;
content-visibility: hidden;
}
4 changes: 2 additions & 2 deletions extensions/amp-iframe/1.0/base-element.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {Iframe} from './component';
import {BentoIframe} from './component';
import {PreactBaseElement} from '#preact/base-element';

export class BaseElement extends PreactBaseElement {}

/** @override */
BaseElement['Component'] = Iframe;
BaseElement['Component'] = BentoIframe;

/** @override */
BaseElement['props'] = {
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-iframe/1.0/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import {useMergeRefs} from '#preact/utils';
const NOOP = () => {};

/**
* @param {!IframeDef.Props} props
* @param {!BentoIframeDef.Props} props
* @return {PreactDef.Renderable}
*/
export function Iframe({
export function BentoIframe({
allowFullScreen,
allowPaymentRequest,
iframeStyle,
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-iframe/1.0/component.type.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @externs */

/** @const */
var IframeDef = {};
var BentoIframeDef = {};

/**
* @typedef {{
Expand All @@ -15,4 +15,4 @@ var IframeDef = {};
* srcdoc: (string),
* }}
*/
IframeDef.Props;
BentoIframeDef.Props;
16 changes: 8 additions & 8 deletions extensions/amp-iframe/1.0/storybook/Basic.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import * as Preact from '#preact';
import {Iframe} from '../component';
import {BentoIframe} from '../component';

export default {
title: 'Iframe',
component: Iframe,
component: BentoIframe,
};

export const _default = () => {
return (
<Iframe
<BentoIframe
style={{width: 800, height: 600}}
iframeStyle={{border: '1px solid black'}}
src="https://www.wikipedia.org/"
title="Wikipedia"
></Iframe>
></BentoIframe>
);
};

Expand All @@ -27,15 +27,15 @@ export const WithIntersectingIframe = () => {
backgroundColor: 'blue',
}}
></div>
<Iframe
<BentoIframe
style={{width: 100, height: 100}}
iframeStyle={{border: '1px solid black'}}
sandbox="allow-scripts allow-same-origin"
resizable
src="/examples/bento/amp-iframe-resizing-example.html"
>
<div placeholder>Placeholder</div>
</Iframe>
</BentoIframe>
<p>The above iframe will not resize and should remain 100x100px</p>
</div>
);
Expand All @@ -53,15 +53,15 @@ export const WithResizableIframe = () => {
backgroundColor: 'blue',
}}
></div>
<Iframe
<BentoIframe
style={{width: 100, height: 100}}
iframeStyle={{border: '1px solid black'}}
sandbox="allow-scripts allow-same-origin"
resizable
src="/examples/bento/amp-iframe-resizing-example.html"
>
<div placeholder>Placeholder</div>
</Iframe>
</BentoIframe>
<p>The above iframe should be 300x300px when visible</p>
</div>
);
Expand Down
14 changes: 7 additions & 7 deletions extensions/amp-iframe/1.0/test/test-component.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
import * as Preact from '#preact';
import {Iframe} from '../component';
import {BentoIframe} from '../component';
import {mount} from 'enzyme';

describes.sandboxed('Iframe preact component v1.0', {}, (env) => {
describes.sandboxed('BentoIframe preact component v1.0', {}, (env) => {
it('should render', () => {
const wrapper = mount(<Iframe src={'https://www.google.com'} />);
const wrapper = mount(<BentoIframe src={'https://www.google.com'} />);

const component = wrapper.find(Iframe.name);
const component = wrapper.find(BentoIframe.name);
expect(component).to.have.lengthOf(1);
expect(component.prop('src')).to.equal('https://www.google.com');
});

it('should set truthy props and strip falsy props', () => {
const wrapper = mount(
<Iframe
<BentoIframe
src={'https://www.google.com'}
allowFullScreen={true}
allowPaymentRequest={false}
/>
);

const component = wrapper.find(Iframe.name);
const component = wrapper.find(BentoIframe.name);
expect(component).to.have.lengthOf(1);
expect(component.prop('src')).to.equal('https://www.google.com');
expect(component.prop('allowFullScreen')).to.be.true;
Expand All @@ -31,7 +31,7 @@ describes.sandboxed('Iframe preact component v1.0', {}, (env) => {
it('should trigger onLoadCallback when iframe loads', () => {
const onLoadSpy = env.sandbox.spy();
const wrapper = mount(
<Iframe src={'https://www.google.com'} onLoad={onLoadSpy} />
<BentoIframe src={'https://www.google.com'} onLoad={onLoadSpy} />
);
wrapper.find('iframe').simulate('load');
expect(onLoadSpy).to.be.calledOnce;
Expand Down
31 changes: 15 additions & 16 deletions extensions/amp-iframe/amp-iframe.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,8 @@ bento: true

## Usage

[tip type="warning"]

This component is work in progress.

[/tip]

Displays an AMP valid iframe. `amp-iframe` has several important differences from vanilla iframes that are
designed to make it more secure and avoid AMP files that are dominated by a
single iframe:
designed to make it more secure and avoid AMP files that are dominated by a single iframe:

- An `amp-iframe` may not appear close to the top of the document (except for
iframes that use `placeholder` as described
Expand All @@ -41,7 +34,6 @@ single iframe:
height="100"
sandbox="allow-scripts allow-same-origin"
layout="responsive"
frameborder="0"
src="https://www.google.com/maps/embed/v1/place?key=AIzaSyAyAS599A2GGPKTmtNr9CptD61LE4gN6oQ&q=iceland"
>
</amp-iframe>
Expand All @@ -50,17 +42,19 @@ single iframe:
<amp-iframe width="200" height="100"
sandbox="allow-scripts allow-same-origin"
layout="responsive"
frameborder="0"
src="https://www.google.com/maps/embed/v1/place?key=AIzaSyAyAS599A2GGPKTmtNr9CptD61LE4gN6oQ&q=iceland">
</amp-iframe>

### Migrating from 0.1

Unlike `0.1`, the experimental `1.0` version of `amp-iframe` has deprecated the `frameborder` attribute. Use the CSS `border` property to control `<amp-iframe>` borders.

### Use existing AMP components instead of `amp-iframe`

The `amp-iframe` component should be considered a fallback if the required user
experience is not possible by other means in AMP, that is, there's not already
an existing AMP component
for the use case. This is because there are many benefits to using an AMP
component tailored for a specific use-case such as:
an existing AMP component for the use case. This is because there are many
benefits to using an AMP component tailored for a specific use-case such as:

- Better resource management and performance
- Custom components can provide built-in placeholder images in some cases.
Expand Down Expand Up @@ -92,6 +86,10 @@ The reasons for this policy are that:
- `amp-iframe` has no fully iframe controlled resize mechanism.
- Viewability information may not be available to `amp-iframe`.

### Standalone use outside valid AMP documents

Bento AMP allows you to use AMP components in non-AMP pages without needing to commit to fully valid AMP. You can take these components and place them in implementations with frameworks and CMSs that don't support AMP. Read more in our guide [Use AMP components in non-AMP pages](https://amp.dev/documentation/guides-and-tutorials/start/bento_guide/). To find the standalone version of `amp-iframe`, see [`bento-iframe`](./1.0/README.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Until README.md exists, this will have a dead link issue in CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. README.me is added in #36210 and will be merged soon.


### Iframe with placeholder <a name="iframe-with-placeholder"></a>

It is possible to have an `amp-iframe` appear at the top of a document when the
Expand Down Expand Up @@ -243,7 +241,8 @@ window.addEventListener('message', function (event) {

The intersection message would be sent by the parent to the iframe in the format of IntersectionObserver entry wheneve there is intersectionRatio change across thresholds [0, 0.05, 0.1, ... 0.9, 0.95, 1].

## Iframe & Consent Data
<!-- TODO(#36211): uncomment when Bento amp-consent (v1.0) is ready -->
<!-- ## Iframe & Consent Data

Iframes can send a `send-consent-data` message to receive consent data if a CMP is present on their parents page.

Expand Down Expand Up @@ -288,6 +287,7 @@ window.addEventListener('message', function (event) {
console.log(event.data.consentString);
});
```
-->

## Attributes

Expand Down Expand Up @@ -353,5 +353,4 @@ direct user purpose such as being invisible or small.

## Validation

See [`amp-iframe` rules](validator-amp-iframe.protoascii)
in the AMP validator specification.
See [`amp-iframe` rules](validator-amp-iframe.protoascii) in the AMP validator specification.