diff --git a/extensions/amp-sidebar/0.1/amp-sidebar.js b/extensions/amp-sidebar/0.1/amp-sidebar.js index 37717ec11ffb..8ae68c57cd49 100644 --- a/extensions/amp-sidebar/0.1/amp-sidebar.js +++ b/extensions/amp-sidebar/0.1/amp-sidebar.js @@ -82,6 +82,8 @@ export class AmpSidebar extends AMP.BaseElement { this.viewport_ = this.getViewport(); + this.viewport_.addToFixedLayer(this.element, /* forceTransfer */true); + if (this.side_ != 'left' && this.side_ != 'right') { const pageDir = this.document_.body.getAttribute('dir') || @@ -176,7 +178,6 @@ export class AmpSidebar extends AMP.BaseElement { this.viewport_.disableTouchZoom(); this.vsync_.mutate(() => { toggle(this.element, /* display */true); - this.viewport_.addToFixedLayer(this.element); this.openMask_(); if (this.isIosSafari_) { this.compensateIosBottombar_(); @@ -221,7 +222,6 @@ export class AmpSidebar extends AMP.BaseElement { } this.openOrCloseTimeOut_ = this.timer_.delay(() => { if (!this.isOpen_()) { - this.viewport_.removeFromFixedLayer(this.element); this.vsync_.mutate(() => { toggle(this.element, /* display */false); this.schedulePause(this.getRealChildren()); diff --git a/src/service/fixed-layer.js b/src/service/fixed-layer.js index 2f1620eec558..dbf37e4776df 100644 --- a/src/service/fixed-layer.js +++ b/src/service/fixed-layer.js @@ -171,9 +171,11 @@ export class FixedLayer { /** * Adds the element directly into the fixed layer, bypassing discovery. * @param {!Element} element + * @param {boolean=} opt_forceTransfer If set to true , then the element needs + * to be forcefully transferred to the fixed layer. */ - addElement(element) { - this.setupFixedElement_(element, /* selector */ '*'); + addElement(element, opt_forceTransfer) { + this.setupFixedElement_(element, /* selector */ '*', opt_forceTransfer); this.sortInDomOrder_(); this.update(); } @@ -205,7 +207,7 @@ export class FixedLayer { * Performs fixed actions. * 1. Updates `top` styling if necessary. * 2. On iOS/Iframe moves elements between fixed layer and BODY depending on - * whether they are currently visible and fixed. + * whether they are currently visible and fixed * @return {!Promise} */ update() { @@ -268,9 +270,13 @@ export class FixedLayer { // Element is indeed fixed. Visibility is added to the test to // avoid moving around invisible elements. const isFixed = ( - position == 'fixed' && - element./*OK*/offsetWidth > 0 && - element./*OK*/offsetHeight > 0); + position == 'fixed' && ( + fe.forceTransfer || ( + element./*OK*/offsetWidth > 0 && + element./*OK*/offsetHeight > 0 + ) + ) + ); if (!isFixed) { state[fe.id] = { fixed: false, @@ -307,11 +313,11 @@ export class FixedLayer { // `height` is constrained to at most 300px. This is to avoid // transfering of more substantial sections for now. Likely to be // relaxed in the future. - const isTransferrable = ( - isFixed && - opacity > 0 && - element./*OK*/offsetHeight < 300 && - (this.isAllowedCoord_(top) || this.isAllowedCoord_(bottom))); + const isTransferrable = isFixed && ( + fe.forceTransfer || ( + opacity > 0 && + element./*OK*/offsetHeight < 300 && + (this.isAllowedCoord_(top) || this.isAllowedCoord_(bottom)))); if (isTransferrable) { hasTransferables = true; } @@ -399,9 +405,11 @@ export class FixedLayer { * * @param {!Element} element * @param {string} selector + * @param {boolean=} opt_forceTransfer If set to true , then the element needs + * to be forcefully transferred to the fixed layer. * @private */ - setupFixedElement_(element, selector) { + setupFixedElement_(element, selector, opt_forceTransfer) { let fe = null; for (let i = 0; i < this.fixedElements_.length; i++) { if (this.fixedElements_[i].element == element) { @@ -424,6 +432,8 @@ export class FixedLayer { }; this.fixedElements_.push(fe); } + + fe.forceTransfer = !!opt_forceTransfer; } /** @@ -668,6 +678,7 @@ export class FixedLayer { * fixedNow: (boolean|undefined), * top: (string|undefined), * transform: (string|undefined), + * forceTransfer: (boolean|undefined), * }} */ let FixedElementDef; diff --git a/src/service/viewport-impl.js b/src/service/viewport-impl.js index 7329c6b751e8..8050da8066b9 100644 --- a/src/service/viewport-impl.js +++ b/src/service/viewport-impl.js @@ -533,9 +533,11 @@ export class Viewport { /** * Adds the element to the fixed layer. * @param {!Element} element + * @param {boolean=} opt_forceTransfer If set to true , then the element needs + * to be forcefully transferred to the fixed layer. */ - addToFixedLayer(element) { - this.fixedLayer_.addElement(element); + addToFixedLayer(element, opt_forceTransfer) { + this.fixedLayer_.addElement(element, opt_forceTransfer); } /** diff --git a/test/functional/test-fixed-layer.js b/test/functional/test-fixed-layer.js index 81c718d58671..23ce0d04674d 100644 --- a/test/functional/test-fixed-layer.js +++ b/test/functional/test-fixed-layer.js @@ -322,6 +322,20 @@ describe('FixedLayer', () => { // Remove. fixedLayer.removeElement(element3); expect(fixedLayer.fixedElements_).to.have.length(2); + + //Add with forceTransfer + fixedLayer.addElement(element3, '*', true); + expect(updateStub.callCount).to.equal(2); + expect(fixedLayer.fixedElements_).to.have.length(3); + const fe1 = fixedLayer.fixedElements_[2]; + expect(fe1.id).to.equal('F3'); + expect(fe1.element).to.equal(element3); + expect(fe1.selectors).to.deep.equal(['*']); + expect(fe1.forceTransfer).to.be.true; + + // Remove. + fixedLayer.removeElement(element3); + expect(fixedLayer.fixedElements_).to.have.length(2); }); it('should remove node when disappeared from DOM', () => { @@ -680,6 +694,27 @@ describe('FixedLayer', () => { expect(state['F0'].transferrable).to.equal(true); }); + it('should not disregard invisible element if it has forceTransfer', () => { + element1.computedStyle['position'] = 'fixed'; + element1.offsetWidth = 0; + element1.offsetHeight = 0; + + expect(vsyncTasks).to.have.length(1); + let state = {}; + vsyncTasks[0].measure(state); + expect(state['F0'].fixed).to.be.false; + expect(state['F0'].transferrable).to.equal(false); + + // Add. + state = {}; + fixedLayer.setupFixedElement_(element1, '*', true); + expect(vsyncTasks).to.have.length(1); + vsyncTasks[0].measure(state); + + expect(state['F0'].fixed).to.be.true; + expect(state['F0'].transferrable).to.equal(true); + }); + it('should collect turn off transferrable with bottom != 0', () => { element1.computedStyle['position'] = 'fixed'; element1.offsetWidth = 10;