amp-bind: Support changing src on amp-list (ampproject#8735)

mrjoro · Apr 28, 2017 · dd80cbe · dd80cbe
1 parent 38fe642
commit dd80cbe
Show file tree

Hide file tree

Showing 9 changed files with 174 additions and 11 deletions.
diff --git a/build-system/app.js b/build-system/app.js
@@ -441,7 +441,6 @@ app.use('/examples/amp-fresh.amp.(min.|max.)?html', function(req, res, next) {
     next();
 });
 
-
 app.use('/impression-proxy/', function(req, res) {
   assertCors(req, res, ['GET']);
   // Fake response with the following optional fields:
@@ -672,6 +671,28 @@ app.use('/bind/ecommerce/sizes', function(req, res, next) {
   }, 1000); // Simulate network delay.
 });
 
+app.use('/list/fruit-data/get', function(req, res, next) {
+  assertCors(req, res, ['GET']);
+  res.json({
+    items: [
+      {name: 'apple', quantity: 47, unitPrice: '0.33'},
+      {name: 'pear', quantity: 538, unitPrice: '0.54'},
+      {name: 'tomato', quantity: 0, unitPrice: '0.23'},
+    ],
+  });
+});
+
+app.use('/list/vegetable-data/get', function(req, res, next) {
+  assertCors(req, res, ['GET']);
+  res.json({
+    items: [
+      {name: 'cabbage', quantity: 5, unitPrice: '1.05'},
+      {name: 'carrot', quantity: 10, unitPrice: '0.01'},
+      {name: 'brocoli', quantity: 7, unitPrice: '0.02'},
+    ],
+  });
+});
+
 // Simulated Cloudflare signed Ad server
 
 const cloudflareDataDir = '/extensions/amp-ad-network-cloudflare-impl/0.1/data';

diff --git a/examples/bind/list.html b/examples/bind/list.html
@@ -0,0 +1,27 @@
+<!doctype html>
+<html ⚡>
+<head>
+  <meta charset="utf-8">
+  <title>amp-bind: XHR</title>
+  <link rel="canonical" href="amps.html">
+  <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
+  <link href="https://fonts.googleapis.com/css?family=Questrial" rel="stylesheet" type="text/css">
+  <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 src="https://cdn.ampproject.org/v0.js"></script>
+  <script async custom-element="amp-bind" src="https://cdn.ampproject.org/v0/amp-bind-0.1.js"></script>
+  <script async custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-0.1.js"></script>
+  <script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.1.js"></script>
+</head>
+<body>
+
+  <button on="tap:AMP.setState({listSrc:'/list/fruit-data/get'})">Show Fruit</button>
+  <button on="tap:AMP.setState({listSrc:'/list/vegetable-data/get'})">Show Vegetables</button>
+
+  <amp-list layout="responsive" src="/list/fruit-data/get" [src]="listSrc || '/list/fruit-data/get'" width="600" height="600">
+    <template type="amp-mustache">
+      <strong>Product</strong>: {{name}}
+      <strong>Quantity:</strong>: {{quantity}}
+      <strong>Unit Price</strong>: ${{unitPrice}}
+    </template>
+  </amp-list>
+</body>
diff --git a/extensions/amp-bind/0.1/bind-validator.js b/extensions/amp-bind/0.1/bind-validator.js
@@ -233,6 +233,13 @@ function createElementRules_() {
         'alternativeName': 'src',
       },
     },
+    'AMP-LIST': {
+      'src': {
+        'allowedProtocols': {
+          'https': true,
+        },
+      },
+    },
     'AMP-SELECTOR': {
       'selected': null,
     },

diff --git a/extensions/amp-bind/0.1/test/test-bind-integration.js b/extensions/amp-bind/0.1/test/test-bind-integration.js
@@ -339,7 +339,7 @@ describe.configure().retryOnSaucelabs().run('amp-bind', function() {
   });
 
   describe('amp-video integration', () => {
-    it('should change src when the src attribute binding changes', () => {
+    it('should support binding to src', () => {
       const button = fixture.doc.getElementById('changeVidSrcButton');
       const vid = fixture.doc.getElementById('video');
       expect(vid.getAttribute('src')).to
@@ -449,4 +449,30 @@ describe.configure().retryOnSaucelabs().run('amp-bind', function() {
       });
     });
   });
+
+  describe('amp-list', () => {
+    it('should support binding to src', () => {
+      const button = fixture.doc.getElementById('listSrcButton');
+      const list = fixture.doc.getElementById('list');
+      expect(list.getAttribute('src'))
+          .to.equal('https://www.google.com/unbound.json');
+      button.click();
+      return waitForBindApplication().then(() => {
+        expect(list.getAttribute('src'))
+            .to.equal('https://www.google.com/bound.json');
+      });
+    });
+
+    it('should NOT change src when new value uses an invalid protocol', () => {
+      const button = fixture.doc.getElementById('httpListSrcButton');
+      const list = fixture.doc.getElementById('list');
+      expect(list.getAttribute('src'))
+          .to.equal('https://www.google.com/unbound.json');
+      button.click();
+      return waitForBindApplication().then(() => {
+        expect(list.getAttribute('src'))
+            .to.equal('https://www.google.com/unbound.json');
+      });
+    });
+  });
 });
