🐛 Prevent toggle() from throwing error when no elements are selected. (…

…ampproject#20795)
noranazmy · Mar 22, 2019 · 33a14b4 · 33a14b4
1 parent 101a7f6
commit 33a14b4
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 2 deletions.
diff --git a/examples/selector.amp.html b/examples/selector.amp.html
@@ -0,0 +1,59 @@
+<!doctype html>
+<html ⚡>
+<head>
+  <meta charset="utf-8">
+  <script async src="https://cdn.ampproject.org/v0.js"></script>
+  <title>AMP Selector Demo</title>
+  <link rel="canonical" href="amps.html">
+  <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
+  <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>
+  <script async custom-element="amp-selector" src="https://cdn.ampproject.org/v0/amp-selector-0.1.js"></script>
+</head>
+<body>
+    <h1>Selector Examples</h1>
+    <h2>Single Select</h2>
+    <amp-selector class="sample-selector"
+        layout="container">
+    <ul>
+        <li option="1">Option 1</li>
+        <li option="2">Option 2</li>
+        <li option="3">Option 3</li>
+        <li option="4">Option 4</li>
+    </ul>
+    </amp-selector>
+
+    <h2>Multi Select</h2>
+    <amp-selector layout="container"
+    class="sample-selector"
+    multiple>
+    <ul>
+        <li option="1">Option 1</li>
+        <li option="2">Option 2</li>
+        <li option="3">Option 3</li>
+        <li option="4">Option 4</li>
+    </ul>
+    </amp-selector>
+
+    <h2>Actions</h2>
+    <amp-selector id="actionsSample"
+  layout="container"
+  class="sample-selector"
+  multiple>
+  <ul>
+        <li option="1">Option 1</li>
+        <li option="2">Option 2</li>
+        <li option="3">Option 3</li>
+        <li option="4">Option 4</li>
+        <li option="5">Option 5</li>
+        <li option="6">Option 6</li>
+    </ul>
+    </amp-selector>
+<button on="tap:actionsSample.clear">clear</button>
+<button on="tap:actionsSample.selectUp">selectUp</button>
+<button on="tap:actionsSample.selectUp(delta=2)">selectUp(delta=2)</button>
+<button on="tap:actionsSample.selectDown">selectDown</button>
+<button on="tap:actionsSample.selectDown(delta=2)">selectDown(delta=2)</button>
+<button on="tap:actionsSample.toggle(index=1)">toggle(index=1)</button>
+<button on="tap:actionsSample.toggle(index=1, value=true)">toggle(index=1, value=true)</button>
+</body>
+</html>
diff --git a/extensions/amp-selector/0.1/amp-selector.js b/extensions/amp-selector/0.1/amp-selector.js
@@ -396,7 +396,10 @@ export class AmpSelector extends AMP.BaseElement {
     // There is a change of the `selected` attribute for the element
     if (selectedIndex !== index) {
       this.setSelection_(el);
-      this.clearSelection_(this.elements_[selectedIndex]);
+      const selectedEl = this.elements_[selectedIndex];
+      if (selectedEl) {
+        this.clearSelection_(selectedEl);
+      }
     } else {
       this.clearSelection_(el);
     }
@@ -435,7 +438,10 @@ export class AmpSelector extends AMP.BaseElement {
     const normalizedIndex = mod(index, this.elements_.length);
 
     this.setSelection_(this.elements_[normalizedIndex]);
-    this.clearSelection_(this.elements_[previousIndex]);
+    const previousEl = this.elements_[previousIndex];
+    if (previousEl) {
+      this.clearSelection_(previousEl);
+    }
   }
 
   /**