From d67d81cbffb25fc45fe92add21aca287e0bc1a8c Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Tue, 16 Feb 2021 10:06:57 -0800
Subject: [PATCH 1/9] Display observer in nested fixed overlays
---
examples/amp-lightbox.amp.html | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/examples/amp-lightbox.amp.html b/examples/amp-lightbox.amp.html
index 7e1c8628e567..13b0031dd421 100644
--- a/examples/amp-lightbox.amp.html
+++ b/examples/amp-lightbox.amp.html
@@ -39,6 +39,7 @@
+
@@ -76,6 +77,17 @@ Scrollable Lightbox
Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit.
Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit.
+
+
+
+
From 8579bc24bf94fe0718f1e896d5fe38a2cb4013d3 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Tue, 16 Feb 2021 17:25:15 -0800
Subject: [PATCH 2/9] Support display observer in the lightbox
---
extensions/amp-lightbox/0.1/amp-lightbox.js | 6 +
src/utils/display-observer.js | 221 +++++++++++++++++---
test/unit/utils/test-display-observer.js | 164 ++++++++++++++-
3 files changed, 356 insertions(+), 35 deletions(-)
diff --git a/extensions/amp-lightbox/0.1/amp-lightbox.js b/extensions/amp-lightbox/0.1/amp-lightbox.js
index 3430b1930b97..a2d96f2eab18 100644
--- a/extensions/amp-lightbox/0.1/amp-lightbox.js
+++ b/extensions/amp-lightbox/0.1/amp-lightbox.js
@@ -39,6 +39,10 @@ import {dict, hasOwn} from '../../../src/utils/object';
import {getMode} from '../../../src/mode';
import {htmlFor} from '../../../src/static-template';
import {isInFie} from '../../../src/iframe-helper';
+import {
+ registerContainer,
+ unregisterContainer,
+} from '../../../src/utils/display-observer';
import {toArray} from '../../../src/types';
import {tryFocus} from '../../../src/dom';
@@ -283,6 +287,7 @@ class AmpLightbox extends AMP.BaseElement {
return;
}
this.initialize_();
+ registerContainer(this.element, this.container_);
this.boundCloseOnEscape_ = /** @type {?function(this:AmpLightbox, Event)} */ (this.closeOnEscape_.bind(
this
));
@@ -557,6 +562,7 @@ class AmpLightbox extends AMP.BaseElement {
if (!this.active_) {
return;
}
+ unregisterContainer(this.element, this.container_);
if (this.isScrollable_) {
setStyle(this.element, 'webkitOverflowScrolling', '');
}
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index d33751697654..e6b76545bba7 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -23,6 +23,13 @@ const SERVICE_ID = 'DisplayObserver';
const DISPLAY_THRESHOLD = 0.51;
+const CONTAINER_OFFSET = 2;
+
+/**
+ * @typedef {function(boolean, !Element)}
+ */
+let ObserverCallbackDef;
+
/**
* Observes whether the specified target is displayable. The initial observation
* is returned shortly after observing, and subsequent observations are
@@ -38,7 +45,7 @@ const DISPLAY_THRESHOLD = 0.51;
* not in the viewport, it's considered to be "displayable".
*
* @param {!Element} target
- * @param {function(boolean)} callback
+ * @param {!ObserverCallbackDef} callback
*/
export function observeDisplay(target, callback) {
getObserver(target).observe(target, callback);
@@ -67,6 +74,22 @@ export function measureDisplay(target) {
});
}
+/**
+ * QQQQ
+ * @param {!Element} container
+ * @param {!Element} root
+ */
+export function registerContainer(container, root) {
+ getObserver(container).registerContainer(container, root);
+}
+
+/**
+ * @param {!Element} container
+ */
+export function unregisterContainer(container) {
+ getObserver(container).unregisterContainer(container);
+}
+
/**
* @implements {Disposable}
* @visibleForTesting
@@ -77,6 +100,9 @@ export class DisplayObserver {
* @param {!AmpDoc} ampdoc
*/
constructor(ampdoc) {
+ /** @private @const */
+ this.ampdoc_ = ampdoc;
+
const {win} = ampdoc;
/** @private @const {!Array} */
@@ -108,11 +134,19 @@ export class DisplayObserver {
}
});
- /** @private @const {!Map>} */
+ /** @private @const {!Map>} */
this.targetObserverCallbacks_ = new Map();
/** @private @const {!Map>} */
this.targetObservations_ = new Map();
+
+ /** @private @const {!Array} */
+ this.containers_ = [];
+
+ /** @private @const {!Map} */
+ this.containerRoots_ = new Map();
+
+ this.containerObserved_ = this.containerObserved_.bind(this);
}
/** @override */
@@ -124,6 +158,58 @@ export class DisplayObserver {
this.targetObservations_.clear();
}
+ /**
+ * @param {!Element} container
+ * @param {!Element} root
+ */
+ registerContainer(container, root) {
+ if (this.containers_.includes(container)) {
+ return;
+ }
+
+ this.containers_.push(container);
+ this.containerRoots_.set(container, root);
+ // QQQ: externalize callback
+ this.observe(container, this.containerObserved_);
+ }
+
+ /**
+ * @param {!Element} container
+ */
+ unregisterContainer(container) {
+ const index = CONTAINER_OFFSET + this.containers_.indexOf(container);
+ if (index < CONTAINER_OFFSET) {
+ // The container has been unregistered already.
+ return;
+ }
+
+ // Remove container.
+ this.containers_.splice(index - CONTAINER_OFFSET, 1);
+ this.containerRoots_.delete(container);
+
+ // Remove observer.
+ const observer = this.observers_[index];
+ this.observers_.splice(index, 1);
+ if (observer) {
+ observer.disconnect();
+ }
+
+ // Unobserve the container itself.
+ this.unobserve(container, this.containerObserved_);
+
+ // Remove observations.
+ this.targetObserverCallbacks_.forEach((callbacks, target) => {
+ const observations = this.targetObservations_.get(target);
+ if (!observations || observations.length <= index) {
+ return;
+ }
+ const oldDisplay = computeDisplay(observations, this.isDocDisplay_);
+ observations.splice(index, 1);
+ const newDisplay = computeDisplay(observations, this.isDocDisplay_);
+ notifyIfChanged(callbacks, target, newDisplay, oldDisplay);
+ });
+ }
+
/**
* @param {!Element} target
* @param {function(boolean)} callback
@@ -133,8 +219,19 @@ export class DisplayObserver {
if (!callbacks) {
callbacks = [];
this.targetObserverCallbacks_.set(target, callbacks);
+
+ // Subscribe observers.
for (let i = 0; i < this.observers_.length; i++) {
- this.observers_[i].observe(target);
+ if (
+ this.observers_[i] &&
+ (i < CONTAINER_OFFSET ||
+ containsNotSelf(this.containers_[i - CONTAINER_OFFSET], target))
+ ) {
+ this.setObservation_(target, i, null, /* callbacks */ null);
+ this.observers_[i].observe(target);
+ } else {
+ this.setObservation_(target, i, false, /* callbacks */ null);
+ }
}
}
if (pushIfNotExist(callbacks, callback)) {
@@ -146,7 +243,7 @@ export class DisplayObserver {
this.isDocDisplay_
);
if (display != null) {
- callCallbackNoInline(callback, display);
+ callCallbackNoInline(callback, target, display);
}
});
}
@@ -167,7 +264,9 @@ export class DisplayObserver {
this.targetObserverCallbacks_.delete(target);
this.targetObservations_.delete(target);
for (let i = 0; i < this.observers_.length; i++) {
- this.observers_[i].unobserve(target);
+ if (this.observers_[i]) {
+ this.observers_[i].unobserve(target);
+ }
}
}
}
@@ -178,7 +277,46 @@ export class DisplayObserver {
const observations = this.targetObservations_.get(target);
const oldDisplay = computeDisplay(observations, !this.isDocDisplay_);
const newDisplay = computeDisplay(observations, this.isDocDisplay_);
- notifyIfChanged(callbacks, newDisplay, oldDisplay);
+ notifyIfChanged(callbacks, target, newDisplay, oldDisplay);
+ });
+ }
+
+ /**
+ * @param {boolean} isDisplayed
+ * @param {!Element} container
+ * @private
+ */
+ containerObserved_(isDisplayed, container) {
+ const index = CONTAINER_OFFSET + this.containers_.indexOf(container);
+ if (index < CONTAINER_OFFSET) {
+ // The container has been unregistered already.
+ return;
+ }
+
+ if (isDisplayed) {
+ const {win} = this.ampdoc_;
+ const root = this.containerRoots_.get(container) || container;
+ const observer = new win.IntersectionObserver(this.observed_.bind(this), {
+ root,
+ threshold: DISPLAY_THRESHOLD,
+ });
+ this.observers_[index] = observer;
+ } else {
+ const observer = this.observers_[index];
+ this.observers_[index] = null;
+ if (observer) {
+ observer.disconnect();
+ }
+ }
+
+ const observer = this.observers_[index];
+ this.targetObserverCallbacks_.forEach((callbacks, target) => {
+ if (observer && containsNotSelf(container, target)) {
+ this.setObservation_(target, index, null, callbacks);
+ observer.observe(target);
+ } else {
+ this.setObservation_(target, index, false, callbacks);
+ }
});
}
@@ -196,19 +334,39 @@ export class DisplayObserver {
}
seen.add(target);
const callbacks = this.targetObserverCallbacks_.get(target);
- if (!callbacks) {
+ const index = this.observers_.indexOf(observer);
+ if (!callbacks || index == -1) {
continue;
}
- let observations = this.targetObservations_.get(target);
- if (!observations) {
- observations = emptyObservations(this.observers_.length);
- this.targetObservations_.set(target, observations);
+ this.setObservation_(target, index, isIntersecting, callbacks);
+ }
+ }
+
+ /**
+ * @param {!Element} target
+ * @param {number} index
+ * @param {?boolean} value
+ * @param {?Array} callbacks
+ * @private
+ */
+ setObservation_(target, index, value, callbacks) {
+ let observations = this.targetObservations_.get(target);
+ if (!observations) {
+ const observers = this.observers_;
+ observations = new Array(observers.length);
+ for (let i = 0; i < observers.length; i++) {
+ observations[i] = observers[i] ? null : false;
}
+ this.targetObservations_.set(target, observations);
+ }
+
+ if (callbacks) {
const oldDisplay = computeDisplay(observations, this.isDocDisplay_);
- const index = this.observers_.indexOf(observer);
- observations[index] = isIntersecting;
+ observations[index] = value;
const newDisplay = computeDisplay(observations, this.isDocDisplay_);
- notifyIfChanged(callbacks, newDisplay, oldDisplay);
+ notifyIfChanged(callbacks, target, newDisplay, oldDisplay);
+ } else {
+ observations[index] = value;
}
}
}
@@ -222,18 +380,6 @@ function getObserver(target) {
return /** @type {!DisplayObserver} */ (getServiceForDoc(target, SERVICE_ID));
}
-/**
- * @param {number} length
- * @return {!Array}
- */
-function emptyObservations(length) {
- const result = new Array(length);
- for (let i = 0; i < length; i++) {
- result[i] = null;
- }
- return result;
-}
-
/**
* @param {?Array} observations
* @param {boolean} isDocDisplay
@@ -243,7 +389,7 @@ function computeDisplay(observations, isDocDisplay) {
if (!isDocDisplay) {
return false;
}
- if (!observations) {
+ if (!observations || observations.length == 0) {
// Unknown yet.
return null;
}
@@ -282,26 +428,37 @@ function displayReducer(acc, value) {
return null;
}
+/**
+ * @param {!Element} container
+ * @param {!Element} child
+ * @return {boolean}
+ */
+function containsNotSelf(container, child) {
+ return container != null && child !== container && container.contains(child);
+}
+
/**
* @param {!Array} callbacks
+ * @param {!Element} target
* @param {boolean} newDisplay
* @param {boolean} oldDisplay
*/
-function notifyIfChanged(callbacks, newDisplay, oldDisplay) {
+function notifyIfChanged(callbacks, target, newDisplay, oldDisplay) {
if (newDisplay != null && newDisplay !== oldDisplay) {
for (let i = 0; i < callbacks.length; i++) {
- callCallbackNoInline(callbacks[i], newDisplay);
+ callCallbackNoInline(callbacks[i], target, newDisplay);
}
}
}
/**
* @param {!ObserverCallbackDef} callback
- * @param {../layout-rect.LayoutSizeDef} size
+ * @param {!Element} target
+ * @param {boolean} isDisplayed
*/
-function callCallbackNoInline(callback, size) {
+function callCallbackNoInline(callback, target, isDisplayed) {
try {
- callback(size);
+ callback(isDisplayed, target);
} catch (e) {
rethrowAsync(e);
}
diff --git a/test/unit/utils/test-display-observer.js b/test/unit/utils/test-display-observer.js
index d3ad5afb4b1e..aa9b029bfece 100644
--- a/test/unit/utils/test-display-observer.js
+++ b/test/unit/utils/test-display-observer.js
@@ -18,14 +18,16 @@ import {Deferred} from '../../../src/utils/promise';
import {
measureDisplay,
observeDisplay,
+ registerContainer,
unobserveDisplay,
+ unregisterContainer,
} from '../../../src/utils/display-observer';
import {removeItem} from '../../../src/utils/array';
describes.realWin('display-observer', {amp: true}, (env) => {
let win, doc, ampdoc;
let element;
- let docObserver, viewportObserver;
+ let docObserver, viewportObserver, containerObservers;
beforeEach(() => {
win = env.win;
@@ -42,7 +44,9 @@ describes.realWin('display-observer', {amp: true}, (env) => {
this.elements = [];
}
- disconnect() {}
+ disconnect() {
+ this.elements.length = 0;
+ }
observe(element) {
if (this.elements.includes(element)) {
@@ -53,7 +57,9 @@ describes.realWin('display-observer', {amp: true}, (env) => {
unobserve(element) {
if (!this.elements.includes(element)) {
- throw new Error('not observed');
+ throw new Error(
+ 'not observed: ' + element.id + ' on ' + this.options?.root?.id
+ );
}
removeItem(this.elements, element);
}
@@ -68,6 +74,7 @@ describes.realWin('display-observer', {amp: true}, (env) => {
docObserver = null;
viewportObserver = null;
+ containerObservers = new Map();
env.sandbox
.stub(win, 'IntersectionObserver')
.value(function (callback, options) {
@@ -80,6 +87,14 @@ describes.realWin('display-observer', {amp: true}, (env) => {
return (docObserver =
docObserver || new FakeIntersectionObserver(callback, options));
}
+ if (options.root) {
+ const containerObserver = new FakeIntersectionObserver(
+ callback,
+ options
+ );
+ containerObservers.set(options.root, containerObserver);
+ return containerObserver;
+ }
return new FakeIntersectionObserver(callback, options);
});
});
@@ -291,4 +306,147 @@ describes.realWin('display-observer', {amp: true}, (env) => {
expect(display3).to.be.true;
});
});
+
+ describe.only('registerContainer', () => {
+ let container;
+ let topElement;
+
+ beforeEach(() => {
+ container = doc.createElement('div');
+ container.id = 'container1';
+ doc.body.appendChild(container);
+ container.appendChild(element);
+
+ topElement = doc.createElement('div');
+ topElement.id = 'topElement1';
+ doc.body.appendChild(topElement);
+ });
+
+ it('should create observer only after container display is known', async () => {
+ registerContainer(container);
+ expect(containerObservers.get(container)).to.not.exist;
+
+ await viewportObserver.notify([
+ {target: container, isIntersecting: false},
+ ]);
+ expect(containerObservers.get(container)).to.not.exist;
+
+ await viewportObserver.notify([
+ {target: container, isIntersecting: true},
+ ]);
+ expect(containerObservers.get(container)).to.exist;
+ });
+
+ it('should only observe contained elements', async () => {
+ const elementCallback = createCallbackCaller();
+ observeDisplay(element, elementCallback);
+ const topElementCallback = createCallbackCaller();
+ observeDisplay(topElement, topElementCallback);
+
+ viewportObserver.notify([{target: element, isIntersecting: false}]);
+ docObserver.notify([{target: element, isIntersecting: false}]);
+ viewportObserver.notify([{target: topElement, isIntersecting: false}]);
+ docObserver.notify([{target: topElement, isIntersecting: false}]);
+
+ const display1 = await elementCallback.next();
+ const display2 = await topElementCallback.next();
+ expect(display1).to.be.false;
+ expect(display2).to.be.false;
+
+ registerContainer(container);
+ await viewportObserver.notify([
+ {target: container, isIntersecting: true},
+ ]);
+
+ const containerObserver = containerObservers.get(container);
+ expect(containerObserver.elements).to.include(element);
+ expect(containerObserver.elements).to.not.include(topElement);
+
+ containerObserver.notify([{target: element, isIntersecting: true}]);
+ const display3 = await elementCallback.next();
+ const display4 = await topElementCallback.next();
+ expect(display3).to.be.true;
+ expect(display4).to.be.false; // no change.
+ });
+
+ it('should unregister observer', async () => {
+ const elementCallback = createCallbackCaller();
+ observeDisplay(element, elementCallback);
+
+ viewportObserver.notify([{target: element, isIntersecting: false}]);
+ docObserver.notify([{target: element, isIntersecting: false}]);
+
+ const display1 = await elementCallback.next();
+ expect(display1).to.be.false;
+
+ registerContainer(container);
+ await viewportObserver.notify([
+ {target: container, isIntersecting: true},
+ ]);
+ const containerObserver = containerObservers.get(container);
+ containerObserver.notify([{target: element, isIntersecting: true}]);
+ const display2 = await elementCallback.next();
+ expect(display2).to.be.true;
+
+ unregisterContainer(container);
+ expect(docObserver.elements).to.not.include(container);
+ expect(containerObserver.elements).to.not.include(element);
+
+ const display3 = await elementCallback.next();
+ expect(display3).to.be.false;
+ });
+
+ it('should change display when container observer is notified', async () => {
+ const elementCallback = createCallbackCaller();
+ observeDisplay(element, elementCallback);
+
+ viewportObserver.notify([{target: element, isIntersecting: false}]);
+ docObserver.notify([{target: element, isIntersecting: false}]);
+
+ const display1 = await elementCallback.next();
+ expect(display1).to.be.false;
+
+ registerContainer(container);
+ await viewportObserver.notify([
+ {target: container, isIntersecting: true},
+ ]);
+
+ const containerObserver = containerObservers.get(container);
+ containerObserver.notify([{target: element, isIntersecting: true}]);
+ const display2 = await elementCallback.next();
+ expect(display2).to.be.true;
+
+ containerObserver.notify([{target: element, isIntersecting: false}]);
+ const display3 = await elementCallback.next();
+ expect(display3).to.be.false;
+ });
+
+ it('should change display when container display has changed', async () => {
+ const elementCallback = createCallbackCaller();
+ observeDisplay(element, elementCallback);
+
+ viewportObserver.notify([{target: element, isIntersecting: false}]);
+ docObserver.notify([{target: element, isIntersecting: false}]);
+
+ const display1 = await elementCallback.next();
+ expect(display1).to.be.false;
+
+ registerContainer(container);
+ await docObserver.notify([{target: container, isIntersecting: false}]);
+ await viewportObserver.notify([
+ {target: container, isIntersecting: true},
+ ]);
+
+ const containerObserver = containerObservers.get(container);
+ containerObserver.notify([{target: element, isIntersecting: true}]);
+ const display2 = await elementCallback.next();
+ expect(display2).to.be.true;
+
+ await viewportObserver.notify([
+ {target: container, isIntersecting: false},
+ ]);
+ const display3 = await elementCallback.next();
+ expect(display3).to.be.false;
+ });
+ });
});
From e517e0a2adc62512fd26199775281e0c647cef53 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Tue, 16 Feb 2021 17:28:35 -0800
Subject: [PATCH 3/9] cleanup
---
examples/amp-lightbox.amp.html | 3 ---
extensions/amp-lightbox/0.1/amp-lightbox.js | 2 +-
src/utils/display-observer.js | 1 -
3 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/examples/amp-lightbox.amp.html b/examples/amp-lightbox.amp.html
index 13b0031dd421..f232cb5e9464 100644
--- a/examples/amp-lightbox.amp.html
+++ b/examples/amp-lightbox.amp.html
@@ -84,9 +84,6 @@ Scrollable Lightbox
height="204"
layout="responsive"
controls>
-
diff --git a/extensions/amp-lightbox/0.1/amp-lightbox.js b/extensions/amp-lightbox/0.1/amp-lightbox.js
index a2d96f2eab18..2d401be08195 100644
--- a/extensions/amp-lightbox/0.1/amp-lightbox.js
+++ b/extensions/amp-lightbox/0.1/amp-lightbox.js
@@ -562,7 +562,7 @@ class AmpLightbox extends AMP.BaseElement {
if (!this.active_) {
return;
}
- unregisterContainer(this.element, this.container_);
+ unregisterContainer(this.element);
if (this.isScrollable_) {
setStyle(this.element, 'webkitOverflowScrolling', '');
}
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index e6b76545bba7..06035955fe38 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -169,7 +169,6 @@ export class DisplayObserver {
this.containers_.push(container);
this.containerRoots_.set(container, root);
- // QQQ: externalize callback
this.observe(container, this.containerObserved_);
}
From 1c91c1461edbcc70dd463a126dabf936a32f6412 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Tue, 16 Feb 2021 21:12:42 -0800
Subject: [PATCH 4/9] docs
---
src/utils/display-observer.js | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index 06035955fe38..909e2b33cbe8 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -75,12 +75,17 @@ export function measureDisplay(target) {
}
/**
- * QQQQ
+ * Registers the container to provide additional display intersection info
+ * for other targets. Mainly aimed for fixed and/or scrollable containers
+ * that can provide display information in addition to the document flow.
+ *
* @param {!Element} container
- * @param {!Element} root
+ * @param {?Element=} opt_root The subelement inside the container to be
+ * used as an intersection root. If not specified, the container will be
+ * used as an intersection root.
*/
-export function registerContainer(container, root) {
- getObserver(container).registerContainer(container, root);
+export function registerContainer(container, opt_root) {
+ getObserver(container).registerContainer(container, opt_root);
}
/**
@@ -160,15 +165,15 @@ export class DisplayObserver {
/**
* @param {!Element} container
- * @param {!Element} root
+ * @param {?Element=} opt_root
*/
- registerContainer(container, root) {
+ registerContainer(container, opt_root) {
if (this.containers_.includes(container)) {
return;
}
this.containers_.push(container);
- this.containerRoots_.set(container, root);
+ this.containerRoots_.set(container, opt_root || null);
this.observe(container, this.containerObserved_);
}
@@ -226,9 +231,13 @@ export class DisplayObserver {
(i < CONTAINER_OFFSET ||
containsNotSelf(this.containers_[i - CONTAINER_OFFSET], target))
) {
+ // The `null` value will wait for the interection before deciding on
+ // the display value of `false`.
this.setObservation_(target, i, null, /* callbacks */ null);
this.observers_[i].observe(target);
} else {
+ // The `false` value will essentially ignore this observe when
+ // computing the display value.
this.setObservation_(target, i, false, /* callbacks */ null);
}
}
From 0b96d8c15eddcb5dce468f15f5c1a36ce8720761 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Wed, 17 Feb 2021 12:21:20 -0800
Subject: [PATCH 5/9] fix types
---
src/utils/display-observer.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index 909e2b33cbe8..868b0399a23a 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -216,7 +216,7 @@ export class DisplayObserver {
/**
* @param {!Element} target
- * @param {function(boolean)} callback
+ * @param {!ObserverCallbackDef} callback
*/
observe(target, callback) {
let callbacks = this.targetObserverCallbacks_.get(target);
@@ -260,7 +260,7 @@ export class DisplayObserver {
/**
* @param {!Element} target
- * @param {function()} callback
+ * @param {!ObserverCallbackDef} callback
*/
unobserve(target, callback) {
const callbacks = this.targetObserverCallbacks_.get(target);
From 474244eb79a9d1f22faefc1f465b6632a3717d32 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Thu, 18 Feb 2021 16:38:59 -0800
Subject: [PATCH 6/9] minor
---
src/utils/display-observer.js | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index 868b0399a23a..3a37d0eca8bb 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -161,6 +161,8 @@ export class DisplayObserver {
this.visibilityUnlisten_ = null;
this.targetObserverCallbacks_.clear();
this.targetObservations_.clear();
+ this.containerRoots_.clear();
+ this.containers_.length = 0;
}
/**
@@ -437,7 +439,7 @@ function displayReducer(acc, value) {
}
/**
- * @param {!Element} container
+ * @param {?Element} container
* @param {!Element} child
* @return {boolean}
*/
From a785f7171b2d5e8b1cda65092e73d80431ac3e11 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Thu, 18 Feb 2021 17:24:10 -0800
Subject: [PATCH 7/9] Combine observers and containers together
---
src/utils/display-observer.js | 174 ++++++++++++++---------
test/unit/utils/test-display-observer.js | 2 +-
2 files changed, 109 insertions(+), 67 deletions(-)
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index 3a37d0eca8bb..1cd05fc682d4 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -23,13 +23,23 @@ const SERVICE_ID = 'DisplayObserver';
const DISPLAY_THRESHOLD = 0.51;
-const CONTAINER_OFFSET = 2;
+const CUSTOM_CONTAINER_OFFSET = 2;
/**
* @typedef {function(boolean, !Element)}
*/
let ObserverCallbackDef;
+/**
+ * @typedef {{
+ * container: ?Element,
+ * root: ?Element,
+ * contains: function(!Element):boolean,
+ * io: ?IntersectionObserver,
+ * }}
+ */
+let ObserverDef;
+
/**
* Observes whether the specified target is displayable. The initial observation
* is returned shortly after observing, and subsequent observations are
@@ -109,23 +119,35 @@ export class DisplayObserver {
this.ampdoc_ = ampdoc;
const {win} = ampdoc;
+ const body = ampdoc.getBody();
+
+ this.observed_ = this.observed_.bind(this);
+ this.containerObserved_ = this.containerObserved_.bind(this);
- /** @private @const {!Array} */
+ /** @private @const {!Array} */
this.observers_ = [];
- const boundObserved = this.observed_.bind(this);
- this.observers_.push(
- new win.IntersectionObserver(boundObserved, {
- root: ampdoc.getBody(),
- threshold: DISPLAY_THRESHOLD,
- })
- );
+
// Viewport observer is only needed because `postion:fixed` elements
// are not observable by a documentElement or body's root.
- this.observers_.push(
- new win.IntersectionObserver(boundObserved, {
+ this.observers_.push({
+ container: null,
+ root: null,
+ contains: () => true,
+ io: new win.IntersectionObserver(this.observed_, {
threshold: DISPLAY_THRESHOLD,
- })
- );
+ }),
+ });
+
+ // Body observer: very close to `display:none` observer.
+ this.observers_.push({
+ container: body,
+ root: body,
+ contains: () => true,
+ io: new win.IntersectionObserver(this.observed_, {
+ root: body,
+ threshold: DISPLAY_THRESHOLD,
+ }),
+ });
/** @private {boolean} */
this.isDocDisplay_ = computeDocIsDisplayed(ampdoc.getVisibilityState());
@@ -144,25 +166,20 @@ export class DisplayObserver {
/** @private @const {!Map>} */
this.targetObservations_ = new Map();
-
- /** @private @const {!Array} */
- this.containers_ = [];
-
- /** @private @const {!Map} */
- this.containerRoots_ = new Map();
-
- this.containerObserved_ = this.containerObserved_.bind(this);
}
/** @override */
dispose() {
- this.observers_.forEach((observer) => observer.disconnect());
+ this.observers_.forEach((observer) => {
+ if (observer.io) {
+ observer.io.disconnect();
+ }
+ });
+ this.observers_.length = 0;
this.visibilityUnlisten_();
this.visibilityUnlisten_ = null;
this.targetObserverCallbacks_.clear();
this.targetObservations_.clear();
- this.containerRoots_.clear();
- this.containers_.length = 0;
}
/**
@@ -170,12 +187,19 @@ export class DisplayObserver {
* @param {?Element=} opt_root
*/
registerContainer(container, opt_root) {
- if (this.containers_.includes(container)) {
+ const index = findObserverByContainer(this.observers_, container);
+ if (index != -1) {
return;
}
- this.containers_.push(container);
- this.containerRoots_.set(container, opt_root || null);
+ this.observers_.push({
+ container,
+ root: opt_root || container,
+ contains: (target) => containsNotSelf(container, target),
+ // Start with null as IntersectionObserver. Will be initialized when
+ // the container itself becomes displayed.
+ io: null,
+ });
this.observe(container, this.containerObserved_);
}
@@ -183,21 +207,17 @@ export class DisplayObserver {
* @param {!Element} container
*/
unregisterContainer(container) {
- const index = CONTAINER_OFFSET + this.containers_.indexOf(container);
- if (index < CONTAINER_OFFSET) {
+ const index = findObserverByContainer(this.observers_, container);
+ if (index < CUSTOM_CONTAINER_OFFSET) {
// The container has been unregistered already.
return;
}
- // Remove container.
- this.containers_.splice(index - CONTAINER_OFFSET, 1);
- this.containerRoots_.delete(container);
-
// Remove observer.
const observer = this.observers_[index];
this.observers_.splice(index, 1);
- if (observer) {
- observer.disconnect();
+ if (observer.io) {
+ observer.io.disconnect();
}
// Unobserve the container itself.
@@ -228,15 +248,11 @@ export class DisplayObserver {
// Subscribe observers.
for (let i = 0; i < this.observers_.length; i++) {
- if (
- this.observers_[i] &&
- (i < CONTAINER_OFFSET ||
- containsNotSelf(this.containers_[i - CONTAINER_OFFSET], target))
- ) {
- // The `null` value will wait for the interection before deciding on
- // the display value of `false`.
+ const observer = this.observers_[i];
+ if (observer.io && observer.contains(target)) {
+ // Reset observation to `null` and wait for the actual measurement.
this.setObservation_(target, i, null, /* callbacks */ null);
- this.observers_[i].observe(target);
+ observer.io.observe(target);
} else {
// The `false` value will essentially ignore this observe when
// computing the display value.
@@ -274,8 +290,9 @@ export class DisplayObserver {
this.targetObserverCallbacks_.delete(target);
this.targetObservations_.delete(target);
for (let i = 0; i < this.observers_.length; i++) {
- if (this.observers_[i]) {
- this.observers_[i].unobserve(target);
+ const observer = this.observers_[i];
+ if (observer.io) {
+ observer.io.unobserve(target);
}
}
}
@@ -297,33 +314,30 @@ export class DisplayObserver {
* @private
*/
containerObserved_(isDisplayed, container) {
- const index = CONTAINER_OFFSET + this.containers_.indexOf(container);
- if (index < CONTAINER_OFFSET) {
+ const index = findObserverByContainer(this.observers_, container);
+ if (index < CUSTOM_CONTAINER_OFFSET) {
// The container has been unregistered already.
return;
}
+ const observer = this.observers_[index];
+
if (isDisplayed) {
const {win} = this.ampdoc_;
- const root = this.containerRoots_.get(container) || container;
- const observer = new win.IntersectionObserver(this.observed_.bind(this), {
- root,
+ observer.io = new win.IntersectionObserver(this.observed_, {
+ root: observer.root,
threshold: DISPLAY_THRESHOLD,
});
- this.observers_[index] = observer;
- } else {
- const observer = this.observers_[index];
- this.observers_[index] = null;
- if (observer) {
- observer.disconnect();
- }
+ } else if (observer.io) {
+ observer.io.disconnect();
+ observer.io = null;
}
- const observer = this.observers_[index];
this.targetObserverCallbacks_.forEach((callbacks, target) => {
- if (observer && containsNotSelf(container, target)) {
+ if (observer.io && observer.contains(target)) {
+ // Reset observation to `null` and wait for the actual measurement.
this.setObservation_(target, index, null, callbacks);
- observer.observe(target);
+ observer.io.observe(target);
} else {
this.setObservation_(target, index, false, callbacks);
}
@@ -332,10 +346,10 @@ export class DisplayObserver {
/**
* @param {!Array} entries
- * @param {!IntersectionObserver} observer
+ * @param {!IntersectionObserver} io
* @private
*/
- observed_(entries, observer) {
+ observed_(entries, io) {
const seen = new Set();
for (let i = entries.length - 1; i >= 0; i--) {
const {target, isIntersecting} = entries[i];
@@ -344,7 +358,7 @@ export class DisplayObserver {
}
seen.add(target);
const callbacks = this.targetObserverCallbacks_.get(target);
- const index = this.observers_.indexOf(observer);
+ const index = findObserverByIo(this.observers_, io);
if (!callbacks || index == -1) {
continue;
}
@@ -365,7 +379,7 @@ export class DisplayObserver {
const observers = this.observers_;
observations = new Array(observers.length);
for (let i = 0; i < observers.length; i++) {
- observations[i] = observers[i] ? null : false;
+ observations[i] = observers[i].io ? null : false;
}
this.targetObservations_.set(target, observations);
}
@@ -439,12 +453,40 @@ function displayReducer(acc, value) {
}
/**
- * @param {?Element} container
+ * @param {!Array} observers
+ * @param {!IntersectionObserver} io
+ * @return {number}
+ */
+function findObserverByIo(observers, io) {
+ for (let i = 0; i < observers.length; i++) {
+ if (observers[i].io === io) {
+ return i;
+ }
+ }
+ return -1;
+}
+
+/**
+ * @param {!Array} observers
+ * @param {!Element} container
+ * @return {number}
+ */
+function findObserverByContainer(observers, container) {
+ for (let i = 0; i < observers.length; i++) {
+ if (observers[i].container === container) {
+ return i;
+ }
+ }
+ return -1;
+}
+
+/**
+ * @param {!Element} container
* @param {!Element} child
* @return {boolean}
*/
function containsNotSelf(container, child) {
- return container != null && child !== container && container.contains(child);
+ return child !== container && container.contains(child);
}
/**
diff --git a/test/unit/utils/test-display-observer.js b/test/unit/utils/test-display-observer.js
index aa9b029bfece..bdb4528c96f8 100644
--- a/test/unit/utils/test-display-observer.js
+++ b/test/unit/utils/test-display-observer.js
@@ -307,7 +307,7 @@ describes.realWin('display-observer', {amp: true}, (env) => {
});
});
- describe.only('registerContainer', () => {
+ describe('registerContainer', () => {
let container;
let topElement;
From 4543b104eaff60cb9397ba726685079db12ee7d2 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Fri, 19 Feb 2021 10:21:41 -0800
Subject: [PATCH 8/9] eager reset of container observations
---
src/utils/display-observer.js | 17 ++++--
test/unit/utils/test-display-observer.js | 76 ++++++++++++++++++++++++
2 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index 1cd05fc682d4..c6b1514cc090 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -187,20 +187,29 @@ export class DisplayObserver {
* @param {?Element=} opt_root
*/
registerContainer(container, opt_root) {
- const index = findObserverByContainer(this.observers_, container);
- if (index != -1) {
+ const existing = findObserverByContainer(this.observers_, container);
+ if (existing != -1) {
return;
}
- this.observers_.push({
+ /** @type {!ObserverDef} */
+ const observer = {
container,
root: opt_root || container,
contains: (target) => containsNotSelf(container, target),
// Start with null as IntersectionObserver. Will be initialized when
// the container itself becomes displayed.
io: null,
- });
+ };
+ const index = this.observers_.length;
+ this.observers_.push(observer);
this.observe(container, this.containerObserved_);
+
+ this.targetObserverCallbacks_.forEach((_, target) => {
+ // Reset observation to `null` and wait for the actual measurement.
+ const value = observer.contains(target) ? null : false;
+ this.setObservation_(target, index, value, /* callbacks */ null);
+ });
}
/**
diff --git a/test/unit/utils/test-display-observer.js b/test/unit/utils/test-display-observer.js
index bdb4528c96f8..39031a9d6b91 100644
--- a/test/unit/utils/test-display-observer.js
+++ b/test/unit/utils/test-display-observer.js
@@ -65,6 +65,9 @@ describes.realWin('display-observer', {amp: true}, (env) => {
}
notify(entries) {
+ if (!entries.some(({target}) => this.elements.includes(target))) {
+ throw new Error('unobserved target');
+ }
const {callback} = this;
return Promise.resolve().then(() => {
callback(entries, this);
@@ -448,5 +451,78 @@ describes.realWin('display-observer', {amp: true}, (env) => {
const display3 = await elementCallback.next();
expect(display3).to.be.false;
});
+
+ it('should compute display for nested observers', async () => {
+ const childContainer = doc.createElement('div');
+ childContainer.id = 'child-container1';
+ container.appendChild(childContainer);
+ childContainer.appendChild(element);
+
+ const elementCallback = createCallbackCaller();
+ observeDisplay(element, elementCallback);
+ await viewportObserver.notify([{target: element, isIntersecting: false}]);
+ expect(elementCallback.isEmpty()).to.be.true;
+
+ await docObserver.notify([{target: element, isIntersecting: false}]);
+ expect(await elementCallback.next()).to.be.false;
+
+ // 1. Register childContainer.
+ registerContainer(childContainer);
+ expect(containerObservers.get(childContainer)).to.not.exist;
+ expect(elementCallback.isEmpty()).to.be.true;
+
+ // 2. Make child container undisplayed.
+ await viewportObserver.notify([
+ {target: childContainer, isIntersecting: false},
+ ]);
+ await docObserver.notify([
+ {target: childContainer, isIntersecting: false},
+ ]);
+ expect(containerObservers.get(childContainer)).to.not.exist;
+ expect(await elementCallback.next()).to.be.false;
+
+ // 3. Register parent container.
+ registerContainer(container);
+ expect(containerObservers.get(container)).to.not.exist;
+ expect(elementCallback.isEmpty()).to.be.true;
+
+ // 4. Make parent container displayed, but child is still undisplayed.
+ await docObserver.notify([{target: container, isIntersecting: true}]);
+ expect(containerObservers.get(container)).to.exist;
+ expect(elementCallback.isEmpty()).to.be.true;
+
+ // 5. Intersect the child container inside the parent container.
+ await containerObservers
+ .get(container)
+ .notify([{target: childContainer, isIntersecting: true}]);
+ expect(containerObservers.get(childContainer)).to.exist;
+ expect(elementCallback.isEmpty()).to.be.true;
+
+ // 6. Intesect the element inside the child container.
+ await containerObservers
+ .get(childContainer)
+ .notify([{target: element, isIntersecting: true}]);
+ expect(await elementCallback.next()).to.be.true;
+ });
+
+ it('should not interrupt observations for the unrelated targets', async () => {
+ const elementCallback = createCallbackCaller();
+ observeDisplay(topElement, elementCallback);
+
+ // 1. Register a container, but not observations on it yet.
+ registerContainer(container);
+ expect(elementCallback.isEmpty()).to.be.true;
+
+ await viewportObserver.notify([
+ {target: topElement, isIntersecting: false},
+ ]);
+ await docObserver.notify([{target: topElement, isIntersecting: false}]);
+ expect(await elementCallback.next()).to.be.false;
+
+ // 2. Provide observations for the container.
+ await docObserver.notify([{target: container, isIntersecting: true}]);
+ expect(containerObservers.get(container)).to.exist;
+ expect(elementCallback.isEmpty()).to.be.true;
+ });
});
});
From 5eaee0dfbff9a90a6cae216ecb0e4210297f2852 Mon Sep 17 00:00:00 2001
From: Dima Voytenko
Date: Fri, 19 Feb 2021 10:32:36 -0800
Subject: [PATCH 9/9] shortcircuit
---
src/utils/display-observer.js | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/utils/display-observer.js b/src/utils/display-observer.js
index c6b1514cc090..fda814cffbe3 100644
--- a/src/utils/display-observer.js
+++ b/src/utils/display-observer.js
@@ -330,6 +330,10 @@ export class DisplayObserver {
}
const observer = this.observers_[index];
+ if (isDisplayed && observer.io) {
+ // Has already been initialized.
+ return;
+ }
if (isDisplayed) {
const {win} = this.ampdoc_;