Force resize when an overflown element becomes active

src/custom-element.js

@@ -996,6 +996,11 @@ export function createAmpElementProto(win, name, implementationClass) {
    * @package @final
    */
   ElementProto.overflowCallback = function(overflown, requestedHeight) {
+    if (!overflown && !this.overflowElement_) {
+      // Overflow has never been initialized and not wanted.
+      return;
+    }
+
     const overflowElement = this.getOverflowElement();
     if (!overflowElement) {
       if (overflown) {

src/focus-history.js

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import {Observable} from './observable';
 import {timer} from './timer';
 
 
@@ -36,6 +37,9 @@ export class FocusHistory {
     /** @private @const {!Array<!{el: !Element, time: time}>} */
     this.history_ = [];
 
+    /** @private @const {!Observable<!Element>} */
+    this.observeFocus_ = new Observable();
+
     /** @private @const {function(!Event)} */
     this.captureFocus_ = e => {
       if (e.target) {
@@ -61,6 +65,15 @@ export class FocusHistory {
     this.win.removeEventListener('blur', this.captureBlur_);
   }
 
+  /**
+   * Add a listener for focus events.
+   * @param {function(!Element)} handler
+   * @return {!Unlisten}
+   */
+  onFocus(handler) {
+    return this.observeFocus_.add(handler);
+  }
+
   /**
    * @param {!Element} element
    * @private
@@ -74,6 +87,7 @@ export class FocusHistory {
       this.history_[this.history_.length - 1].time = now;
     }
     this.purgeBefore(now - this.purgeTimeout_);
+    this.observeFocus_.fire(element);
   }
 
   /**

src/resources.js

@@ -17,12 +17,13 @@
 import {FocusHistory} from './focus-history';
 import {Pass} from './pass';
 import {assert} from './asserts';
+import {closest} from './dom';
+import {documentStateFor} from './document-state';
 import {expandLayoutRect, layoutRectLtwh, layoutRectsOverlap} from
     './layout-rect';
+import {getService} from './service';
 import {inputFor} from './input';
 import {log} from './log';
-import {documentStateFor} from './document-state';
-import {getService} from './service';
 import {makeBodyVisible} from './styles';
 import {reportError} from './error';
 import {timer} from './timer';
@@ -180,6 +181,10 @@ export class Resources {
       this.schedulePass(1);
     });
 
+    this.activeHistory_.onFocus(element => {
+      this.checkPendingChangeHeight_(element);
+    });
+
     this.relayoutAll_ = true;
     this.schedulePass();
   }
@@ -594,6 +599,8 @@ export class Resources {
             minTop = minTop == -1 ? box.top : Math.min(minTop, box.top);
           }
           request.resource./*OK*/changeHeight(request.newHeight);
+          request.resource.overflowCallback(/* overflown */ false,
+              request.newHeight);
         }
       }
 
@@ -630,6 +637,24 @@ export class Resources {
     }
   }
 
+  /**
+   * Reschedules change height request when an overflown element is activated.
+   * @param {!Element} element
+   * @private
+   */
+  checkPendingChangeHeight_(element) {
+    const resourceElement = closest(element, el => el[RESOURCE_PROP_]);
+    if (!resourceElement) {
+      return;
+    }
+    const resource = this.getResourceForElement(resourceElement);
+    const pendingChangeHeight = resource.getPendingChangeHeight();
+    if (pendingChangeHeight !== undefined) {
+      this.scheduleChangeHeight_(resource, pendingChangeHeight,
+          /* force */ true);
+    }
+  }
+
   /**
    * Discovers work that needs to be done since the last pass. If viewport
    * has changed, it will try to build new elements, measure changed elements,
@@ -936,6 +961,7 @@ export class Resources {
    * @private
    */
   scheduleChangeHeight_(resource, newHeight, force) {
+    resource.resetPendingChangeHeight();
     if (resource.getLayoutBox().height == newHeight) {
       // Nothing to do.
       return;
@@ -1177,6 +1203,12 @@ export class Resource {
      * @private {!Function|undefined}
      */
     this.onUpgraded_;
+
+    /**
+     * Pending change height that was requested but could not be satisfied.
+     * @private {number|undefined}
+     */
+    this.pendingChangeHeight_;
   }
 
   /**
@@ -1282,9 +1314,24 @@ export class Resource {
    * @param {number} requestedHeight
    */
   overflowCallback(overflown, requestedHeight) {
+    if (overflown) {
+      this.pendingChangeHeight_ = requestedHeight;
+    }
     this.element.overflowCallback(overflown, requestedHeight);
   }
 
+  /** @private */
+  resetPendingChangeHeight() {
+    this.pendingChangeHeight_ = undefined;
+  }
+
+  /**
+   * @return {number|undefined}
+   */
+  getPendingChangeHeight() {
+    return this.pendingChangeHeight_;
+  }
+
   /**
    * Measures the resource's boundaries. Only allowed for upgraded elements.
    */

test/functional/test-custom-element.js

@@ -1092,6 +1092,7 @@ describe('CustomElement Overflow Element', () => {
   });
 
   it('should unset overflow', () => {
+    element.getOverflowElement();
     overflowElement.classList.toggle('amp-visible', true);
     element.overflowCallback(false, 117);
     expect(element.overflowElement_).to.equal(overflowElement);

test/functional/test-resources.js

@@ -370,6 +370,7 @@ describe('Resources changeHeight', () => {
 
   function createResource(id, rect) {
     const resource = new Resource(id, createElement(rect), resources);
+    resource.element['__AMP__RESOURCE'] = resource;
     resource.state_ = ResourceState_.READY_FOR_LAYOUT;
     resource.layoutBox_ = rect;
     resource.changeHeight = sinon.spy();
@@ -460,7 +461,7 @@ describe('Resources changeHeight', () => {
       overflowCallbackSpy = sinon.spy();
       resource1.element.overflowCallback = overflowCallbackSpy;
       viewportMock.expects('getRect').returns(
-          {top: 0, left: 0, right: 100, bottom: 200, height: 200}).once();
+          {top: 0, left: 0, right: 100, bottom: 200, height: 200}).atLeast(1);
       resource1.layoutBox_ = {top: 10, left: 0, right: 100, bottom: 50,
           height: 50};
       vsyncSpy = sandbox.stub(resources.vsync_, 'run');
@@ -479,6 +480,7 @@ describe('Resources changeHeight', () => {
       expect(overflowCallbackSpy.callCount).to.equal(1);
       expect(overflowCallbackSpy.firstCall.args[0]).to.equal(true);
       expect(overflowCallbackSpy.firstCall.args[1]).to.equal(111);
+      expect(resource1.getPendingChangeHeight()).to.equal(111);
     });
 
     it('should change height when new height is lower', () => {
@@ -494,7 +496,8 @@ describe('Resources changeHeight', () => {
       resources.mutateWork_();
       expect(resources.requestsChangeHeight_.length).to.equal(0);
       expect(resource1.changeHeight.callCount).to.equal(1);
-      expect(overflowCallbackSpy.callCount).to.equal(0);
+      expect(overflowCallbackSpy.callCount).to.equal(1);
+      expect(overflowCallbackSpy.firstCall.args[0]).to.equal(false);
     });
 
     it('should change height when document is invisible', () => {
@@ -503,7 +506,8 @@ describe('Resources changeHeight', () => {
       resources.mutateWork_();
       expect(resources.requestsChangeHeight_.length).to.equal(0);
       expect(resource1.changeHeight.callCount).to.equal(1);
-      expect(overflowCallbackSpy.callCount).to.equal(0);
+      expect(overflowCallbackSpy.callCount).to.equal(1);
+      expect(overflowCallbackSpy.firstCall.args[0]).to.equal(false);
     });
 
     it('should change height when active', () => {
@@ -512,7 +516,8 @@ describe('Resources changeHeight', () => {
       resources.mutateWork_();
       expect(resources.requestsChangeHeight_.length).to.equal(0);
       expect(resource1.changeHeight.callCount).to.equal(1);
-      expect(overflowCallbackSpy.callCount).to.equal(0);
+      expect(overflowCallbackSpy.callCount).to.equal(1);
+      expect(overflowCallbackSpy.firstCall.args[0]).to.equal(false);
     });
 
     it('should change height when below the viewport', () => {
@@ -522,7 +527,8 @@ describe('Resources changeHeight', () => {
       resources.mutateWork_();
       expect(resources.requestsChangeHeight_.length).to.equal(0);
       expect(resource1.changeHeight.callCount).to.equal(1);
-      expect(overflowCallbackSpy.callCount).to.equal(0);
+      expect(overflowCallbackSpy.callCount).to.equal(1);
+      expect(overflowCallbackSpy.firstCall.args[0]).to.equal(false);
     });
 
     it('should change height when slightly above the viewport', () => {
@@ -532,7 +538,8 @@ describe('Resources changeHeight', () => {
       resources.mutateWork_();
       expect(resources.requestsChangeHeight_.length).to.equal(0);
       expect(resource1.changeHeight.callCount).to.equal(1);
-      expect(overflowCallbackSpy.callCount).to.equal(0);
+      expect(overflowCallbackSpy.callCount).to.equal(1);
+      expect(overflowCallbackSpy.firstCall.args[0]).to.equal(false);
     });
 
     it('should NOT change height when significantly above the viewport', () => {
@@ -544,6 +551,7 @@ describe('Resources changeHeight', () => {
       expect(overflowCallbackSpy.callCount).to.equal(1);
       expect(overflowCallbackSpy.firstCall.args[0]).to.equal(true);
       expect(overflowCallbackSpy.firstCall.args[1]).to.equal(111);
+      expect(resource1.getPendingChangeHeight()).to.equal(111);
     });
 
     it('should defer when above the viewport and scrolling on', () => {
@@ -611,6 +619,33 @@ describe('Resources changeHeight', () => {
       expect(resource1.changeHeight.firstCall.args[0]).to.equal(111);
       expect(resources.relayoutTop_).to.equal(resource1.layoutBox_.top);
     });
+
+    it('should reset pending change height when rescheduling', () => {
+      resources.scheduleChangeHeight_(resource1, 111, false);
+      resources.mutateWork_();
+      expect(resource1.getPendingChangeHeight()).to.equal(111);
+
+      resources.scheduleChangeHeight_(resource1, 112, false);
+      expect(resource1.getPendingChangeHeight()).to.be.undefined;
+    });
+
+    it('should force resize after focus', () => {
+      resources.scheduleChangeHeight_(resource1, 111, false);
+      resources.mutateWork_();
+      expect(resource1.getPendingChangeHeight()).to.equal(111);
+      expect(resources.requestsChangeHeight_.length).to.equal(0);
+
+      resources.checkPendingChangeHeight_(resource1.element);
+      expect(resource1.getPendingChangeHeight()).to.be.undefined;
+      expect(resources.requestsChangeHeight_.length).to.equal(1);
+
+      resources.mutateWork_();
+      expect(resources.requestsChangeHeight_.length).to.equal(0);
+      expect(resource1.changeHeight.callCount).to.equal(1);
+      expect(resource1.changeHeight.firstCall.args[0]).to.equal(111);
+      expect(overflowCallbackSpy.callCount).to.equal(2);
+      expect(overflowCallbackSpy.lastCall.args[0]).to.equal(false);
+    });
   });
 });