Skip to content

Commit

Permalink
[amp-story-player] Fix duplicate player loading scenario (#32971)
Browse files Browse the repository at this point in the history
* solve for the duplicate load() case

* console log

* more docs

* docs revision

* add missing public api
  • Loading branch information
Enriqe committed Mar 19, 2021
1 parent add3b6b commit f2b85d9
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 23 deletions.
15 changes: 9 additions & 6 deletions spec/amp-story-player.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,23 @@ URL pointing to the story.

## Programmatic Control

Call the player's various methods to programmatically control the player. These methods are exposed on the HTML element, `const playerEl = document.querySelector('amp-story-player')` and on instances of the global class variable, `const player = new AmpStoryPlayer(window, playerEl)`.
Call the player's various methods to programmatically control the player. These methods are exposed on the HTML element, `const playerEl = document.querySelector('amp-story-player')`.

### Methods

#### load

Will initialize the player manually. This can be useful when the player is dynamically.
Will initialize the player manually. This can be useful when creating the player dynamically.

Note that the element must be connected to the DOM before calling `load()`.
Note that the amp-story-player JS will automatically do this when the player is already in the HTML markup, so only do this when you really need to.

Also note that the element must be connected to the DOM before calling `load()`.

```javascript
const playerEl = document.body.querySelector('amp-story-player');
const player = new AmpStoryPlayer(window, playerEl);
player.load();
const playerEl = document.createElement('amp-story-player');
new AmpStoryPlayer(window, playerEl);
document.body.appendChild(playerEl);
playerEl.load();
```

#### go
Expand Down
12 changes: 5 additions & 7 deletions src/amp-story-player/amp-story-component-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,16 @@ export class AmpStoryComponentManager {

/**
* Calls layoutCallback on the element when it is close to the viewport.
* @param {!AmpStoryPlayer|!AmpStoryEntryPoint} elImpl
* @param {!Element} element
* @private
*/
layoutEl_(elImpl) {
new AmpStoryPlayerViewportObserver(
this.win_,
elImpl.getElement(),
elImpl.layoutCallback.bind(elImpl)
layoutEl_(element) {
new AmpStoryPlayerViewportObserver(this.win_, element, () =>
element.layoutCallback()
);

const scrollHandler = () => {
elImpl.layoutCallback();
element.layoutCallback();
this.win_.removeEventListener('scroll', scrollHandler);
};

Expand Down
22 changes: 12 additions & 10 deletions src/amp-story-player/amp-story-player-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,6 @@ export class AmpStoryPlayer {
/** @private {?Element} */
this.rootEl_ = null;

/** @private {boolean} */
this.isLaidOut_ = false;

/** @private {boolean} */
this.isBuilt_ = false;

/** @private {number} */
this.currentIdx_ = 0;

Expand Down Expand Up @@ -241,13 +235,18 @@ export class AmpStoryPlayer {

/** @private {boolean} */
this.autoplay_ = true;

return this.element_;
}

/**
* Attaches callbacks to the DOM element for them to be used by publishers.
* @private
*/
attachCallbacksToElement_() {
this.element_.buildCallback = this.buildCallback.bind(this);
this.element_.layoutCallback = this.layoutCallback.bind(this);
this.element_.getElement = this.getElement.bind(this);
this.element_.getStories = this.getStories.bind(this);
this.element_.load = this.load.bind(this);
this.element_.show = this.show.bind(this);
Expand All @@ -271,6 +270,9 @@ export class AmpStoryPlayer {
`[${TAG}] element must be connected to the DOM before calling load().`
);
}
if (!!this.element_.isBuilt_) {
throw new Error(`[${TAG}] calling load() on an already loaded element.`);
}
this.buildCallback();
this.layoutCallback();
}
Expand Down Expand Up @@ -364,7 +366,7 @@ export class AmpStoryPlayer {

/** @public */
buildCallback() {
if (this.isBuilt_) {
if (!!this.element_.isBuilt_) {
return;
}

Expand All @@ -378,7 +380,7 @@ export class AmpStoryPlayer {
this.initializePageScroll_();
this.initializeCircularWrapping_();
this.signalReady_();
this.isBuilt_ = true;
this.element_.isBuilt_ = true;
}

/**
Expand Down Expand Up @@ -681,7 +683,7 @@ export class AmpStoryPlayer {
* @public
*/
layoutCallback() {
if (this.isLaidOut_) {
if (!!this.element_.isLaidOut_) {
return;
}

Expand All @@ -691,7 +693,7 @@ export class AmpStoryPlayer {

this.render_();

this.isLaidOut_ = true;
this.element_.isLaidOut_ = true;
}

/**
Expand Down

0 comments on commit f2b85d9

Please sign in to comment.