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

sl-carousel goToSlide has synchronous layout #2280

Closed
patrick-mcdougle opened this issue Nov 22, 2024 · 2 comments · Fixed by #2281
Closed

sl-carousel goToSlide has synchronous layout #2280

patrick-mcdougle opened this issue Nov 22, 2024 · 2 comments · Fixed by #2281
Labels
bug Things that aren't working right in the library.

Comments

@patrick-mcdougle
Copy link
Contributor

Describe the bug

The initial render of the sl-carousel calls the initialize slides function which calls goToSlide. goToSlide uses getBoundingClientRect to determine the geometry of the slides to know how far to scroll them. This is fine in the case that your app is rendering 1 carousel, but components that render many carousels suffer from having a lot of synchronous layouts in a row. For example, on Rothy's collection pages, each of our product cards has a sl-carousel on them.

To Reproduce

  1. Have multiple carousels on one page (and mount them all in the same tick)
  2. Perform a performance capture on page load
  3. See that each of the carousels has a forced relayout

Screenshots

Legible Flamechart

Screenshot 2024-11-22 at 4 00 21 PM

Zooming out we have 100 carousels!

Screenshot 2024-11-22 at 4 00 41 PM

Browser / OS

  • OS: Mac
  • Browser: Chrome
  • Browser version: 131

Additional information

https://web.dev/articles/avoid-large-complex-layouts-and-layout-thrashing
https://gist.github.com/paulirish/5d52fb081b3570c81e3a

@patrick-mcdougle patrick-mcdougle added the bug Things that aren't working right in the library. label Nov 22, 2024
@patrick-mcdougle
Copy link
Contributor Author

In the process of making a PR.

@claviska
Copy link
Member

Thanks for the issue, and for taking a stab at a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants