Redo the overflow toggling system

build-system/tasks/presubmit-checks.js

@@ -186,6 +186,7 @@ var forbiddenTermsSrcInclusive = {
   '\\.getBBox(?!_)': bannedTermsHelpString,
   '\\.webkitConvertPointFromNodeToPage(?!_)': bannedTermsHelpString,
   '\\.webkitConvertPointFromPageToNode(?!_)': bannedTermsHelpString,
+  '\\.changeHeight(?!_)': bannedTermsHelpString,
 };
 
 // Terms that must appear in a source file.

css/amp.css

@@ -267,16 +267,18 @@ i-amp-scroll-container {
 
 
 /**
- * `-amp-overflow` is a support for "overflow" element. This is
- * an element shown when more content is available. Typically tapping on this
- * element shows the full content.
+ * "overflow" element is an element shown when more content is available but
+ * not currently visible. Typically tapping on this element shows the full
+ * content.
  */
-.-amp-overflow {
-  z-index: 1;
+.-amp-element > [overflow] {
+  cursor: pointer;
+  z-index: 2;
+  visibility: hidden;
 }
 
-.-amp-overflow.amp-hidden {
-  visibility: hidden;
+.-amp-element > [overflow].amp-visible {
+  visibility: visible;
 }
 
 

extensions/amp-iframe/0.1/amp-iframe.js

@@ -106,12 +106,6 @@ class AmpIframe extends AMP.BaseElement {
 
   /** @override */
   buildCallback() {
-    /** @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 */
@@ -142,7 +136,7 @@ class AmpIframe extends AMP.BaseElement {
     this.isResizable_ = this.element.hasAttribute('resizable');
     if (this.isResizable_) {
       this.element.setAttribute('scrolling', 'no');
-      assert(this.overflowElement_,
+      assert(this.getOverflowElement(),
           'Overflow element must be defined for resizable frames: %s',
           this.element);
     }
@@ -183,14 +177,7 @@ class AmpIframe extends AMP.BaseElement {
           this.element);
       return;
     }
-    this.requestChangeHeight(newHeight, actualHeight => {
-      assert(this.overflowElement_);
-      this.overflowElement_.classList.toggle('amp-hidden', false);
-      this.overflowElement_.onclick = () => {
-        this.overflowElement_.classList.toggle('amp-hidden', true);
-        this.changeHeight(actualHeight);
-      };
-    });
+    this.requestChangeHeight(newHeight);
   }
 };
 

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

@@ -300,16 +300,6 @@ describe('amp-iframe', () => {
       expect(impl.changeHeight.callCount).to.equal(0);
       expect(impl.requestChangeHeight.callCount).to.equal(1);
       expect(impl.requestChangeHeight.firstCall.args[0]).to.equal(217);
-
-      const fallback = impl.requestChangeHeight.firstCall.args[1];
-      fallback(219);
-      expect(impl.overflowElement_).to.not.be.null;
-      expect(impl.overflowElement_).to.have.class('-amp-overflow');
-      expect(impl.overflowElement_).to.not.have.class('amp-hidden');
-      impl.overflowElement_.onclick();
-      expect(impl.overflowElement_).to.have.class('amp-hidden');
-      expect(impl.changeHeight.callCount).to.equal(1);
-      expect(impl.changeHeight.firstCall.args[0]).to.equal(219);
     });
   });
 

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

@@ -52,13 +52,6 @@ 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 */
@@ -95,19 +88,7 @@ export class AmpList extends AMP.BaseElement {
       const scrollHeight = this.container_./*OK*/scrollHeight;
       const height = this.element./*OK*/offsetHeight;
       if (scrollHeight > height) {
-        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);
-          }
-        });
+        this.requestChangeHeight(scrollHeight);
       }
     });
   }

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

@@ -87,8 +87,7 @@ describe('amp-list component', () => {
         measureFunc = func;
       }
     }).once();
-    listMock.expects('requestChangeHeight').withExactArgs(newHeight,
-        sinon.match.func);
+    listMock.expects('requestChangeHeight').withExactArgs(newHeight);
     return list.layoutCallback().then(() => {
       return promise.all([xhrPromise, renderPromise]).then(() => {
         expect(list.container_.contains(itemElement)).to.be.true;
@@ -154,77 +153,4 @@ 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

@@ -65,9 +65,6 @@ An example:
   position: absolute;
   bottom: 0;
 }
-.list-overflow.amp-hidden {
-  visibility: hidden;
-}
 ```
 
 #### Substitutions

extensions/amp-twitter/0.1/amp-twitter.js

@@ -49,7 +49,7 @@ class AmpTwitter extends AMP.BaseElement {
       const amp = iframe.parentElement;
       amp.setAttribute('height', data.height);
       amp.setAttribute('width', data.width);
-      this.changeHeight(data.height);
+      this./*OK*/changeHeight(data.height);
     });
     return loadPromise(iframe);
   }

src/base-element.js

@@ -419,6 +419,15 @@ export class BaseElement {
     this.element.toggleFallback(state);
   }
 
+  /**
+   * Returns an optional overflow element for this custom element.
+   * @return {?Element}
+   * @protected @final
+   */
+  getOverflowElement() {
+    return this.element.getOverflowElement();
+  }
+
   /**
    * Returns the original nodes of the custom element without any service nodes
    * that could have been added for markup. These nodes can include Text,
@@ -511,22 +520,21 @@ export class BaseElement {
    * @protected
    */
   changeHeight(newHeight) {
-    this.resources_.changeHeight(this.element, newHeight);
+    this.resources_./*OK*/changeHeight(this.element, newHeight);
   }
 
   /**
    * Requests the runtime to update the height of this element to the specified
    * value. The runtime will schedule this request and attempt to process it
    * as soon as possible. However, unlike in {@link changeHeight}, the runtime
-   * may refuse to make a change in which case it will call the provided
-   * fallback with the height value. The fallback is expected to provide the
-   * reader with the user action to update the height manually.
+   * may refuse to make a change in which case it will show the element's
+   * overflow element if provided, which is supposed to provide the reader with
+   * the necessary user action.
    * @param {number} newHeight
-   * @param {function(number)} fallback
    * @protected
    */
-  requestChangeHeight(newHeight, fallback) {
-    this.resources_.requestChangeHeight(this.element, newHeight, fallback);
+  requestChangeHeight(newHeight) {
+    this.resources_.requestChangeHeight(this.element, newHeight);
   }
 
   /**

src/custom-element.js

@@ -222,7 +222,8 @@ function isInternalOrServiceNode(node) {
     return true;
   }
   if (node.tagName && (node.hasAttribute('placeholder') ||
-          node.hasAttribute('fallback'))) {
+          node.hasAttribute('fallback') ||
+          node.hasAttribute('overflow'))) {
     return true;
   }
   return false;
@@ -310,6 +311,9 @@ export function createAmpElementProto(win, name, implementationClass) {
     /** @private {?Element} */
     this.loadingElement_ = null;
 
+    /** @private {?Element|undefined} */
+    this.overflowElement_;
+
     /** @private {!BaseElement} */
     this.implementation_ = new implementationClass(this);
     this.implementation_.createdCallback();
@@ -519,12 +523,13 @@ export function createAmpElementProto(win, name, implementationClass) {
    * Changes the height of the element.
    *
    * This method is called by Resources and shouldn't be called by anyone else.
+   * This method must always be called in the mutation context.
    *
    * @param {number} newHeight
    * @final
    * @package
    */
-  ElementProto.changeHeight = function(newHeight) {
+  ElementProto./*OK*/changeHeight = function(newHeight) {
     if (this.sizerElement_) {
       // From the moment height is changed the element becomes fully
       // responsible for managing its height. Aspect ratio is no longer
@@ -963,6 +968,57 @@ export function createAmpElementProto(win, name, implementationClass) {
     });
   };
 
+  /**
+   * Returns an optional overflow element for this custom element.
+   * @return {?Element}
+   * @private
+   */
+  ElementProto.getOverflowElement = function() {
+    if (this.overflowElement_ === undefined) {
+      this.overflowElement_ = dom.childElementByAttr(this, 'overflow');
+      if (this.overflowElement_) {
+        if (!this.overflowElement_.hasAttribute('tabindex')) {
+          this.overflowElement_.setAttribute('tabindex', '0');
+        }
+        if (!this.overflowElement_.hasAttribute('role')) {
+          this.overflowElement_.setAttribute('role', 'button');
+        }
+      }
+    }
+    return this.overflowElement_;
+  };
+
+  /**
+   * Hides or shows the overflow, if available. This function must only
+   * be called inside a mutate context.
+   * @param {boolean} overflown
+   * @param {number} requestedHeight
+   * @package @final
+   */
+  ElementProto.overflowCallback = function(overflown, requestedHeight) {
+    const overflowElement = this.getOverflowElement();
+    if (!overflowElement) {
+      if (overflown) {
+        log.warn(TAG_,
+            'Cannot resize element and overlfow is not available', this);
+      }
+      return;
+    }
+
+    overflowElement.classList.toggle('amp-visible', overflown);
+
+    if (overflown) {
+      this.overflowElement_.onclick = () => {
+        this.resources_./*OK*/changeHeight(this, requestedHeight);
+        this.getVsync_().mutate(() => {
+          this.overflowCallback(/* overflown */ false, requestedHeight);
+        });
+      };
+    } else {
+      this.overflowElement_.onclick = null;
+    }
+  };
+
   return ElementProto;
 }