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

Ability to have automatic rootMargin for all nested scroll containers #431

Closed
zcorpan opened this issue May 19, 2020 · 15 comments · Fixed by #511
Closed

Ability to have automatic rootMargin for all nested scroll containers #431

zcorpan opened this issue May 19, 2020 · 15 comments · Fixed by #511

Comments

@zcorpan
Copy link
Member

zcorpan commented May 19, 2020

Originally posted by @zcorpan in whatwg/html#5408 (comment):

Scrolling vertically & horizontally for:

  • Element scroll container scrolling

So, I'm not sure how this would work. In particular, for the image carousel use case, using only the implicit root (which I think browser implementations do now) would mean that there is no threshold for the element scroll container case, so those images would only start loading after they are partially in view.

There is likely a performance hit to observing all scrollable elements when lazy images are used. Is there a good way to make it "do what I want" without adding more API surface? Or is the web developer explicitly setting the root the best way to solve this?

Should IntersectionObserver API provide for a way to apply a rootMargin for all scroll containers, without needing to create multiple observers (and respond to styling changes causing scroll containers to appear/disappear)?

One use case here is lazy-loading for image carousels. The page has a scrollable div and many images with horizontal scrolling. To speed up page load time and reduce network usage, the images that aren't initially visible are annotated with loading=lazy.

The loading=lazy feature is defined in terms of IntersectionObserver, with root set to implicit root for all elements.

As specified and implemented right now, this causes the images in the image carousel to only start loading when they are at least partially visible -- because the root is the top-level viewport, not the scroll container.

I think ideally an image in this case would start loading when:

  • the scroll container is "about to become visible" or is visible
  • the image in the scroll container is "about to become visible" or is visible

I don't know how to solve this, but I'm hoping for something that results in a good user experience with the current web developer ergonomics of only adding loading=lazy to the right images.

cc @domfarolino @emilio @smfr

@chrishtr
Copy link

chrishtr commented Jun 2, 2020

A somewhat related use-case is providing clip margins for lazy-loaded images underneath a clipping ancestor. e.g.:

<div style="overflow:hidden">
  <img src=... loading="lazy" style="margin-left: -1px">
</div>

As currently specified this image will never load, because it has a 0x0 size at first and is clipped out.

It might be useful for there to be an option where the starting box of the image is increased by some extra margin to avoid off-by-one or off-by-constant issues like this, and the image has no dimensions set.

I think this example violates developer best practices - sites should not be using lazy-loaded images without a width/height attribute set or CSS equivalent. However, this option might make sites more reliable in cases where they fail to set width or height.

@emilio
Copy link
Collaborator

emilio commented Oct 26, 2020

FWIW I think implementation-wise this should be pretty easy.

@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

@smfr
Copy link

smfr commented Oct 30, 2020

What I hear from the authors of music.apple.com (which has script-driven lazy loading right now) is that for this scenario they would prefer, for lazy-loaded images in carousels, that the vertical root margin be some multiple of viewport height (e.g. 2x), and the margin for the scrollers are some smaller multiple of scroller or viewport width (say 0.5x).

@johannesodland
Copy link

This would be useful both for JS based and native lazy loading in carousels, as native lazy loading is using IntersectionObserver under the hood.

At the moment the Google result page has a scroll carousel for shopping ads and images. DuckDuckGo has one for recent news. NY Times is using one on the front page for iframed videos. Instagram is using one for image carousels.

@smfr
Copy link

smfr commented Sep 29, 2021

WebKit is interested in finishing off lazy image loading, and we believe this issue needs to be resolved for a good implementation of that feature.

@zcorpan
Copy link
Member Author

zcorpan commented Oct 21, 2021

Should we discuss this again at TPAC this year, to try to come up with a proposal? @szager-chromium @emilio

@zcorpan
Copy link
Member Author

zcorpan commented Oct 27, 2021

Minutes from the TPAC meeting for this issue: https://www.w3.org/2021/10/26-webapps-minutes.html#t17

One-line summary:

sz: we just need a clear + detailed proposal

@tcaptan-cr
Copy link
Collaborator

This request and the related issue of trying to efficiently load images in carousels using lazy loading continues to be raised by partners. Based on the previous conversation it seems that all that was needed was to get

"a clear + detailed proposal".

Inspired by Emilio's proposal, how about we add an optional scrollMargin entry to the IntersectionObserverInit dictionary. If unspecified it would default to zero, maintaining the current behavior. The intersection observer algorithm would be modified (section 3.2.7, as step 3.3) to inflate the intersectionRect by scrollMargin for each nested scroll container.

