AMP List overflow fallback and relax some restrictions on change height

extensions/README.md

@@ -27,6 +27,7 @@ Current list of extended components:
 | [`amp-image-lightbox`](amp-image-lightbox/amp-image-lightbox.md) | Allows for a “image lightbox” or similar experience.                                        |
 | [`amp-instagram`](amp-instagram/amp-instagram.md)           | Displays an instagram embed.                                                                |
 | [`amp-lightbox`](amp-lightbox/amp-lightbox.md)             | Allows for a “lightbox” or similar experience.                                              |
+| [`amp-list`](amp-list/amp-list.md)             | A dynamic list that can download data and create list items using a template |
 | [`amp-twitter`](amp-twitter/amp-twitter.md)               | Displays a Twitter Tweet.                                                                   |
 | [`amp-youtube`](amp-youtube/amp-youtube.md)               | Displays a Youtube video.                                                                   |
 

extensions/amp-list/0.1/amp-list.js

@@ -16,13 +16,18 @@
 
 import {UrlReplacements} from '../../../src/url-replacements';
 import {assertHttpsUrl} from '../../../src/url';
+import {childElementByAttr} from '../../../src/dom';
 import {isLayoutSizeDefined} from '../../../src/layout';
+import {log} from '../../../src/log';
 import {templatesFor} from '../../../src/template';
 import {xhrFor} from '../../../src/xhr';
 
 /** @const {!Function} */
 const assert = AMP.assert;
 
+/** @const {string} */
+const TAG = 'AmpList';
+
 
 /**
  * The implementation of `amp-list` component. See {@link ../amp-list.md} for
@@ -47,6 +52,13 @@ export class AmpList extends AMP.BaseElement {
 
     /** @private @const {!UrlReplacements} */
     this.urlReplacements_ = new UrlReplacements(this.getWin());
+
+    /** @private {?Element} */
+    this.overflowElement_ = childElementByAttr(this.element, 'overflow');
+    if (this.overflowElement_) {
+      this.overflowElement_.classList.add('-amp-overflow');
+      this.overflowElement_.classList.toggle('amp-hidden', true);
+    }
   }
 
   /** @override */
@@ -83,8 +95,18 @@ export class AmpList extends AMP.BaseElement {
       const scrollHeight = this.container_./*OK*/scrollHeight;
       const height = this.element./*OK*/offsetHeight;
       if (scrollHeight > height) {
-        this.requestChangeHeight(scrollHeight, newHeight => {
-          // TODO(dvoytenko): Implement fallback.
+        this.requestChangeHeight(scrollHeight, actualHeight => {
+          if (this.overflowElement_) {
+            this.overflowElement_.classList.toggle('amp-hidden', false);
+            this.overflowElement_.onclick = () => {
+              this.overflowElement_.classList.toggle('amp-hidden', true);
+              this.changeHeight(actualHeight);
+            };
+          } else {
+            log.warn(TAG,
+                'Cannot resize element and overlfow is not available',
+                this.element);
+          }
         });
       }
     });

extensions/amp-list/0.1/test/test-amp-list.js

@@ -154,4 +154,77 @@ describe('amp-list component', () => {
         .returns(Promise.resolve([])).once();
     return list.layoutCallback();
   });
+
+  it('should fallback resize without overflow', () => {
+    const items = [{title: 'Title1'}];
+    const newHeight = 127;
+    const itemElement = document.createElement('div');
+    itemElement.style.height = newHeight + 'px';
+    const xhrPromise = Promise.resolve({items: items});
+    const renderPromise = Promise.resolve([itemElement]);
+    let measureFunc;
+    let resizeFallbackFunc;
+    xhrMock.expects('fetchJson').withExactArgs('https://data.com/list.json',
+        sinon.match(opts => !opts.credentials))
+        .returns(xhrPromise).once();
+    templatesMock.expects('findAndRenderTemplateArray').withExactArgs(
+        element, items)
+        .returns(renderPromise).once();
+    listMock.expects('getVsync').returns({
+      measure: func => {
+        measureFunc = func;
+      }
+    }).once();
+    listMock.expects('requestChangeHeight').withExactArgs(newHeight,
+        sinon.match(fallback => resizeFallbackFunc = fallback));
+    return list.layoutCallback().then(() => {
+      return promise.all([xhrPromise, renderPromise]).then(() => {
+        expect(list.container_.contains(itemElement)).to.be.true;
+        expect(measureFunc).to.exist;
+        measureFunc();
+        expect(resizeFallbackFunc).to.exist;
+        resizeFallbackFunc();
+      });
+    });
+  });
+
+  it('should fallback resize with overflow', () => {
+    const overflow = document.createElement('div');
+    overflow.setAttribute('overflow', '');
+    element.appendChild(overflow);
+    list.buildCallback();
+
+    const items = [{title: 'Title1'}];
+    const newHeight = 127;
+    const itemElement = document.createElement('div');
+    itemElement.style.height = newHeight + 'px';
+    const xhrPromise = Promise.resolve({items: items});
+    const renderPromise = Promise.resolve([itemElement]);
+    let measureFunc;
+    let resizeFallbackFunc;
+    xhrMock.expects('fetchJson').withExactArgs('https://data.com/list.json',
+        sinon.match(opts => !opts.credentials))
+        .returns(xhrPromise).once();
+    templatesMock.expects('findAndRenderTemplateArray').withExactArgs(
+        element, items)
+        .returns(renderPromise).once();
+    listMock.expects('getVsync').returns({
+      measure: func => {
+        measureFunc = func;
+      }
+    }).once();
+    listMock.expects('requestChangeHeight').withExactArgs(newHeight,
+        sinon.match(fallback => resizeFallbackFunc = fallback));
+    return list.layoutCallback().then(() => {
+      return promise.all([xhrPromise, renderPromise]).then(() => {
+        expect(list.container_.contains(itemElement)).to.be.true;
+        expect(measureFunc).to.exist;
+        measureFunc();
+        expect(resizeFallbackFunc).to.exist;
+        expect(overflow).to.have.class('amp-hidden');
+        resizeFallbackFunc();
+        expect(overflow).to.not.have.class('amp-hidden');
+      });
+    });
+  });
 });

