Reopen page attachment from browser history. (ampproject#20292)

* Reopen attachment from browser history. * Using enum. * Circular dependency.
noranazmy · Mar 22, 2019 · 53833a8 · 53833a8
1 parent 9ff52c4
commit 53833a8
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 50 deletions.
diff --git a/extensions/amp-story/1.0/amp-story-page-attachment.js b/extensions/amp-story/1.0/amp-story-page-attachment.js
@@ -14,16 +14,23 @@
  * limitations under the License.
  */
 
-import {Action, getStoreService} from './amp-story-store-service';
+import {
+  Action,
+  StateProperty,
+  getStoreService,
+} from './amp-story-store-service';
 import {CSS} from '../../../build/amp-story-page-attachment-header-1.0.css';
+import {
+  HistoryState,
+  createShadowRootWithStyle,
+  setHistoryState,
+} from './utils';
 import {Layout} from '../../../src/layout';
 import {closest} from '../../../src/dom';
-import {createShadowRootWithStyle} from './utils';
 import {dev} from '../../../src/log';
 import {htmlFor} from '../../../src/static-template';
 import {resetStyles, setImportantStyles, toggle} from '../../../src/style';
 
-
 /** @const {number} */
 const TOGGLE_THRESHOLD_PX = 50;
 
@@ -356,9 +363,9 @@ export class AmpStoryPageAttachment extends AMP.BaseElement {
 
   /**
    * Fully opens the attachment from its current position.
-   * @public
+   * @param {boolean=} shouldAnimate
    */
-  open() {
+  open(shouldAnimate = true) {
     if (this.state_ === AttachmentState.OPEN) {
       return;
     }
@@ -370,9 +377,23 @@ export class AmpStoryPageAttachment extends AMP.BaseElement {
 
     this.mutateElement(() => {
       resetStyles(this.element, ['transform', 'transition']);
+
+      if (!shouldAnimate) {
+        // Resets the 'transition' property, and removes this override in the
+        // next frame, after the element is positioned.
+        setImportantStyles(this.element, {transition: 'initial'});
+        this.mutateElement(() => resetStyles(this.element, ['transition']));
+      }
+
       this.element.classList.add('i-amphtml-story-page-attachment-open');
       toggle(dev().assertElement(this.containerEl_), true);
     });
+
+    setHistoryState(
+        this.win,
+        HistoryState.ATTACHMENT_PAGE_ID,
+        /** @type {string} */
+        (this.storeService_.get(StateProperty.CURRENT_PAGE_ID)));
   }
 
   /**
@@ -397,5 +418,7 @@ export class AmpStoryPageAttachment extends AMP.BaseElement {
       setTimeout(
           () => toggle(dev().assertElement(this.containerEl_), false), 250);
     });
+
+    setHistoryState(this.win, HistoryState.ATTACHMENT_PAGE_ID, null);
   }
 }
diff --git a/extensions/amp-story/1.0/amp-story-page.js b/extensions/amp-story/1.0/amp-story-page.js
@@ -965,14 +965,28 @@ export class AmpStoryPage extends AMP.BaseElement {
       this.openAttachmentEl_ = buildOpenAttachmentElement(this.element);
       this.openAttachmentEl_
           .querySelector('.i-amphtml-story-page-open-attachment-text')
-          .addEventListener('click', () => {
-            attachmentEl.getImpl().then(attachment => attachment.open());
-          });
+          .addEventListener('click', () => this.openAttachment());
+
       this.mutateElement(
           () => this.element.appendChild(this.openAttachmentEl_));
     }
   }
 
+  /**
+   * Opens the attachment, if any.
+   * @param {boolean=} shouldAnimate
+   */
+  openAttachment(shouldAnimate = true) {
+    const attachmentEl =
+        this.element.querySelector('amp-story-page-attachment');
+
+    if (!attachmentEl) {
+      return;
+    }
+
+    attachmentEl.getImpl().then(attachment => attachment.open(shouldAnimate));
+  }
+
   /**
    * check to see if this page is a wrapper for an ad
    * @return {boolean}

diff --git a/extensions/amp-story/1.0/amp-story.js b/extensions/amp-story/1.0/amp-story.js
@@ -51,6 +51,13 @@ import {CSS} from '../../../build/amp-story-1.0.css';
 import {CommonSignals} from '../../../src/common-signals';
 import {EventType, dispatch} from './events';
 import {Gestures} from '../../../src/gesture';
+import {
+  HistoryState,
+  getHistoryState,
+  removeAttributeInMutate,
+  setAttributeInMutate,
+  setHistoryState,
+} from './utils';
 import {InfoDialog} from './amp-story-info-dialog';
 import {Keys} from '../../../src/utils/key-codes';
 import {Layout} from '../../../src/layout';
@@ -91,10 +98,8 @@ import {findIndex} from '../../../src/utils/array';
 import {getConsentPolicyState} from '../../../src/consent';
 import {getDetail} from '../../../src/event-helper';
 import {getMode} from '../../../src/mode';
-import {getState} from '../../../src/history';
 import {isExperimentOn} from '../../../src/experiments';
 import {registerServiceBuilder} from '../../../src/service';
-import {removeAttributeInMutate, setAttributeInMutate} from './utils';
 import {upgradeBackgroundAudio} from './audio';
 import LocalizedStringsAr from './_locales/ar';
 import LocalizedStringsDe from './_locales/de';
@@ -144,12 +149,6 @@ const Attributes = {
   SUPPORTS_LANDSCAPE: 'supports-landscape',
 };
 
-/** @enum {string} */
-const HistoryStates = {
-  PAGE_ID: 'ampStoryPageId',
-  BOOKEND_ACTIVE: 'ampStoryBookendActive',
-};
-
 /**
  * The duration of time (in milliseconds) to wait for a page to be loaded,
  * before the story becomes visible.
@@ -765,7 +764,7 @@ export class AmpStory extends AMP.BaseElement {
         this.element.querySelector('amp-story-page'),
         'Story must have at least one page.');
 
-    const initialPageId = this.getHistoryState_(HistoryStates.PAGE_ID) ||
+    const initialPageId = getHistoryState(this.win, HistoryState.PAGE_ID) ||
         firstPageEl.id;
 
     this.initializeSidebar_();
@@ -786,7 +785,7 @@ export class AmpStory extends AMP.BaseElement {
         .then(() => this.initializeBookend_())
         .then(() => {
           const bookendInHistory =
-              !!this.getHistoryState_(HistoryStates.BOOKEND_ACTIVE);
+              !!getHistoryState(this.win, HistoryState.BOOKEND_ACTIVE);
           if (bookendInHistory) {
             return this.hasBookend_().then(hasBookend => {
               if (hasBookend) {
@@ -798,6 +797,13 @@ export class AmpStory extends AMP.BaseElement {
         .then(() => this.switchTo_(initialPageId, NavigationDirection.NEXT))
         .then(() => this.updateViewportSizeStyles_())
         .then(() => {
+          const shouldReOpenAttachmentForPageId =
+              getHistoryState(this.win, HistoryState.ATTACHMENT_PAGE_ID);
+
+          if (shouldReOpenAttachmentForPageId === this.activePage_.element.id) {
+            this.activePage_.openAttachment(false /** shouldAnimate */);
+          }
+
           // Preloads and prerenders the share menu if mobile, where the share
           // button is visible.
           const uiState = this.storeService_.get(StateProperty.UI_STATE);
@@ -1344,7 +1350,7 @@ export class AmpStory extends AMP.BaseElement {
       return;
     }
 
-    this.setHistoryState_(HistoryStates.PAGE_ID, pageId);
+    setHistoryState(this.win, HistoryState.PAGE_ID, pageId);
   }
 
   /**
@@ -1693,36 +1699,7 @@ export class AmpStory extends AMP.BaseElement {
   onBookendStateUpdate_(isActive) {
     this.toggleElementsOnBookend_(/* display */ isActive);
     this.element.classList.toggle('i-amphtml-story-bookend-active', isActive);
