Validate amp-base-carousel and friends to enable loop on empty va…

…lue (#35568) * Update validator rules to allow empty value for loop * Allow empty string in 0.1 * Update docs and examples * Add closing tag * Update out
ampproject · Aug 11, 2021 · 47c5394 · 47c5394
1 parent f062e8d
commit 47c5394
Show file tree

Hide file tree

Showing 17 changed files with 110 additions and 71 deletions.
diff --git a/examples/bento/lazy-v0.html b/examples/bento/lazy-v0.html
@@ -46,7 +46,7 @@
     ></amp-social-share>
   </div>
 
-  <amp-base-carousel id="carousel-1" width="400" height="300" layout="responsive" loop="true">
+  <amp-base-carousel id="carousel-1" width="400" height="300" layout="responsive" loop>
     <img src="img/hero@1x.jpg">
     <img src="img/sea@1x.jpg">
   </amp-base-carousel>

diff --git a/extensions/amp-base-carousel/0.1/amp-base-carousel.js b/extensions/amp-base-carousel/0.1/amp-base-carousel.js
@@ -137,7 +137,7 @@ class AmpCarousel extends AMP.BaseElement {
         this.carousel_.updateHorizontal(newValue === 'true');
       },
       'loop': (newValue) => {
-        this.carousel_.updateLoop(newValue === 'true');
+        this.carousel_.updateLoop(newValue === 'true' || newValue === '');
       },
       'mixed-length': (newValue) => {
         this.carousel_.updateMixedLength(newValue === 'true');

diff --git a/extensions/amp-base-carousel/0.1/test/validator-amp-base-carousel.html b/extensions/amp-base-carousel/0.1/test/validator-amp-base-carousel.html
@@ -70,6 +70,9 @@
     <!-- Valid: integer values with multiple media queries -->
     <amp-base-carousel width="4" height="3" advance-count="(min-width: 800px) -3, (max-height: 1000px) 3, 1">
     </amp-base-carousel>
+    <!-- Valid: no value for loop -->
+    <amp-base-carousel width="4" height="3" loop>
+    </amp-base-carousel>
     <!-- Invalid: no default value-->
     <amp-base-carousel width="4" height="3" advance-count="(min-width: 800px) 3">
     </amp-base-carousel>

diff --git a/extensions/amp-base-carousel/0.1/test/validator-amp-base-carousel.out b/extensions/amp-base-carousel/0.1/test/validator-amp-base-carousel.out
@@ -79,25 +79,28 @@ amp-base-carousel/0.1/test/validator-amp-base-carousel.html:55:4 The attribute '
 |      <!-- Valid: integer values with multiple media queries -->
 |      <amp-base-carousel width="4" height="3" advance-count="(min-width: 800px) -3, (max-height: 1000px) 3, 1">
 |      </amp-base-carousel>
+|      <!-- Valid: no value for loop -->
+|      <amp-base-carousel width="4" height="3" loop>
+|      </amp-base-carousel>
 |      <!-- Invalid: no default value-->
 |      <amp-base-carousel width="4" height="3" advance-count="(min-width: 800px) 3">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:74:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) 3'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:77:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) 3'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: no value -->
 |      <amp-base-carousel width="4" height="3" advance-count>
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:77:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value ''. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:80:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value ''. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: floating point value -->
 |      <amp-base-carousel width="4" height="3" advance-count="3.2">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:80:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value '3.2'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:83:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value '3.2'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: floating point value in media query -->
 |      <amp-base-carousel width="4" height="3" advance-count="(min-width: 800px) -3, (max-height: 1000px) 3.2, 1">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:83:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) -3, (max-height: 1000px) 3.2, 1'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:86:4 The attribute 'advance-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) -3, (max-height: 1000px) 3.2, 1'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |    </section>
 |
@@ -118,27 +121,27 @@ amp-base-carousel/0.1/test/validator-amp-base-carousel.html:83:4 The attribute '
 |      <!-- Invalid: negative integer value -->
 |      <amp-base-carousel width="4" height="3" visible-count="-2">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:102:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value '-2'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:105:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value '-2'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: no default value-->
 |      <amp-base-carousel width="4" height="3" visible-count="(min-width: 800px) 3">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:105:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) 3'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:108:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) 3'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: no value -->
 |      <amp-base-carousel width="4" height="3" visible-count>
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:108:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value ''. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:111:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value ''. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: string value -->
 |      <amp-base-carousel width="4" height="3" visible-count="foo">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:111:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value 'foo'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:114:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value 'foo'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |      <!-- Invalid: negative value in media query -->
 |      <amp-base-carousel width="4" height="3" visible-count="(min-width: 800px) -3, (max-height: 1000px) 3, 1">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:114:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) -3, (max-height: 1000px) 3, 1'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:117:4 The attribute 'visible-count' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px) -3, (max-height: 1000px) 3, 1'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |    </section>
 |
@@ -153,7 +156,7 @@ amp-base-carousel/0.1/test/validator-amp-base-carousel.html:114:4 The attribute
 |      <!-- Invalid: no space between media query and value -->
 |      <amp-base-carousel width="4" height="3" auto-advance="(min-width: 800px)true, false">
 >>     ^~~~~~~~~
-amp-base-carousel/0.1/test/validator-amp-base-carousel.html:127:4 The attribute 'auto-advance' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px)true, false'. (see https://amp.dev/documentation/components/amp-base-carousel/)
+amp-base-carousel/0.1/test/validator-amp-base-carousel.html:130:4 The attribute 'auto-advance' in tag 'amp-base-carousel' is set to the invalid value '(min-width: 800px)true, false'. (see https://amp.dev/documentation/components/amp-base-carousel/)
 |      </amp-base-carousel>
 |    </section>
 |  </body>

diff --git a/extensions/amp-base-carousel/1.0/test/validator-amp-base-carousel.html b/extensions/amp-base-carousel/1.0/test/validator-amp-base-carousel.html
@@ -42,6 +42,9 @@
     <!-- Valid: booleans with multiple media queries -->
     <amp-base-carousel width="4" height="3" auto-advance="(min-width: 800px) true, (max-height: 1000px) true, false">
     </amp-base-carousel>
+    <!-- Valid: no value for loop -->
+    <amp-base-carousel width="4" height="3" loop>
+    </amp-base-carousel>
     <!-- Invalid: no default value-->
     <amp-base-carousel width="4" height="3" auto-advance="(min-width: 800px) true">
     </amp-base-carousel>