extensions/amp-list/amp-list.md

@@ -28,11 +28,6 @@ The `amp-list` defines data source using the following attributes:
 [Fetch API](https://fetch.spec.whatwg.org/). To send credentials, pass the
 value of "include".
 
-The request URL is allowed to contain the following substitutions:
-
-- CANONICAL_URL
-- TODO
-
 The response must be a JSON object that contains an array property "items":
 ```json
 {
@@ -47,6 +42,9 @@ The template can be specified using either of the following two ways:
 
 For more details on templates see [AMP HTML Templates](../../spec/amp-html-templates.md).
 
+Optionally, `amp-list` element can contain an element with `overflow` attribute. This
+element will be shown if AMP Runtime cannot resize the `amp-list` element as requested.
+
 An example:
 ```html
 <amp-list src="https://data.com/articles.json?ref=CANONICAL_URL">
@@ -56,9 +54,22 @@ An example:
       {{title}}
     </div>
   </template>
+  <div overflow role=button aria-lable="Show more" class="list-overflow">
+    Show more
+  </div>
 </amp-list>
 ```
 
+```css
+.list-overflow {
+  position: absolute;
+  bottom: 0;
+}
+.list-overflow.amp-hidden {
+  visibility: hidden;
+}
+```
+
 #### Substitutions
 
 The `amp-list` allows all standard URL variable substitutions.
@@ -76,7 +87,10 @@ The loading is triggered using normal AMP rules depending on how far the element
 the current viewport.
 
 If `amp-list` needs more space after loading it requests the AMP runtime to update its
-height using the normal AMP flow.
+height using the normal AMP flow. If AMP Runtime cannot satisfy the request for new
+height, it will display `overflow` element when available. Notice however, the typical
+placement of `amp-list` elements at the bottom of the document almost always guarantees
+that AMP Runtime can resize it.
 
 By default, `amp-list` adds `list` ARIA role to the list element and `listitem` role to item
 elements rendered via the template.

src/resources.js

@@ -517,6 +517,7 @@ export class Resources {
         now - this.lastScrollTime_ > MUTATE_DEFER_DELAY_ ||
         now - this.lastScrollTime_ > MUTATE_DEFER_DELAY_ * 2);
     const offset = 10;
+    const bottomOffset = viewportRect.height / 4;
 
     if (this.deferredMutates_.length > 0) {
       log.fine(TAG_, 'deferred mutates:', this.deferredMutates_.length);
@@ -554,7 +555,7 @@ export class Resources {
           // 2. Active elements are immediately resized. The assumption is that
           // the resize is triggered by the user action or soon after.
           resize = true;
-        } else if (box.bottom >= viewportRect.bottom - offset) {
+        } else if (box.bottom >= viewportRect.bottom - bottomOffset) {
           // 3. Elements under viewport are resized immediately.
           resize = true;
         } else if (box.bottom <= viewportRect.top + offset) {

test/functional/test-resources.js

@@ -459,7 +459,7 @@ describe('Resources changeHeight', () => {
   describe('requestChangeHeight rules when element is in viewport', () => {
     beforeEach(() => {
       viewportMock.expects('getRect').returns(
-          {top: 0, left: 0, right: 100, bottom: 200}).once();
+          {top: 0, left: 0, right: 100, bottom: 200, height: 200}).once();
       resource1.layoutBox_ = {top: 10, left: 0, right: 100, bottom: 50,
           height: 50};
     });
@@ -520,6 +520,23 @@ describe('Resources changeHeight', () => {
       expect(resource1.changeHeight.callCount).to.equal(1);
     });
 
+    it('should change height when slightly above the viewport', () => {
+      resource1.layoutBox_ = {top: 10, left: 0, right: 100, bottom: 180,
+          height: 50};
+      resources.scheduleChangeHeight_(resource1, 111, false, null);
+      resources.mutateWork_();
+      expect(resources.changeHeightRequests_.length).to.equal(0);
+      expect(resource1.changeHeight.callCount).to.equal(1);
+    });
+
+    it('should NOT change height when significantly above the viewport', () => {
+      resource1.layoutBox_ = {top: 10, left: 0, right: 100, bottom: 100,
+          height: 50};
+      resources.scheduleChangeHeight_(resource1, 111, false, null);
+      resources.mutateWork_();
+      expect(resource1.changeHeight.callCount).to.equal(0);
+    });
+
     it('should defer when above the viewport and scrolling on', () => {
       resource1.layoutBox_ = {top: -1200, left: 0, right: 100, bottom: -1050,
           height: 50};

test/manual/amp-list.amp.html

@@ -14,16 +14,32 @@
       display: block;
       margin: 8px;
     }
-    figure {
-      margin: 0;
-    }
+
     amp-list {
       border: 1px solid green;
     }
+    amp-list [overflow] {
+      background: gray;
+      color: #fff;
+      cursor: pointer;
+      position: absolute;
+      bottom: 0;
+      left: 0;
+      right: 0;
+      z-index: 2;
+      padding: 16px;
+    }
+    amp-list [overflow].amp-hidden {
+      visibility: hidden;
+    }
     .story-entry {
       padding: 40px;
       border-bottom: 1px solid green;
     }
+
+    .split {
+      height: 300px;
+    }
   </style>
   <style>body {opacity: 0}</style><noscript><style>body {opacity: 1}</style></noscript>
   <script async src="../../dist/amp.js" development></script>
@@ -34,14 +50,32 @@
 <article>
   <h1>amp-list</h1>
 
-  <amp-list width="396" height="200" layout=responsive
+  <amp-list width="396" height="80" layout=responsive
+      src="http://localhost:8000/test/manual/amp-list-data.json?RANDOM">
+    <template type="amp-mustache">
+      <div class="story-entry">
+        <amp-img src="{{imageUrl}}" width=80 height=60></amp-img>
+        {{title}}
+      </div>
+    </template>
+    <div overflow>
+      SEE MORE
+    </div>
+  </amp-list>
+
+  <div class="split"></div>
+
+  <amp-list width="396" height="80" layout=responsive
       src="http://localhost:8000/test/manual/amp-list-data.json?RANDOM">
     <template type="amp-mustache">
       <div class="story-entry">
         <amp-img src="{{imageUrl}}" width=80 height=60></amp-img>
         {{title}}
       </div>
     </template>
+    <div overflow>
+      SEE MORE
+    </div>
   </amp-list>
 </article>
 </body>