-    this.setHistoryState_(HistoryStates.BOOKEND_ACTIVE, isActive);
-  }
-
-  /**
-   * Updates the value for a given state in the window history.
-   * @param {string} stateName
-   * @param {string|boolean} value
-   * @private
-   */
-  setHistoryState_(stateName, value) {
-    const {history} = this.win;
-    const state = getState(history) || {};
-    const newHistory = Object.assign({}, /** @type {!Object} */ (state),
-        {[stateName]: value});
-
-    history.replaceState(newHistory, '');
-  }
-
-  /**
-   * Returns the value of a given state of the window history.
-   * @param {string} stateName
-   * @return {?string}
-   * @private
-   */
-  getHistoryState_(stateName) {
-    const {history} = this.win;
-    if (history && getState(history)) {
-      return getState(history)[stateName];
-    }
-    return null;
+    setHistoryState(this.win, HistoryState.BOOKEND_ACTIVE, isActive);
   }
 
   /**
@@ -1898,7 +1875,8 @@ export class AmpStory extends AMP.BaseElement {
    * @private
    */
   buildAndPreloadBookend_() {
-    this.bookend_.build(!!this.getHistoryState_(HistoryStates.BOOKEND_ACTIVE));
+    this.bookend_
+        .build(!!getHistoryState(this.win, HistoryState.BOOKEND_ACTIVE));
     return this.bookend_.loadConfigAndMaybeRenderBookend();
   }
 

diff --git a/extensions/amp-story/1.0/utils.js b/extensions/amp-story/1.0/utils.js
@@ -18,6 +18,7 @@ import {closestBySelector} from '../../../src/dom';
 import {createShadowRoot} from '../../../src/shadow-embed';
 import {getMode} from '../../../src/mode';
 import {getSourceOrigin} from '../../../src/url';
+import {getState} from '../../../src/history';
 import {user, userAssert} from '../../../src/log';
 
 /**
@@ -223,3 +224,39 @@ export function getSourceOriginForElement(element, url) {
   }
   return domainName;
 }
+
+/** @enum {string} */
+export const HistoryState = {
+  ATTACHMENT_PAGE_ID: 'ampStoryAttachmentPageId',
+  BOOKEND_ACTIVE: 'ampStoryBookendActive',
+  PAGE_ID: 'ampStoryPageId',
+};
+
+/**
+ * Updates the value for a given state in the window history.
+ * @param {!Window} win
+ * @param {string} stateName
+ * @param {string|boolean|null} value
+ */
+export function setHistoryState(win, stateName, value) {
+  const {history} = win;
+  const state = getState(history) || {};
+  const newHistory = Object.assign({}, /** @type {!Object} */ (state),
+      {[stateName]: value});
+
+  history.replaceState(newHistory, '');
+}
+
+/**
+ * Returns the value of a given state of the window history.
+ * @param {!Window} win
+ * @param {string} stateName
+ * @return {?string}
+ */
+export function getHistoryState(win, stateName) {
+  const {history} = win;
+  if (history && getState(history)) {
+    return getState(history)[stateName];
+  }
+  return null;
+}