PolymerLabs / uni-virtualizer

Monorepo for a "universal" (renderer-agnostic) virtual scroller and renderer-specific variants
BSD 3-Clause "New" or "Revised" License
95 stars 13 forks source link

lit-virtualizer doesn't work os iOS/ipadOS properly #54

Open KapJI opened 4 years ago

KapJI commented 4 years ago

I noticed lit-virtualizer doesn't work on iOS both in Safari and Chrome. Scrolling stops when new element is added in VirtualScroller. So you can scroll one item at time. You can see this on your examples.

But at the same time it works fine in Safari and Chrome on macOS as well as in Chrome for Android.

Interestingly, that example scroll-to-index-lit-html with scroll() works fine. I'm not sure what the difference is. But simple replacing <lit-virtualizer> with scroll() doesn't help in the project which I'm trying to fix :(

KapJI commented 4 years ago

It seems that it happens when you try to append element to shadow DOM root. If in scroll-to-index-lit-html you pass useShadowDOM=true to scroll() then it also stops working. But if you don't create new shadow root in VirtualScroller._applyContainerStyles() then it works fine even if scroll() is inside shadow root itself (but then you need to set styles manually). Hope that helps.

bramkragten commented 3 years ago

The useShadowDOM option was removed in version 0.5, and it now always creates a shadow root, there seems no easy way the fix this issue for Safari anymore now...

Was the removal of useShadowDOM a necessity for the new layout features?

graynorton commented 3 years ago

Thanks for bringing this to my attention. I'm not inclined to restore this API option, but I will figure out a way to fix the issue with mobile Safari.

graynorton commented 3 years ago

I filed this WebKit bug with a simple repro case:

https://bugs.webkit.org/show_bug.cgi?id=226195

Meanwhile, I am working on a temporary fix.

graynorton commented 3 years ago

I think the best workaround for now is for virtualizer not to use Shadow DOM at all but instead set CSS properties directly via the host element's style property.

This is a bit ugly and requires some care, but I can't think of a better option.

Specifically, virtualizer will:

This will allow everything to "just work" in the most common case where virtualizer is in complete control, while still allowing these properties to be overridden by the developer (using explicit style props or stylesheet rules with !important) for unusual use cases.

Does that sound reasonable to you, @bramkragten & @KapJI?

bramkragten commented 3 years ago

Yes, that sounds ok, we used to have something like that with using the directive without useShadowDOM instead of the custom element and setting our own styles.

KapJI commented 3 years ago

I think this should work.

Have you considered just wrapping it with extra div? As far as I remember this issue only occurs when you add elements to the shadow DOM root, adding to non root node should be fine, but this may cause compatibility issues with the current version.

graynorton commented 3 years ago

Thanks for the suggestion, @KapJI! I took a quick look at what it would mean to wrap the light-dom children in a div. That would be another potentially viable solution, but it would require deeper changes to virtualizer code, so I think I'll proceed with the fix I outlined above.

KapJI commented 3 years ago

Thanks!