Skip to content

Commit

Permalink
Bento: iframe components attemptChangeHeight (#35027)
Browse files Browse the repository at this point in the history
* forceChangeHeight -> attemptChangeeHeight for Bento

* Add information to guide.

* Exclude amp-twitter

* Warn if attemptChangeHeight fails

* Restore changes to amp-twitter

* Move warning inside PreactBaseElement

* Fix user imports

* Alan comments

* return super.attemptChangeHeight()

* Update documentation

* Throw caught error
  • Loading branch information
caroqliu committed Jul 9, 2021
1 parent b393190 commit ab85c09
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 16 deletions.
17 changes: 13 additions & 4 deletions docs/building-a-bento-iframe-component.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ function FantasticEmbedWithRef({...rest}, ref) {

AMP documents additionally guarantee layout stability to the degree that it manages when components may or may not resize on the page. Because of this, the `IframeEmbed` component takes a `requestResize` prop where a different flow of logic may be passed in by the publisher to respond to measure events.

In your AMP element implementation, you will use `requestResize` to pass in the `attemptChangeHeight` method that is extended from the `BaseElement` class:
In your AMP element implementation, you will use `requestResize` to pass in the `attemptChangeHeight` method that is extended from the `PreactBaseElement` class:

```javascript
// amp-fantastic-embed.js
Expand All @@ -242,14 +242,23 @@ class AmpFantasticEmbed extends BaseElement {
/** @override */
init() {
return dict({
'requestResize': (height) => {
this.attemptChangeHeight(height);
},
'requestResize': (height) => this.attemptChangeHeight(height),
});
}
}
```

For components that request a resize that is denied by the AMP runtime, publishers are recommended to use an `overflow` element to solicit user interaction in order to resize as a layout stability best-practice.

This information can be provided in the component documentation with an exemplary code sample:

```html
<amp-fantastic-embed layout="fixed" width="400" height="200">
<button overflow>Click me to load the full iframed content!</button>
</amp-fantastic-embed>
<p>Content below the component.</p>
```

#### Passing or overriding props

In the previous example, props received from the `ProxyIframeEmbed` are implicitly set through `...rest`. If we set each explicitly, we see the `HTMLIframeElement` attributes handled.
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-facebook/1.0/amp-facebook.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class AmpFacebook extends BaseElement {
init() {
return dict({
'onReady': () => this.togglePlaceholder(false),
'requestResize': (height) => this.forceChangeHeight(height),
'requestResize': (height) => this.attemptChangeHeight(height),
});
}

Expand Down
10 changes: 7 additions & 3 deletions extensions/amp-facebook/1.0/test/test-amp-facebook.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015 The AMP HTML Authors. All Rights Reserved.
* Copyright 2021 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.
Expand Down Expand Up @@ -286,7 +286,11 @@ describes.realWin(
await waitForRender();

const impl = await element.getImpl(false);
const forceChangeHeightStub = env.sandbox.stub(impl, 'forceChangeHeight');
const attemptChangeHeightStub = env.sandbox.stub(
impl,
'attemptChangeHeight'
);
attemptChangeHeightStub.returns(Promise.resolve());

const mockEvent = new CustomEvent('message');
const sentinel = JSON.parse(
Expand All @@ -298,7 +302,7 @@ describes.realWin(
mockEvent.source =
element.shadowRoot.querySelector('iframe').contentWindow;
win.dispatchEvent(mockEvent);
expect(forceChangeHeightStub).to.be.calledOnce.calledWith(1000);
expect(attemptChangeHeightStub).to.be.calledOnce.calledWith(1000);
});
}
);
4 changes: 1 addition & 3 deletions extensions/amp-instagram/1.0/amp-instagram.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ class AmpInstagram extends BaseElement {
/** @override */
init() {
return dict({
'requestResize': (height) => {
this.forceChangeHeight(height);
},
'requestResize': (height) => this.attemptChangeHeight(height),
});
}
}
Expand Down
8 changes: 6 additions & 2 deletions extensions/amp-instagram/1.0/test/test-amp-instagram.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ describes.realWin(
await waitForRender();

const impl = await element.getImpl(false);
const forceChangeHeightStub = env.sandbox.stub(impl, 'forceChangeHeight');
const attemptChangeHeightStub = env.sandbox.stub(
impl,
'attemptChangeHeight'
);
attemptChangeHeightStub.returns(Promise.resolve());

const mockEvent = new CustomEvent('message');
mockEvent.origin = 'https://www.instagram.com';
Expand All @@ -107,7 +111,7 @@ describes.realWin(
element.shadowRoot.querySelector('iframe').contentWindow;
win.dispatchEvent(mockEvent);

expect(forceChangeHeightStub).to.be.calledOnce.calledWith(1000);
expect(attemptChangeHeightStub).to.be.calledOnce.calledWith(1000);
});
}
);
2 changes: 1 addition & 1 deletion extensions/amp-twitter/1.0/amp-twitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class AmpTwitter extends BaseElement {
/** @override */
init() {
return dict({
'requestResize': (height) => this.forceChangeHeight(height),
'requestResize': (height) => this.attemptChangeHeight(height),
});
}

Expand Down
8 changes: 6 additions & 2 deletions extensions/amp-twitter/1.0/test/test-amp-twitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ describes.realWin(
await waitForRender();

const impl = await element.getImpl(false);
const forceChangeHeightStub = env.sandbox.stub(impl, 'forceChangeHeight');
const attemptChangeHeightStub = env.sandbox.stub(
impl,
'attemptChangeHeight'
);
attemptChangeHeightStub.returns(Promise.resolve());

const mockEvent = new CustomEvent('message');
const sentinel = JSON.parse(
Expand All @@ -86,7 +90,7 @@ describes.realWin(
mockEvent.source =
element.shadowRoot.querySelector('iframe').contentWindow;
win.dispatchEvent(mockEvent);
expect(forceChangeHeightStub).to.be.calledOnce.calledWith(1000);
expect(attemptChangeHeightStub).to.be.calledOnce.calledWith(1000);
});

it('should replace iframe after tweetid mutation', async () => {
Expand Down
13 changes: 13 additions & 0 deletions src/preact/base-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,19 @@ export class PreactBaseElement extends AMP.BaseElement {
}
}

/** @override */
attemptChangeHeight(newHeight) {
return super.attemptChangeHeight(newHeight).catch((e) => {
if (this.getOverflowElement && !this.getOverflowElement()) {
console./* OK */ warn(
'[overflow] element not found. Provide one to enable resizing to full contents.',
this.element
);
}
throw e;
});
}

/**
* @protected
* @param {!JsonObject} props
Expand Down

0 comments on commit ab85c09

Please sign in to comment.