Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Dima Voytenko committed Mar 24, 2021
1 parent a88b2e1 commit 14d60c8
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 6 deletions.
6 changes: 5 additions & 1 deletion src/base-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,11 @@ export class BaseElement {

/**
* Set itself as a container element that can be monitored by the scheduler
* for auto-mounting.
* for auto-mounting. Scheduler is used for V1 elements. A container is
* usually a top-level scrollable overlay such as a lightbox or a sidebar.
* The main scheduler (`IntersectionObserver`) cannot properly handle elements
* inside a non-document scroller and this method instructs the scheduler
* to also use the `IntersectionObserver` corresponding to the container.
*
* @param {!Element} opt_scroller A child of the container that should be
* monitored. Typically a scrollable element.
Expand Down
2 changes: 1 addition & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
}

/**
* See `BaseElement.setAsContainer`.
* See `BaseElement.removeAsContainer`.
* @restricted
* @final
*/
Expand Down
4 changes: 2 additions & 2 deletions src/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -984,8 +984,8 @@ export function dispatchCustomEvent(node, name, opt_data, opt_options) {
/**
* Ensures the child is contained by the parent, but not the parent itself.
*
* @param {!Element} parent
* @param {!Element} child
* @param {!Node} parent
* @param {!Node} child
* @return {boolean}
*/
export function containsNotSelf(parent, child) {
Expand Down
9 changes: 7 additions & 2 deletions src/service/scheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ export class Scheduler {
});
this.containerMap_.set(container, observer);

// Subscribe all pending children.
// Subscribe all pending children. Ignore `asap` targets since they
// will be scheduled immediately and do not need an intersection
// observer input.
this.targets_.forEach(({asap}, target) => {
if (!asap && containsNotSelf(container, target)) {
observer.observe(target);
Expand All @@ -167,6 +169,7 @@ export class Scheduler {

// Disconnect. All children will be unobserved automatically.
observer.disconnect();
this.containerMap_.delete(container);
}

/** @private*/
Expand Down Expand Up @@ -247,7 +250,9 @@ export class Scheduler {
}

const isIntersecting = isThisIntersecting || current.isIntersecting;
this.targets_.set(target, {asap: current.asap, isIntersecting});
if (isIntersecting !== current.isIntersecting) {
this.targets_.set(target, {asap: current.asap, isIntersecting});
}
if (isIntersecting) {
this.maybeBuild_(target);
}
Expand Down

0 comments on commit 14d60c8

Please sign in to comment.