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

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

Open
KapJI opened this issue Feb 19, 2020 · 9 comments
Open

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

KapJI opened this issue Feb 19, 2020 · 9 comments
Assignees
Labels

Comments

@KapJI
Copy link

KapJI commented Feb 19, 2020

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 :(

@nicolejadeyee nicolejadeyee added the Type: Bug Something isn't working label Feb 20, 2020
@KapJI
Copy link
Author

KapJI commented Feb 20, 2020

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
Copy link

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
Copy link

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
Copy link

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
Copy link

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:

  • Set the host element's style.display to block if it's not already set to something else
  • Set the host element's style.position to relative if it's not already set to something else
  • Set the host element's style.overflow to auto if it's not already set to something else
  • Set the host element's style.contain to strict if it's not already set to something else

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
Copy link

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
Copy link
Author

KapJI commented May 25, 2021

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
Copy link

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
Copy link
Author

KapJI commented May 25, 2021

Thanks!

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

No branches or pull requests

4 participants