Triggering the bookend through the store. (#14036)

* Removing the bookend events to rely on the store and its actions. * Use synchronous store service. * More cleaning and unit tests. * Reviews.
ampproject · Mar 16, 2018 · ecc62dd · ecc62dd
1 parent 8cbfc88
commit ecc62dd
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 134 deletions.
diff --git a/extensions/amp-story/0.1/amp-story-page.js b/extensions/amp-story/0.1/amp-story-page.js
@@ -594,8 +594,7 @@ export class AmpStoryPage extends AMP.BaseElement {
   next(opt_isAutomaticAdvance) {
     const pageId = this.getNextPageId(opt_isAutomaticAdvance);
 
-    if (pageId === null) {
-      dispatch(this.element, EventType.SHOW_BOOKEND, /* opt_bubbles */ true);
+    if (!pageId) {
       return;
     }
 

diff --git a/extensions/amp-story/0.1/amp-story-store-service.js b/extensions/amp-story/0.1/amp-story-store-service.js
@@ -66,6 +66,9 @@ export const Action = {
 const actions = (state, action, data) => {
   switch (action) {
     case Action.TOGGLE_BOOKEND:
+      if (!state[StateProperty.CAN_SHOW_BOOKEND]) {
+        return state;
+      }
       return /** @type {!State} */ (Object.assign(
           {}, state, {[StateProperty.BOOKEND_STATE]: !!data}));
     default:

diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js
@@ -27,11 +27,15 @@
 import './amp-story-auto-ads';
 import './amp-story-grid-layer';
 import './amp-story-page';
+import {
+  Action,
+  AmpStoryStoreService,
+  StateProperty,
+} from './amp-story-store-service';
 import {ActionTrust} from '../../../src/action-trust';
 import {AmpStoryAnalytics} from './analytics';
 import {AmpStoryBackground} from './background';
 import {AmpStoryHint} from './amp-story-hint';
-import {AmpStoryStoreService, StateProperty} from './amp-story-store-service';
 import {AmpStoryVariableService} from './variable-service';
 import {Bookend} from './bookend';
 import {CSS} from '../../../build/amp-story-0.1.css';
@@ -257,7 +261,7 @@ export class AmpStory extends AMP.BaseElement {
 
     /** @private {!NavigationState} */
     this.navigationState_ =
-        new NavigationState(element, () => this.hasBookend_());
+        new NavigationState(this.win, () => this.hasBookend_());
 
     /** @const @private {!../../../src/service/vsync-impl.Vsync} */
     this.vsync_ = this.getVsync();
@@ -397,18 +401,6 @@ export class AmpStory extends AMP.BaseElement {
       this.previous_();
     });
 
-    this.element.addEventListener(EventType.SHOW_BOOKEND, () => {
-      this.hasBookend_().then(hasBookend => {
-        if (hasBookend) {
-          this.showBookend_();
-        }
-      });
-    });
-
-    this.element.addEventListener(EventType.CLOSE_BOOKEND, () => {
-      this.hideBookend_();
-    });
-
     this.element.addEventListener(EventType.MUTE, () => {
       this.mute_();
     });
@@ -426,7 +418,7 @@ export class AmpStory extends AMP.BaseElement {
     });
 
     this.element.addEventListener(EventType.SWITCH_PAGE, e => {
-      if (this.bookend_.isActive()) {
+      if (this.storeService_.get(StateProperty.BOOKEND_STATE)) {
         // Disallow switching pages while the bookend is active.
         return;
       }
@@ -464,6 +456,10 @@ export class AmpStory extends AMP.BaseElement {
       this.performTapNavigation_(direction);
     });
 
+    this.storeService_.subscribe(StateProperty.BOOKEND_STATE, isActive => {
+      this.onBookendStateUpdate_(isActive);
+    });
+
     this.win.document.addEventListener('keydown', e => {
       this.onKeyDown_(e);
     }, true);
@@ -553,7 +549,7 @@ export class AmpStory extends AMP.BaseElement {
 
   /** @private */
   buildPaginationButtons_() {
-    this.paginationButtons_ = PaginationButtons.create(this.win.document);
+    this.paginationButtons_ = PaginationButtons.create(this.win);
 
     this.paginationButtons_.attach(this.element);
 
@@ -794,7 +790,11 @@ export class AmpStory extends AMP.BaseElement {
         activePage !== lastPage) {
       activePage.next(opt_isAutomaticAdvance);
     } else {
-      dispatch(this.element, EventType.SHOW_BOOKEND);
+      this.hasBookend_().then(hasBookend => {
+        if (hasBookend) {
+          this.showBookend_();
+        }
+      });
     }
   }
 
@@ -1099,20 +1099,8 @@ export class AmpStory extends AMP.BaseElement {
    * @private
    */
   showBookend_() {
-    if (this.bookend_.isActive()) {
-      return;
-    }
-
     this.buildBookend_().then(() => {
-      this.systemLayer_.hideDeveloperLog();
-
-      this.activePage_.pauseCallback();
-
-      this.toggleElementsOnBookend_(/* display */ false);
-
-      this.element.classList.add('i-amphtml-story-bookend-active');
-
-      this.bookend_.show();
+      this.storeService_.dispatch(Action.TOGGLE_BOOKEND, true);
     });
   }
 
@@ -1122,25 +1110,35 @@ export class AmpStory extends AMP.BaseElement {
    * @private
    */
   hideBookend_() {
-    if (!this.bookend_.isActive()) {
-      return;
-    }
+    this.storeService_.dispatch(Action.TOGGLE_BOOKEND, false);
+  }
 
-    this.activePage_.resumeCallback();
 
-    this.toggleElementsOnBookend_(/* display */ true);
+  /**
+   * @param {boolean} isActive
+   * @private
+   */
+  onBookendStateUpdate_(isActive) {
+    this.toggleElementsOnBookend_(/* display */ isActive);
+    this.element.classList.toggle('i-amphtml-story-bookend-active', isActive);
 
-    this.element.classList.remove('i-amphtml-story-bookend-active');
+    if (isActive) {
+      this.systemLayer_.hideDeveloperLog();
+      this.activePage_.pauseCallback();
+    }
 
-    this.bookend_.hide();
+    if (!isActive) {
+      this.activePage_.resumeCallback();
+    }
   }
 
+
   /**
    * Toggle content when bookend is opened/closed.
-   * @param {boolean} display
+   * @param {boolean} isActive
    * @private
    */
-  toggleElementsOnBookend_(display) {
+  toggleElementsOnBookend_(isActive) {
     if (!this.isDesktop_()) {
       return;
     }
@@ -1149,13 +1147,13 @@ export class AmpStory extends AMP.BaseElement {
         HIDE_ON_BOOKEND_SELECTOR);
 
     Array.prototype.forEach.call(elements, el => {
-      if (display) {
-        resetStyles(el, ['opacity', 'transition']);
-      } else {
+      if (isActive) {
         setImportantStyles(el, {
           opacity: 0,
           transition: 'opacity 0.3s',
         });
+      } else {
+        resetStyles(el, ['opacity', 'transition']);
       }
     });
   }
@@ -1330,7 +1328,7 @@ export class AmpStory extends AMP.BaseElement {
       return Promise.resolve(true);
     }
     return this.loadBookendConfig_().then(config =>
-      config && config.relatedArticles && config.relatedArticles.length);
+      !!(config && config.relatedArticles && config.relatedArticles.length));
   }
 
 
@@ -1358,26 +1356,6 @@ export class AmpStory extends AMP.BaseElement {
   }
 
 
-  /**
-   * @param {!Element} el
-   * @return {boolean}
-   * @private
-   */
-  isBookend_(el) {
-    return this.bookend_.isBuilt() && el === this.bookend_.getRoot();
-  }
-
-
-  /**
-   * @param {!Element} el
-   * @return {boolean}
-   * @private
-   */
-  isTopBar_(el) {
-    return !!this.topBar_ && this.topBar_.contains(el);
-  }
-
-
   /**
    * @param {string} id The ID of the page whose index should be retrieved.
    * @return {number} The index of the page.
@@ -1588,9 +1566,7 @@ export class AmpStory extends AMP.BaseElement {
   /** @private */
   replay_() {
     if (this.bookend_.isActive()) {
-      // Dispaching event instead of calling method directly so that all
-      // listeners can respond.
-      dispatch(this.element, EventType.CLOSE_BOOKEND);
+      this.hideBookend_();
     }
     this.switchTo_(dev().assertElement(this.pages_[0].element).id);
   }

diff --git a/extensions/amp-story/0.1/bookend.js b/extensions/amp-story/0.1/bookend.js
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import {Action, StateProperty} from './amp-story-store-service';
 import {EventType, dispatch} from './events';
 import {KeyCodes} from '../../../src/utils/key-codes';
 import {ScrollableShareWidget} from './share';
@@ -211,7 +212,10 @@ export class Bookend {
     this.closeBtn_ = null;
 
     /** @private {!ScrollableShareWidget} */
-    this.shareWidget_ = ScrollableShareWidget.create(win);
+    this.shareWidget_ = ScrollableShareWidget.create(this.win_);
+
+    /** @private @const {!./amp-story-store-service.AmpStoryStoreService} */
+    this.storeService_ = Services.storyStoreService(this.win_);
   }
 
   /**
@@ -254,9 +258,12 @@ export class Bookend {
       }
       if (e.keyCode == KeyCodes.ESCAPE) {
         e.preventDefault();
-        this.dispatchClose_();
+        this.close_();
       }
     });
+
+    this.storeService_.subscribe(
+        StateProperty.BOOKEND_STATE, isActive => this.toggle_(isActive));
   }
 
   /** @return {boolean} */
@@ -282,13 +289,15 @@ export class Bookend {
   maybeClose_(e) {
     if (this.elementOutsideUsableArea_(dev().assertElement(e.target))) {
       e.stopPropagation();
-      this.dispatchClose_();
+      this.close_();
     }
   }
 
-  /** @private */
-  dispatchClose_() {
-    dispatch(this.getRoot(), EventType.CLOSE_BOOKEND, /* opt_bubbles */ true);
+  /**
+   * Closes the bookend.
+   */
+  close_() {
+    this.storeService_.dispatch(Action.TOGGLE_BOOKEND, false);
   }
 
   /**
@@ -319,16 +328,6 @@ export class Bookend {
     }, {});
   }
 
-  /** Hides bookend with a transition. */
-  hide() {
-    this.toggle_(false);
-  }
-
-  /** Shows bookend with a transition. */
-  show() {
-    this.toggle_(true);
-  }
-
   /**
    * @param {boolean} show
    * @private

diff --git a/extensions/amp-story/0.1/events.js b/extensions/amp-story/0.1/events.js
@@ -19,12 +19,6 @@ import {createCustomEvent} from '../../../src/event-helper';
 
 /** @const {!Object<string, string>} */
 export const EventType = {
-  // Triggered when the bookend should be opened
-  SHOW_BOOKEND: 'ampstory:showbookend',
-
-  // Triggered when the user clicks the close bookend button
-  CLOSE_BOOKEND: 'ampstory:closebookend',
-
   // Triggered when the user mutes the story
   MUTE: 'ampstory:mute',