diff --git a/extensions/amp-bind/amp-bind.md b/extensions/amp-bind/amp-bind.md
@@ -328,6 +328,11 @@ Only binding to the following components and attributes are allowed:
     <td><code>[alt]</code><br><code>[attribution]</code><br><code>[src]</code><br><code>[srcset]</code></td>
     <td>See corresponding <a href="https://www.ampproject.org/docs/reference/components/media/amp-img#attributes">amp-img attributes</a>.</td>
   </tr>
+  <tr>
+    <td><code>&lt;amp-list></code></td>
+    <td><code>[src]</code><sup>1</sup></td>
+    <td>Fetches JSON from the new URL and re-renders, replacing old content.</td>
+  </tr>
   <tr>
     <td><code>&lt;amp-selector></code></td>
     <td><code>[selected]</code><sup>1</sup></td>
@@ -399,8 +404,6 @@ Only binding to the following components and attributes are allowed:
     <td>See corresponding <a href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#Attributes">textarea attributes</a>.</td>
   </tr>
 </table>
-
-
 <sup>1</sup>Denotes bindable attributes that don't have a non-bindable counterpart.
 
 ## Debugging

diff --git a/extensions/amp-list/0.1/amp-list.js b/extensions/amp-list/0.1/amp-list.js
@@ -15,7 +15,9 @@
  */
 
 import {fetchBatchedJsonFor} from '../../../src/batched-json';
+import {isArray} from '../../../src/types';
 import {isLayoutSizeDefined} from '../../../src/layout';
+import {removeChildren} from '../../../src/dom';
 import {templatesFor} from '../../../src/services';
 import {user} from '../../../src/log';
 
@@ -36,9 +38,14 @@ export class AmpList extends AMP.BaseElement {
     this.container_ = this.win.document.createElement('div');
     this.applyFillContent(this.container_, true);
     this.element.appendChild(this.container_);
+
     if (!this.container_.hasAttribute('role')) {
       this.container_.setAttribute('role', 'list');
     }
+
+    if (!this.element.hasAttribute('aria-live')) {
+      this.element.setAttribute('aria-live', 'polite');
+    }
   }
 
   /** @override */
