Skip to content

Commit

Permalink
✨[amp-story-player] Upgrading amp-story-player to CE (#27320)
Browse files Browse the repository at this point in the history
* upgrade to CEv1, move ctor to init, change from onload to load listener

* add viewpoer meta tag to sample

* failing test

* fix tests

* expand comment on tests

* call connectedCallback() instead of init

* fix test

* change to suffix underscore for convention

* rely on connectedCallback rather than calling build + layout manaully

* prettier parens

* Revert "change to suffix underscore for convention"

This reverts commit 3118d00c06085d325f1c40d5a3d462e3f24e70e9.

* call buildcallback & layoutWHenVisible from connectedCallback, making each player responsible to call them individually.
  • Loading branch information
Enriqe committed Apr 6, 2020
1 parent 7f6a191 commit df04d54
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 80 deletions.
1 change: 1 addition & 0 deletions examples/amp-story/player.html
Expand Up @@ -32,6 +32,7 @@
text-shadow: 3px 3px #000;
}
</style>
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
</head>
<body>
<h1>This is a NON-AMP page that embeds a story below:</h1>
Expand Down
45 changes: 21 additions & 24 deletions src/amp-story-player-impl.js
Expand Up @@ -14,18 +14,19 @@
* limitations under the License.
*/

import {IframePool} from './amp-story-player-iframe-pool';
import {Messaging} from '@ampproject/viewer-messaging';
import {VisibilityState} from './visibility-state';
import {
addParamsToUrl,
getFragment,
parseUrlWithA,
removeFragment,
} from './url';
import {applySandbox} from './3p-frame';
import {dict, map} from './utils/object';
// Source for this constant is css/amp-story-player-iframe.css
import {IframePool} from './amp-story-player-iframe-pool';
import {VisibilityState} from './visibility-state';
import {applySandbox} from './3p-frame';
import {AmpStoryPlayerManager} from './amp-story-player-manager';
import {cssText} from '../build/amp-story-player-iframe.css';
import {resetStyles, setStyle, setStyles} from './style';
import {toArray} from './types';
Expand Down Expand Up @@ -66,29 +67,21 @@ export const IFRAME_IDX = '__AMP_IFRAME_IDX__';
* Note that this is a vanilla JavaScript class and should not depend on AMP
* services, as v0.js is not expected to be loaded in this context.
*/
export class AmpStoryPlayer {
export class AmpStoryPlayer extends HTMLElement {
/**
* @param {!Window} win
* @param {!Element} element
* @constructor
* Player is appended in the document.
*/
constructor(win, element) {
console./*OK*/ assert(
element.childElementCount > 0,
'Missing configuration.'
);
connectedCallback() {
console./*OK*/ assert(this.childElementCount > 0, 'Missing configuration.');

/** @private {!Window} */
this.win_ = win;
this.win_ = self;

/** @private {!Array<!Element>} */
this.iframes_ = [];

/** @private {!Element} */
this.element_ = element;

/** @private {!Document} */
this.doc_ = win.document;
this.doc_ = this.win_.document;

/** @private {!Element} */
this.cachedA_ = this.doc_.createElement('a');
Expand Down Expand Up @@ -121,19 +114,23 @@ export class AmpStoryPlayer {
lastX: 0,
isSwipeX: null,
};

this.buildCallback_();
const manager = new AmpStoryPlayerManager(self);
manager.layoutWhenVisible(this);
}

/**
* @public
* @return {!Element}
*/
getElement() {
return this.element_;
return this;
}

/** @public */
buildCallback() {
this.stories_ = toArray(this.element_.querySelectorAll('a'));
/** @private */
buildCallback_() {
this.stories_ = toArray(this.querySelectorAll('a'));

this.initializeShadowRoot_();
this.initializeIframes_();
Expand All @@ -158,7 +155,7 @@ export class AmpStoryPlayer {
this.rootEl_ = this.doc_.createElement('main');

// Create shadow root
const shadowRoot = this.element_.attachShadow({mode: 'open'});
const shadowRoot = this.attachShadow({mode: 'open'});

// Inject default styles
const styleEl = this.doc_.createElement('style');
Expand Down Expand Up @@ -250,12 +247,12 @@ export class AmpStoryPlayer {
iframeEl.onload = () => {
this.rootEl_.classList.remove(LoadStateClass.LOADING);
this.rootEl_.classList.add(LoadStateClass.LOADED);
this.element_.classList.add(LoadStateClass.LOADED);
this.classList.add(LoadStateClass.LOADED);
};
iframeEl.onerror = () => {
this.rootEl_.classList.remove(LoadStateClass.LOADING);
this.rootEl_.classList.add(LoadStateClass.ERROR);
this.element_.classList.add(LoadStateClass.ERROR);
this.classList.add(LoadStateClass.ERROR);
};
}

Expand Down
19 changes: 1 addition & 18 deletions src/amp-story-player-manager.js
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import {AmpStoryPlayer} from './amp-story-player-impl';
import {throttle} from './utils/rate-limit';

/** @const {string} */
Expand All @@ -33,9 +32,8 @@ export class AmpStoryPlayerManager {
/**
* Calls layoutCallback on the player when it is close to the viewport.
* @param {!AmpStoryPlayer} playerImpl
* @private
*/
layoutPlayer_(playerImpl) {
layoutWhenVisible(playerImpl) {
if (!this.win_.IntersectionObserver || this.win_ !== this.win_.parent) {
this.layoutFallback_(playerImpl);
return;
Expand Down Expand Up @@ -90,19 +88,4 @@ export class AmpStoryPlayerManager {
playerImpl.layoutCallback();
}
}

/**
* Builds and layouts the players when appropiate.
* @public
*/
loadPlayers() {
const doc = this.win_.document;
const players = doc.getElementsByTagName('amp-story-player');
for (let i = 0; i < players.length; i++) {
const playerEl = players[i];
const player = new AmpStoryPlayer(this.win_, playerEl);
player.buildCallback();
this.layoutPlayer_(player);
}
}
}
9 changes: 4 additions & 5 deletions src/amp-story-player.js
Expand Up @@ -14,9 +14,8 @@
* limitations under the License.
*/

import {AmpStoryPlayerManager} from './amp-story-player-manager';
import {AmpStoryPlayer} from './amp-story-player-impl';

self.onload = () => {
const manager = new AmpStoryPlayerManager(self);
manager.loadPlayers();
};
self.addEventListener('load', () => {
customElements.define('amp-story-player', AmpStoryPlayer);
});
78 changes: 45 additions & 33 deletions test/unit/test-amp-story-player.js
Expand Up @@ -14,35 +14,30 @@
* limitations under the License.
*/

import {AmpStoryPlayerManager} from '../../src/amp-story-player-manager';
import {IFRAME_IDX} from '../../src/amp-story-player-impl';
import {AmpStoryPlayer, IFRAME_IDX} from '../../src/amp-story-player-impl';
import {Messaging} from '@ampproject/viewer-messaging';
import {toArray} from '../../src/types';

describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {
let win;
let playerEl;
let url;
let manager;

const fireHandler = [];
const DEFAULT_URL =
'https://www-washingtonpost-com.cdn.ampproject.org/v/s/www.washingtonpost.com/graphics/2019/lifestyle/travel/amp-stories/a-locals-guide-to-what-to-eat-and-do-in-new-york-city/';
let fakeMessaging;
let messagingMock;
let oldPrototype;

function buildStoryPlayer(numStories = 1) {
function buildStoryPlayer(numStories = 1, url = DEFAULT_URL) {
playerEl = win.document.createElement('amp-story-player');
for (let i = 0; i < numStories; i++) {
const storyAnchor = win.document.createElement('a');
url =
'https://www-washingtonpost-com.cdn.ampproject.org/v/s/www.washingtonpost.com/graphics/2019/lifestyle/travel/amp-stories/a-locals-guide-to-what-to-eat-and-do-in-new-york-city/';
storyAnchor.setAttribute('href', url);
playerEl.appendChild(storyAnchor);
}
win.document.body.appendChild(playerEl);
manager = new AmpStoryPlayerManager(win);

env.sandbox
.stub(Messaging, 'waitForHandshakeFromDocument')
.resolves(fakeMessaging);
win.document.body.appendChild(playerEl);
}

function swipeLeft() {
Expand All @@ -69,6 +64,24 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {

beforeEach(() => {
win = env.win;

// By the time this code runs, the outer window has already defined
// AmpStoryPlayer in the import, which still references the HTMLElement
// native function it extends. Then the `custom-elements.js` polyfill
// replaces it in the inner window `realWin`, but by this point it's no good
// because it's already been accessed, throwing an error.
//
// By using `Object.setPrototypeOf(AmpStoryPlayer, win.HTMLElement)`, we
// make `AmpStoryPlayer` native to the current inner `realWin` window,
// making sure it uses the polyfill and not the native function.
//
// Doing this causes the class to be poisoned, so we reset it on every test
// run.
oldPrototype = Object.getPrototypeOf(AmpStoryPlayer);
Object.setPrototypeOf(AmpStoryPlayer, win.HTMLElement);
Object.setPrototypeOf(AmpStoryPlayer.prototype, win.HTMLElement.prototype);
win.customElements.define('amp-story-player', AmpStoryPlayer);

fakeMessaging = {
setDefaultHandler: () => {},
sendRequest: () => {},
Expand All @@ -78,47 +91,51 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {
},
};
messagingMock = env.sandbox.mock(fakeMessaging);
env.sandbox
.stub(Messaging, 'waitForHandshakeFromDocument')
.resolves(fakeMessaging);
});

afterEach(() => {
messagingMock.verify();
Object.setPrototypeOf(AmpStoryPlayer, oldPrototype);
Object.setPrototypeOf(AmpStoryPlayer.prototype, oldPrototype.prototype);
});

it('should build an iframe for each story', () => {
buildStoryPlayer();
manager.loadPlayers();

expect(playerEl.shadowRoot.querySelector('iframe')).to.exist;
});

it('should correctly append params at the end of the story url', () => {
buildStoryPlayer();
manager.loadPlayers();

const storyIframe = playerEl.shadowRoot.querySelector('iframe');

expect(storyIframe.getAttribute('src')).to.equals(
url +
'?amp_js_v=0.1#visibilityState=visible&origin=about%3Asrcdoc&showStoryUrlInfo=0&storyPlayer=v0&cap=swipe'
DEFAULT_URL +
'?amp_js_v=0.1#visibilityState=visible&origin=http%3A%2F%2Flocalhost' +
'%3A9876&showStoryUrlInfo=0&storyPlayer=v0&cap=swipe'
);
});

it('should correctly append params at the end of a story url with existing params', () => {
buildStoryPlayer();
url += '?testParam=true#myhash=hashValue';
playerEl.firstElementChild.setAttribute('href', url);
it('should correctly append params at the end of a story url with existing params', async () => {
const existingParams = '?testParam=true#myhash=hashValue';
buildStoryPlayer(1, DEFAULT_URL + existingParams);

manager.loadPlayers();
const storyIframe = playerEl.shadowRoot.querySelector('iframe');

expect(storyIframe.getAttribute('src')).to.equals(
url +
'&amp_js_v=0.1#visibilityState=visible&origin=about%3Asrcdoc&showStoryUrlInfo=0&storyPlayer=v0&cap=swipe'
DEFAULT_URL +
existingParams +
'&amp_js_v=0.1#visibilityState=visible&origin=http%3A%2F%2Flocalhost' +
'%3A9876&showStoryUrlInfo=0&storyPlayer=v0&cap=swipe'
);
});

it('should set first story as visible', () => {
buildStoryPlayer(3);
manager.loadPlayers();

const storyIframes = playerEl.shadowRoot.querySelectorAll('iframe');
expect(storyIframes[0].getAttribute('src')).to.include(
Expand All @@ -128,7 +145,6 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {

it('should prerender next stories', () => {
buildStoryPlayer(3);
manager.loadPlayers();

const storyIframes = playerEl.shadowRoot.querySelectorAll('iframe');
expect(storyIframes[1].getAttribute('src')).to.include(
Expand All @@ -143,7 +159,7 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {
buildStoryPlayer(4);
const stories = toArray(playerEl.querySelectorAll('a'));

await manager.loadPlayers();
await Promise.resolve(); // Microtask tick.

swipeLeft();
expect(stories[0][IFRAME_IDX]).to.eql(0);
Expand All @@ -162,7 +178,7 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {
buildStoryPlayer(4);
const stories = toArray(playerEl.querySelectorAll('a'));

await manager.loadPlayers();
await Promise.resolve(); // Microtask tick.

swipeLeft();
swipeLeft();
Expand All @@ -181,15 +197,11 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {
messagingMock.expects('registerHandler').withArgs('touchmove');
messagingMock.expects('registerHandler').withArgs('touchend');
messagingMock.expects('setDefaultHandler');

await manager.loadPlayers();
});

it('should navigate to next story when the last page of a story is tapped', async () => {
buildStoryPlayer(2);

await manager.loadPlayers();

const fakeData = {next: true};
fireHandler['selectDocument']('selectDocument', fakeData);

Expand All @@ -202,7 +214,7 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {

it('should navigate when swiping', async () => {
buildStoryPlayer(4);
await manager.loadPlayers();
await Promise.resolve(); // Microtask tick.

swipeLeft();

Expand All @@ -215,7 +227,7 @@ describes.realWin('AmpStoryPlayer', {amp: false}, (env) => {

it('should not navigate when swiping last story', async () => {
buildStoryPlayer(2);
await manager.loadPlayers();
await Promise.resolve(); // Microtask tick.

swipeLeft();
swipeLeft();
Expand Down

0 comments on commit df04d54

Please sign in to comment.