Skip to content

Commit

Permalink
♻️ onPageNavigation_ method in 360 component (#30683)
Browse files Browse the repository at this point in the history
* onPageNavigation_ method in 360 component.

* Mark play_ pause_ and rewind_ private.

* Remove commented out code.

* Check canAnimate in play_.
  • Loading branch information
processprocess committed Oct 15, 2020
1 parent 5fa3930 commit f73b455
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 42 deletions.
28 changes: 21 additions & 7 deletions extensions/amp-story-360/0.1/amp-story-360.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,12 +361,13 @@ export class AmpStory360 extends AMP.BaseElement {

storeService.subscribe(StateProperty.CURRENT_PAGE_ID, (currPageId) => {
this.isOnActivePage_ = currPageId === this.getPageId_();
this.onPageNavigation_();
this.maybeShowDiscoveryAnimation_();
this.maybeSetGyroscopeDefaultHeading_();
});

this.storeService_.subscribe(StateProperty.PAUSED_STATE, (isPaused) => {
isPaused ? this.pause() : this.play();
isPaused ? this.pause_() : this.play_();
});
}),

Expand Down Expand Up @@ -400,6 +401,16 @@ export class AmpStory360 extends AMP.BaseElement {
);
}

/** @private */
onPageNavigation_() {
if (this.isOnActivePage_) {
this.play_();
} else {
this.pause_();
this.rewind_();
}
}

/**
* @param {string} permissionState
* @private
Expand Down Expand Up @@ -796,13 +807,16 @@ export class AmpStory360 extends AMP.BaseElement {
this.mutateElement(() => loop());
}

/** @public */
pause() {
/** @private */
pause_() {
this.isPlaying_ = false;
}

/** @public */
play() {
/** @private */
play_() {
if (!this.canAnimate) {
return;
}
userAssert(
this.canAnimate,
'amp-story-360 is either not configured to play an animation or ' +
Expand All @@ -814,8 +828,8 @@ export class AmpStory360 extends AMP.BaseElement {
}
}

/** @public */
rewind() {
/** @private */
rewind_() {
if (!this.canAnimate) {
return;
}
Expand Down
35 changes: 0 additions & 35 deletions extensions/amp-story/1.0/amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ export const Selectors = {
// work with this current implementation.
ALL_PLAYBACK_MEDIA:
'> audio, amp-story-grid-layer audio, amp-story-grid-layer video',
ALL_STORY_360: 'amp-story-360',
ALL_VIDEO: 'amp-story-grid-layer video',
};

Expand Down Expand Up @@ -315,9 +314,6 @@ export class AmpStoryPage extends AMP.BaseElement {

/** @private {?number} Time at which an audio element failed playing. */
this.playAudioElementFromTimestamp_ = null;

/** @private {?Array<!Promise<!../../amp-story-360/0.1/amp-story-360.AmpStory360>>}*/
this.story360componentsCache_ = null;
}

/**
Expand Down Expand Up @@ -510,13 +506,6 @@ export class AmpStoryPage extends AMP.BaseElement {
if (this.animationManager_) {
this.animationManager_.cancelAll();
}

this.story360components_.forEach((componentPromise) => {
componentPromise.then((component) => {
component.pause();
component.rewind();
});
});
}

/**
Expand Down Expand Up @@ -550,13 +539,6 @@ export class AmpStoryPage extends AMP.BaseElement {
this.checkPageHasElementWithPlayback_();
this.renderOpenAttachmentUI_();
this.findAndPrepareEmbeddedComponents_();
this.story360components_.forEach((componentPromise) => {
componentPromise.then((component) => {
if (component.canAnimate) {
component.play();
}
});
});
}

this.reportDevModeErrors_();
Expand Down Expand Up @@ -1856,21 +1838,4 @@ export class AmpStoryPage extends AMP.BaseElement {
isAutoAdvance() {
return this.advancement_.isAutoAdvance();
}

/**
* @private
* @return {!Array<!Promise<!../../amp-story-360/0.1/amp-story-360.AmpStory360>>}
*/
get story360components_() {
if (!this.story360componentsCache_) {
this.story360componentsCache_ = toArray(
scopedQuerySelectorAll(this.element, Selectors.ALL_STORY_360)
).map((element) =>
whenUpgradedToCustomElement(element).then((customEl) =>
customEl.getImpl()
)
);
}
return this.story360componentsCache_;
}
}

0 comments on commit f73b455

Please sign in to comment.