diff --git a/src/custom-element.js b/src/custom-element.js index b72151804924..85d881bc5a02 100644 --- a/src/custom-element.js +++ b/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) { diff --git a/src/focus-history.js b/src/focus-history.js index f4ba7ad66881..fa97160034a0 100644 --- a/src/focus-history.js +++ b/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} */ this.history_ = []; + /** @private @const {!Observable} */ + 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); } /** diff --git a/src/resources.js b/src/resources.js index 9cab1ca49be8..95da29e3763f 100644 --- a/src/resources.js +++ b/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. */ diff --git a/test/functional/test-custom-element.js b/test/functional/test-custom-element.js index 7a8cfda783fb..1cb01984176b 100644 --- a/test/functional/test-custom-element.js +++ b/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); diff --git a/test/functional/test-resources.js b/test/functional/test-resources.js index ffd457ebbb92..2cbae661d1e8 100644 --- a/test/functional/test-resources.js +++ b/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); + }); }); });