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.

Sign up for GitHub

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

Jump to bottom

Allow dynamically changing rootMargin #428

Open
zcorpan opened this issue May 8, 2020 · 8 comments
Open

Allow dynamically changing rootMargin #428

zcorpan opened this issue May 8, 2020 · 8 comments
Labels
enhancement

Comments

@zcorpan
Copy link
Member

zcorpan commented May 8, 2020 •
edited
Loading

In whatwg/html#5510 I'm trying to specify <img loading=lazy> in terms of IntersectionObserver as per discussion in whatwg/html#5236

The desired behavior for lazy-loading is to dynamically change the rootMargin in response to things like scrolling and network changes. See whatwg/html#5408 (comment)

Since IntersectionObserver only allows setting the rootMargin upon initialization, this would require one of

  • Register a new IntersectionObserver and observe all the same elements as the old one, and get rid of the old one. This seems bad, but I assume is what a JS polyfill would have to do?
  • Have the browser internally update the rootMargin even though the public API doesn't allow for it.

I think the second option seems better, but per the extensible web manifesto we try to have web features be explained by lower-level APIs without having special magic that isn't accessible to web developers.

Are there issues with modifying the rootMargin after initialization?
@zcorpan zcorpan mentioned this issue May 8, 2020
Merged
3 tasks
@domfarolino
Copy link
Member

This seems reasonable, but also I don't think it should block whatwg/html#5510

@zcorpan
Copy link
Member Author

zcorpan commented May 18, 2020

Agreed. I think it doesn't need to block fixing whatwg/html#5408 either, we could temporarily violate the extensible web manifesto here. But fixing this first certainly seems nicer to me!

@zcorpan zcorpan mentioned this issue Sep 15, 2020
Closed
This was referenced Sep 30, 2020
Closed
Closed
@zcorpan zcorpan mentioned this issue Oct 16, 2020
Closed
@emilio
Copy link
Collaborator

emilio commented Oct 19, 2020

I don't think there'd be an issue allowing this.

@zcorpan
Copy link
Member Author

zcorpan commented Oct 29, 2020

There will be a TPAC breakout session tomorrow (30 October 14:00–15:00 UTC) to discuss this issue.

https://www.w3.org/2020/10/TPAC/breakout-schedule.html#intersectionobserver

@zcorpan
Copy link
Member Author

zcorpan commented Oct 30, 2020

The breakout starts in ~20 minutes. Agenda and meeting notes here: https://docs.google.com/document/d/1SXwbMiwOUkLPXSM_klk4q9mFEanbIH8IAgTM80cCMXU/edit?usp=sharing

@dysfunc dysfunc mentioned this issue Mar 23, 2021
Merged
@asakusuma
Copy link

@zcorpan I read the meeting notes, has someone proposed an API here to allow rootMargin mutation?

Another use case here is top navbars that change size.

@zcorpan
Copy link
Member Author

zcorpan commented Sep 23, 2021

@asakusuma The API we gravitated towards was to allow assigning to rootMargin (and @smfr suggested also allow changing thresholds for symmetry).

In metonym/svelte-intersection-observer#12 (comment) @dysfunc also explains a use case for this feature. The implementation in svelte-intersection-observer validates that there is a web developer need for this feature.

cc @chrishtr

zcorpan added a commit to bocoup/IntersectionObserver that referenced this issue Oct 21, 2021
@zcorpan
Allow setting rootMargin and thresholds
24fe6a1 
Fixes w3c#428.
@zcorpan zcorpan mentioned this issue Oct 21, 2021
Open
4 tasks
@zcorpan
Copy link
Member Author

zcorpan commented Oct 21, 2021 •
edited
Loading

PRs: #485 (thresholds), #486 (rootMargin)

zcorpan added a commit to bocoup/IntersectionObserver that referenced this issue Oct 26, 2021
@zcorpan
Allow setting rootMargin
9c77efd 
Part of w3c#428.
@zcorpan zcorpan mentioned this issue Oct 26, 2021
Open
4 tasks
@pmusaraj pmusaraj mentioned this issue Oct 28, 2021
Merged
This was referenced Jun 9, 2022
Closed
Closed
@shcheklein shcheklein mentioned this issue Nov 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@zcorpan @asakusuma @emilio @szager-chromium @domfarolino