Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watcher support for elements not within the DOM #76

Closed
lynchbomb opened this issue Jan 17, 2018 · 6 comments
Closed

Watcher support for elements not within the DOM #76

lynchbomb opened this issue Jan 17, 2018 · 6 comments
Assignees

Comments

@lynchbomb
Copy link
Member

Spaniel v2.4.6 does not provide support for elements being watched that are not within the DOM. Currently the ElementScheduler caches getBoundingClientRect which is set during the watch phase. Elements not yet within the DOM return default values on getBoundingClientRect which are then cached within the ElementScheduler.

watch(el: Element, callback: (frame: FrameInterface, id: string, clientRect?: ClientRect) => void, id?: string): string {
this.startTicking();
id = id || generateToken();
let clientRect = el.getBoundingClientRect();
this.queue.push({
el,
callback,
id,
clientRect
});
return id;
}

Thoughts on exposing a method within Spaniel to force a revalidation of cached getBoundingClientRect on elements being watched? Either on the Watcher or exposing the W (window proxy).

@stefanpenner
Copy link
Member

stefanpenner commented Jan 17, 2018

Thoughts on exposing a method within Spaniel to force a revalidation of cached getBoundingClientRect on elements being watched? Either on the Watcher or exposing the W (window proxy).

To make sure we solve this correctly, do you mind providing 1 or 2 examples of needing this functionality. That way we can make sure we are solving the right problem.

e.g. in what valid cases should we try to watch a detached element. Often times, I can see that being an error, in what cases is it legit?

@lynchbomb
Copy link
Member Author

The seemingly legit use-case is via lazy loading images with fixed dimensions and ghosting. Something like leveraging Ember-Spaniel#onInViewportOnce for implementation which would leverage the Spaniel#ElementScheduler.

@lynchbomb
Copy link
Member Author

DM with @stefanpenner:
So one question would be, in theory could Ember-Spaniel#onInViewport be leveraged only when the lazy loaded image is in the DOM? Assuming it was properly detached. Hypothetically this would work around the IE11 bug entirely (#75).

In addition we may actually want to consider making Spaniel (in dev) explode if Watcher tries to watch something detached from the DOM.

@stefanpenner
Copy link
Member

stefanpenner commented Jan 18, 2018

I believe that is safe, and would be an "honest" cross browser implementation, as it would only support what it could safely support across its browsers. Eventually if IE11 is dropped, we could choose to loosen.

thoughts @asakusuma / @krisselden

@lynchbomb
Copy link
Member Author

After DMing with @krisselden another proposed solve would be removing the caching of getBoundingClientRect during the init of ElementScheduler#watch. Keeping the ideal of nothing in the read queue should cause a run loop, set() or manipulate the DOM. Nothing outside the queue should read from the DOM only memoized state from read queue.

https://github.com/linkedin/spaniel/blob/v2-release/src/metal/scheduler.ts#L162-L188

If we set clientRect to null here: https://github.com/linkedin/spaniel/blob/v2-release/src/metal/scheduler.ts#L179

and memoized clientRect within the ElementScheduler#applyQueue (read queue) this issue should be solved.

@lynchbomb
Copy link
Member Author

This PR: #79 should address this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants