make sure outlinks still work

ampproject · Feb 20, 2019 · ca62b59 · ca62b59
1 parent c37f7ee
commit ca62b59
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 11 deletions.
diff --git a/examples/visual-tests/amp-story/sidebar-link.html b/examples/visual-tests/amp-story/sidebar-link.html
@@ -0,0 +1,79 @@
+<!doctype html>
+<html amp lang="en">
+  <head>
+    <meta charset="utf-8">
+    <script async src="https://cdn.ampproject.org/v0.js"></script>
+    <script async custom-element="amp-story"
+        src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
+    <script async custom-element="amp-sidebar" src="https://cdn.ampproject.org/v0/amp-sidebar-0.1.js"></script>
+    <title>amp-story sidebar visual diff</title>
+    <meta name="viewport"
+          content="width=device-width,minimum-scale=1,initial-scale=1">
+    <link rel="canonical" href="./template-vertical.html">
+    <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
+    <style amp-custom>
+      amp-story-page {
+        background: black;
+        color: white;
+        font-family: sans-serif;
+      }
+      .i-amphtml-story-spinner-container {
+        /* Hide the spinner for loading pages, since this causes flakes */
+        display: none;
+      }
+      /*
+       * The share pill text does not show in the same place consistently, and
+       * as such causes the visual tests to be flaky.  This hides the share pill
+       * with visibility: hidden, so as to keep other elements in the same place
+       * that they would otherwise be, while deflaking the tests.  See #19890.
+       */
+       .i-amphtml-story-share-pill {
+        visibility: hidden;
+      }
+      h1.hello-world {
+        color: white;
+        text-shadow: 3px 3px rgba(0, 0, 0, 0.5);
+      }
+    </style>
+  </head>
+
+  <body>
+    <amp-story
+        id="story"
+        standalone publisher="AMP Team" title="Visual Diff Test"
+        publisher-logo-src="https://www.ampproject.org/static/img/icons/120.png"
+        poster-portrait-src="https://picsum.photos/900/1600?image=981"
+        poster-landscape-src="https://picsum.photos/1600/900?image=981"
+        poster-square-src="https://picsum.photos/1600/1600?image=981">
+      <amp-sidebar id="sidebar1" layout="nodisplay" >
+        <ul>
+        <li on="tap:story.goToPage(id=page-2)">Link to Page 2 using action</li>
+        <li><a href="#page=page-2"> Link to Page 2 using a link </a></li>
+        </ul>
+      </amp-sidebar>
+      <amp-story-page id="cover">
+        <amp-story-grid-layer template="fill">
+          <amp-video autoplay layout="fill" poster="https://picsum.photos/1600/900?image=980">
+          </amp-video>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="vertical">
+          <h1 class="hello-world">Hello world!</h1>
+          <p>Page one of two</p>
+          <button on="tap:story.goToPage(id=page-2)"> click me </button>
+        </amp-story-grid-layer>
+      </amp-story-page>
+
+      <amp-story-page id="page-2">
+        <amp-story-grid-layer template="fill">
+          <amp-video autoplay layout="fill" poster="https://picsum.photos/1600/900?image=981">
+          </amp-video>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="vertical">
+          <h1 class="hello-world">Hello world!</h1>
+          <p>Page two of two</p>
+        </amp-story-grid-layer>
+      </amp-story-page>
+
+    </amp-story>
+  </body>
+
diff --git a/extensions/amp-story/1.0/amp-story.js b/extensions/amp-story/1.0/amp-story.js
@@ -416,7 +416,8 @@ export class AmpStory extends AMP.BaseElement {
             this.switchTo_(args['id'], NavigationDirection.NEXT);
             this.closeOpacityMask_();
           }
-        } this.switchTo_(args['id'], NavigationDirection.NEXT);
+        }
+        this.switchTo_(args['id'], NavigationDirection.NEXT);
       });
     }
   }
@@ -2241,19 +2242,22 @@ export class AmpStory extends AMP.BaseElement {
     }
     if (isExperimentOn(this.win,'amp-story-branching')) {
       this.sidebar_.addEventListener('click', e => {
-        if (e.target.tagName === 'A') {
-          // Do not let the browser scroll
-          e.preventDefault();
 
+        if (e.target.tagName === 'A') {
           const url = e.target.getAttribute('href');
-          // Handle for absolute URLs, in addition to fragments only.
-          this.win.location.hash = url.slice(url.search('\#(.*)'));
+          if (url.indexOf('#page=') >= 0) {
+            // Do not let the browser scroll
+            e.preventDefault();
 
-          const actions = Services.actionServiceForDoc(this.element);
-          actions.execute(dev().assertElement(this.sidebar_),
-              'close', /* args */ null, /* source */ null, /* caller */ null,
-              /* event */ null, ActionTrust.HIGH);
-          this.closeOpacityMask_();
+            // Handle for absolute URLs, in addition to fragments only.
+            this.win.location.hash = url.slice(url.search('\#(.*)'));
+
+            const actions = Services.actionServiceForDoc(this.element);
+            actions.execute(dev().assertElement(this.sidebar_),
+                'close', /* args */ null, /* source */ null, /* caller */ null,
+                /* event */ null, ActionTrust.HIGH);
+            this.closeOpacityMask_();
+          }
         }
       });
     }