A lazy load scrollMargin could then be integrated into the lazy loading feature in a similar manner to lazy load root margin to address the issues around images in scroll containers (e.g., carousels) not being lazy loaded.

@chrishtr
Copy link

The WG held a meeting to discuss this issue this week.

Outcomes:

  • Consensus from attendees to add this feature. Next step is to write the detailed spec for further feedback.
  • Implementation notes:
    • Should be defined against the bounding rect of the clip (not the border box of the element)
    • Should apply to all types of clip (including clip-path etc)
    • Should stop margins at a cross-origin iframe boundary for security
    • Should support percentage units
Detailed notes Philip: use case is in part for carousels in nested scrollers. TL;DR of proposal is to provide automatic margin for nested scrollers. Emilio: seems generally uncontroversial. And should be more than just for this case, with a generalized name. Maybe scroll margin. Also, should it work with clip path and clip? Stefan: or clip margin? Emilio: just need to make sure that all of the details of scrolling in all its modes, and nested scrollers, works correctly. Stefan: can think of a case where it works with overflow but not with clip path. Think we should probably restrict it to just overflow scrolling. Emilio: seems weird that switching from overflow clip to clip path would make the feature stop working. Perhaps we could have a dictionary argument identifying opt-ins or opt-outs for cases when we don’t want it to apply to all types of clips. Stefan: clip-path or css clip seem different because it is not common or easy to move something out from under such clips, because they don’t come with a user or developer-facing API to make things come out? Simon: +1 to what Stefan said. Emilio: have seen examples of animated clip paths that reveal content. Chris: +1 to what Emilio said. Pdr: should we consider only expanding in the direction of where scrolling is allowed? What happens if you have an image with rounded corners and then inside of a scroller. Don’t want to apply a double margin just because there is a clipping effect on the image. Traian: you don’t add up margins, you just expand each clip applied on the way up. Emilio: there may be cases where you only apply margins in the desired scrolling direction, but.. Stefan: that seems hard, don’t want to do that? Emilio: agree. But if you have overflow-x: clip; overflow-y: scroll, it could be reasonable to have that. Don’t need to do it now, but should design the API so that we could add it later if needed. Chris: is a single margin across all nested scrollers good enough? Stefan: prefer to keep it simple, and also solve more obscure corner cases with composition. Simon: should we allow a percentage value for the margin? Stefan: agree, but how should we define this percentage? Stefan: choices seem to be against the root, and also against each nested scroller. Current root margin is a percentage of the explicit or implicit root element. Stefan: also need to decide what to do about cross-origin iframes. Do we honor scroll margin for implicit intersection observers created in cross-origin iframes? Stefan: think it would be safer for security not to honor it for cross-origin. Chris: need percentages to not go across a cross-origin iframe. Emilio: if we do percentages we should probably do it per element that introduces clip. We could also skip percentages for now, but does seem useful. Chris: what’s the relation to overflow-clip-margin? Emilio: this means we should use the bounding rect of the clip including any clip margin, then add margin with percentages resolved according to the size of the bounding clip rect. Stefan: we should go and verify whether the current root margin feature of IO works correctly with overflow-clip-margin. Chris: “resolved”

@zcorpan
Copy link
Member Author

zcorpan commented May 11, 2023

Should support percentage units

Also, percentages should be relative to each clipping element's bounding rect of the clip.

@wangxianzhu
Copy link

I have two questions:

  1. Should scrollMargin apply in the following case?
<div id="target" style="position: fixed; top: -200px; width: 100px; height: 100px"></div>
  1. As scrollMargin applies to all types of clips, should it be named clipMargin instead of scrollMargin?

@emilio
Copy link
Collaborator

emilio commented Dec 12, 2023

I have two questions:

1. Should scrollMargin apply in the following case?
<div id="target" style="position: fixed; top: -200px; width: 100px; height: 100px"></div>

If it applies to the root scrollport, yes.

2. As `scrollMargin` applies to all types of clips, should it be named `clipMargin` instead of `scrollMargin`?

It should only apply to the scrollport clip, right? https://w3c.github.io/IntersectionObserver/#apply-scroll-margin-to-a-scrollport

@wangxianzhu
Copy link

I asked the first question because I was wondering if scrollMargin should apply to targets that can never be scrolled to be visible. Now I know that we want to keep the spec simple by ignoring scrollability for now.

I realized my second question was based on #431 (comment) (which mentioned scrollMargin should apply to all types of clips) instead of the current spec.

It seems that we also don't need to consider overflow-clip-margin because it applies to only overflow-clip which doesn't create a scroll container.

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