-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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
[Bug Report][3.3.3] VInfiniteScroll does not set root element correctly unless margin prop is explicitly supplied #17583
[Bug Report][3.3.3] VInfiniteScroll does not set root element correctly unless margin prop is explicitly supplied #17583
Comments
I feel like it makes more sense to always use the viewport as the root, never the actual root element. For example if you don't specify a height it works as expected: Playground |
By this do you mean it gets stuck after the first load cycle? Or something else? If yes then that's the issue in #17358. Thinking about always using the viewport for the root, what then happens if you have a relatively small infinitely scrollable area within the viewport and you do set the height explicitly? If the viewport is used as the root then the intersection will never leave the viewport, no? Or does the intersection observer get notified when the intersection |
Specifically in the playground I posted, which loads enough items to avoid #17358
Yes, it takes into account other scrolling elements between root and target. You can see this on https://vuetifyjs.com/en/components/infinite-scroller/, none of the examples have margin so root = null |
Interesting. I was under the (seemingly incorrect) assumption that Also interesting that you say none of the examples at https://vuetifyjs.com/en/components/infinite-scroller/ have the root set, but all work as one might expect. Again, empirically that is the case. In my digging I came across https://blog.webdevsimplified.com/2022-01/intersection-observer/ which has some similar examples for observing content as it scrolls and the author states he had to specify the root in order to get them working correctly. Quote follows, unfortunately he doesn't go into more detail as to what the issues were without the root set.
Conceptually it feels to me that the It seems there are a few different use cases in play.
What would be really confusing and a horrible UX would be if you end up in a situation with scrollbars on both the viewport and the All that said, my initial concern was the inconsistency in the configuration, in that the root element was only specified if margin is also set. If consistent behaviour can be obtained using the viewport as the root always then that seems ok. |
I guess to put it another way, if the intersection is always calculated taking into account clipping of the target element w.r.t. any of its parent elements between the target and the I've answered my own question re documentation and am reading the w3c spec. Comment near the start.
In which case maybe the only reason to specify a root element is for efficiency to avoid performing as many clipping/bounds calculations if it's a complex layout. 🤷♂️ |
Even the MDN example responds to both its container and the window and that's in an iframe.
That's maybe correct behaviour as you wouldn't actually be able to see the bottom of the list anyway. We could work around it by adding observers for both rootEl and window if rootEl is scrollable but having two scrollbars like that is horrible UX in the first place so it's kinda your fault if it ends up broken. |
Good point, I hadn't tried playing with that example with the container half outside the viewport. It would have been patently obvious very quickly that my assumption was incorrect as it updates regardless of which you scroll. Long story short, I agree with your initial comment that using the viewport as the root should be fine in all cases.
Completely agree. Maybe something could be added to the docs to indicate it's suggested to either set an explicit height, or make the scroller itself non-scrollable to avoid it. Meh. Not sure if that adds any value. |
I'm switching to a negative margin-top on the intersect element instead of using rootMargin. |
Environment
Vuetify Version: 3.3.3
Vue Version: 3.3.4
Browsers: Chrome 114.0.0.0
OS: Mac OS 10.15.7
Steps to reproduce
Use a
v-infinite-scroll
element without themargin
prop. Theroot
is not passed to theIntersectionOberserver
which therefore uses the viewport as the root for intersection calculations.More details
In the linked playground run as-is. It will use the viewport for the root element. It will only load one extra set of data (3 more items) [and get stuck - as per issue 17358]. Resize the viewport such that the intersection
div
leaves and re-enters the viewport. The infinite scroll will be triggered to load more data.Comment/uncomment the two
v-infinite-scroll
tags in the playground. Behaviour changes such that the wrapperdiv
is now correctly the root element. But, no amount of scrolling will now get more data to load.Expected Behavior
Whether the
margin
prop is specified or not, thev-infinite-scroll
wrapperdiv
is used for the root element.Actual Behavior
With no explicit margin prop the
IntersectionObserver
uses the viewport as the root for intersection calculations.Reproduction Link
https://play.vuetifyjs.com/#...
Other comments
Rather than defaulting to
undefined
,margin
should probably default to zero.Interestingly, this issue allows a workaround for #17358 as the browser can be resized continually to move the intersection
div
in and out of the viewport until such time as the loaded content fills the container and naturally pushes the intersectiondiv
out of the viewport. At this time the behaviour returns to expected and new content can be loaded simply by scrolling.Thus, fixing this issue before that issue will prevent that workaround.
The text was updated successfully, but these errors were encountered: