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.

Sign up for GitHub

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

amp-video-iframe: use InOb instead of ce.getIntersectionChangeEntry. #31730

Merged
merged 4 commits into from
Jan 13, 2021
Merged

amp-video-iframe: use InOb instead of ce.getIntersectionChangeEntry. #31730

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28b4911
amp-video-iframe: use InOb instead of ce.getIntersectionChangeEntry.
samouri Dec 23, 2020
f715271
update to use new helper
samouri Jan 8, 2021
2141301
lint
samouri Jan 8, 2021
2dbf3fb
address Dima comment
samouri Jan 12, 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
18 changes: 14 additions & 4 deletions extensions/amp-video-iframe/0.1/amp-video-iframe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
import {getData, listen} from '../../../src/event-helper';
import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl';
import {isLayoutSizeDefined} from '../../../src/layout';
import {measureIntersection} from '../../../src/utils/intersection';
import {once} from '../../../src/utils/function';

/** @private @const */
Expand Down Expand Up @@ -126,6 +127,12 @@ class AmpVideoIframe extends AMP.BaseElement {
* @private
*/
this.boundOnMessage_ = (e) => this.onMessage_(e);

/**
* @param {!Element} element
* @return {!Promise<IntersectionObserverEntry>} element
*/
this.measureIntersection = (element) => measureIntersection(element);
samouri marked this conversation as resolved.
Show resolved Hide resolved
}

/** @override */
Expand Down Expand Up @@ -275,6 +282,7 @@ class AmpVideoIframe extends AMP.BaseElement {

/**
* @param {!Event} event
* @return {Promise}
samouri marked this conversation as resolved.
Show resolved Hide resolved
* @private
*/
onMessage_(event) {
Expand Down Expand Up @@ -302,8 +310,9 @@ class AmpVideoIframe extends AMP.BaseElement {

if (methodReceived) {
if (methodReceived == 'getIntersection') {
this.postIntersection_(messageId);
return;
return this.measureIntersection(this.element).then((intersection) => {
this.postIntersection_(messageId, intersection);
});
}
userAssert(false, 'Unknown method `%s`.', methodReceived);
return;
Expand Down Expand Up @@ -361,10 +370,11 @@ class AmpVideoIframe extends AMP.BaseElement {

/**
* @param {number} messageId
* @param {!IntersectionObserverEntry} intersection
* @private
*/
postIntersection_(messageId) {
const {time, intersectionRatio} = this.element.getIntersectionChangeEntry();
postIntersection_(messageId, intersection) {
const {intersectionRatio, time} = intersection;

// Only post ratio > 0 when in autoplay range to prevent internal autoplay
// implementations that differ from ours.
Expand Down
14 changes: 7 additions & 7 deletions extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ describes.realWin(
);
}

function stubIntersectionEntry(element, time, intersectionRatio) {
function stubMeasureIntersection(videoIframe, time, intersectionRatio) {
const entry = {time, intersectionRatio};
env.sandbox
./*OK*/ stub(element, 'getIntersectionChangeEntry')
.returns(entry);
.stub(videoIframe.implementation_, 'measureIntersection')
.returns(Promise.resolve(entry));
return entry;
}

Expand Down Expand Up @@ -290,10 +290,10 @@ describes.realWin(

const expectedResponseMessage = {
id,
args: stubIntersectionEntry(videoIframe, time, intersectionRatio),
args: stubMeasureIntersection(videoIframe, time, intersectionRatio),
};

videoIframe.implementation_.onMessage_(message);
await videoIframe.implementation_.onMessage_(message);

expect(postMessage.withArgs(env.sandbox.match(expectedResponseMessage)))
.to.have.been.calledOnce;
Expand All @@ -311,7 +311,7 @@ describes.realWin(

const postMessage = stubPostMessage(videoIframe);

stubIntersectionEntry(videoIframe, time, intersectionRatio);
stubMeasureIntersection(videoIframe, time, intersectionRatio);

acceptMockedMessages(videoIframe);

Expand All @@ -325,7 +325,7 @@ describes.realWin(
},
};

videoIframe.implementation_.onMessage_(message);
await videoIframe.implementation_.onMessage_(message);

expect(postMessage.withArgs(env.sandbox.match(expectedResponseMessage)))
.to.have.been.calledOnce;
Expand Down