@@ -48,10 +55,31 @@ export class AmpList extends AMP.BaseElement {
 
   /** @override */
   layoutCallback() {
+    return this.populateList_();
+  }
+
+  /** @override */
+  mutatedAttributesCallback(mutations) {
+    if (mutations['src'] != undefined) {
+      this.populateList_();
+    }
+  }
+
+  /** @override */
+  getDynamicElementContainers() {
+    return [this.container_];
+  }
+
+  /**
+   * Request list data from `src` and return a promise that resolves when
+   * the list has been populated with rendered list items.
+   * @return {!Promise}
+   */
+  populateList_() {
     const itemsExpr = this.element.getAttribute('items') || 'items';
     return fetchBatchedJsonFor(
         this.getAmpDoc(), this.element, itemsExpr).then(items => {
-          user().assert(items && Array.isArray(items),
+          user().assert(isArray(items),
               'Response must contain an array at "%s". %s',
               itemsExpr, this.element);
           return templatesFor(this.win).findAndRenderTemplateArray(
@@ -66,6 +94,7 @@ export class AmpList extends AMP.BaseElement {
    * @private
    */
   rendered_(elements) {
+    removeChildren(this.container_);
     elements.forEach(element => {
       if (!element.hasAttribute('role')) {
         element.setAttribute('role', 'listitem');
@@ -82,6 +111,7 @@ export class AmpList extends AMP.BaseElement {
       }
     });
   }
+
 }
 
 AMP.registerElement('amp-list', AmpList);
diff --git a/extensions/amp-list/0.1/test/test-amp-list.js b/extensions/amp-list/0.1/test/test-amp-list.js
@@ -72,13 +72,13 @@ describe('amp-list component', () => {
     itemElement.style.height = newHeight + 'px';
     const xhrPromise = Promise.resolve({items});
     const renderPromise = Promise.resolve([itemElement]);
-    let measureFunc;
     xhrMock.expects('fetchJson').withExactArgs('https://data.com/list.json',
         sinon.match(opts => opts.credentials === undefined))
         .returns(xhrPromise).once();
     templatesMock.expects('findAndRenderTemplateArray').withExactArgs(
         element, items)
         .returns(renderPromise).once();
+    let measureFunc;
     listMock.expects('getVsync').returns({
       measure: func => {
         measureFunc = func;
@@ -87,11 +87,51 @@ describe('amp-list component', () => {
     listMock.expects('attemptChangeHeight').withExactArgs(newHeight).returns(
         Promise.resolve());
     return list.layoutCallback().then(() => {
-      return Promise.all([xhrPromise, renderPromise]).then(() => {
-        expect(list.container_.contains(itemElement)).to.be.true;
-        expect(measureFunc).to.exist;
-        measureFunc();
-      });
+      return Promise.all([xhrPromise, renderPromise]);
+    }).then(() => {
+      expect(list.container_.contains(itemElement)).to.be.true;
+      expect(measureFunc).to.exist;
+      measureFunc();
+    });
+  });
+
+  it('should reload data if the src attribute changes', () => {
+    const initialItems = [
+      {title: 'Title1'},
+    ];
+    const newItems = [
+      {title: 'Title2'}, {title: 'Title3'},
+    ];
+    const itemElement = document.createElement('div');
+    const itemElement2 = document.createElement('div');
+    const itemElement3 = document.createElement('div');
+    const xhrPromise = Promise.resolve({items: initialItems});
+    const renderPromise = Promise.resolve([itemElement]);
+    xhrMock.expects('fetchJson').withExactArgs('https://data.com/list.json',
+        sinon.match(opts => opts.credentials === undefined))
+        .returns(xhrPromise);
+    templatesMock.expects('findAndRenderTemplateArray').withExactArgs(
+        element, initialItems)
+        .returns(renderPromise);
+    listMock.expects('getVsync').returns({
+      measure: () => {},
+    }).twice();
+    return list.layoutCallback().then(() => {
+      return Promise.all([xhrPromise, renderPromise]);
+    }).then(() => {
+      expect(list.container_.contains(itemElement)).to.be.true;
+      const newXhrPromise = Promise.resolve({items: newItems});
+      const newRenderPromise = Promise.resolve([itemElement2, itemElement3]);
+      xhrMock.expects('fetchJson').withExactArgs('https://data2.com/list.json',
+          sinon.match(opts => opts.credentials === undefined))
+          .returns(newXhrPromise).once();
+      templatesMock.expects('findAndRenderTemplateArray').withExactArgs(
+          element, newItems)
+          .returns(newRenderPromise).once();
+      const spy = sandbox.spy(list, 'populateList_');
+      element.setAttribute('src', 'https://data2.com/list.json');
+      list.mutatedAttributesCallback({'src': 'https://data2.com/list.json'});
+      expect(spy).to.be.calledOnce;
     });
   });
 

diff --git a/extensions/amp-list/0.1/validator-amp-list.protoascii b/extensions/amp-list/0.1/validator-amp-list.protoascii
@@ -45,6 +45,8 @@ tags: {  # <amp-list>
   # TODO(gregable): Implement validation that requires the template attr value
   # to reference the id of an existing template element.
   attrs: { name: "template" }
+  # <amp-bind>
+  attrs: { name: "[src]" }
   attr_lists: "extended-amp-global"
   spec_url: "https://www.ampproject.org/docs/reference/components/dynamic/amp-list"
   amp_layout: {

diff --git a/test/fixtures/amp-bind-integrations.html b/test/fixtures/amp-bind-integrations.html
@@ -20,6 +20,7 @@
   <script custom-element="amp-brightcove" src="/dist/v0/amp-brightcove-0.1.max.js"></script>
   <script custom-element="amp-carousel" src="/dist/v0/amp-carousel-0.1.max.js"></script>
   <script custom-element="amp-iframe" src="/dist/v0/amp-iframe-0.1.max.js"></script>
+  <script custom-element="amp-list" src="/dist/v0/amp-list-0.1.max.js"></script>
   <script custom-element="amp-live-list" src="/dist/v0/amp-live-list-0.1.max.js"></script>
   <script custom-element="amp-selector" src="/dist/v0/amp-selector-0.1.max.js"></script>
   <script custom-element="amp-youtube" src="/dist/v0/amp-youtube-0.1.max.js"></script>
@@ -129,5 +130,11 @@
       src="https://player.vimeo.com/video/140261016" [src]="iframe">
   </amp-iframe>
 
+  <p>AMP-LIST</p>
+  <button id='listSrcButton' on="tap:AMP.setState({listSrc: 'https://www.google.com/bound.json'})">Change amp-list</button>
+  <button id='httpListSrcButton' on="tap:AMP.setState({listSrc: 'http://www.google.com/justhttp.json'})">Change amp-list</button>
+  <amp-list id='list' src="https://www.google.com/unbound.json" [src]=listSrc>
+  </amp-list>
+
 </body>
 </html>