Skip to content

Commit

Permalink
Fix rendering performance bottleneck in carousel (#2281)
Browse files Browse the repository at this point in the history
* Fix rendering performance bottleneck in carousel

* Remove check before using RAF

* Optimistically mark the slide change as pending until the rAF callback runs
  • Loading branch information
patrick-mcdougle authored Dec 4, 2024
1 parent ac64141 commit 97fcab8
Showing 1 changed file with 24 additions and 15 deletions.
39 changes: 24 additions & 15 deletions src/components/carousel/carousel.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,21 +506,30 @@ export default class SlCarousel extends ShoelaceElement {
}

private scrollToSlide(slide: HTMLElement, behavior: ScrollBehavior = 'smooth') {
const scrollContainer = this.scrollContainer;
const scrollContainerRect = scrollContainer.getBoundingClientRect();
const nextSlideRect = slide.getBoundingClientRect();

const nextLeft = nextSlideRect.left - scrollContainerRect.left;
const nextTop = nextSlideRect.top - scrollContainerRect.top;

if (nextLeft || nextTop) {
this.pendingSlideChange = true;
scrollContainer.scrollTo({
left: nextLeft + scrollContainer.scrollLeft,
top: nextTop + scrollContainer.scrollTop,
behavior
});
}
// Since the geometry doesn't happen until rAF, we don't know if we'll be scrolling or not...
// It's best to assume that we will and cleanup in the else case below if we didn't need to
this.pendingSlideChange = true;
window.requestAnimationFrame(() => {
const scrollContainer = this.scrollContainer;
const scrollContainerRect = scrollContainer.getBoundingClientRect();
const nextSlideRect = slide.getBoundingClientRect();

const nextLeft = nextSlideRect.left - scrollContainerRect.left;
const nextTop = nextSlideRect.top - scrollContainerRect.top;

if (nextLeft || nextTop) {
// This is here just in case someone set it back to false
// between rAF being requested and the callback actually running
this.pendingSlideChange = true;
scrollContainer.scrollTo({
left: nextLeft + scrollContainer.scrollLeft,
top: nextTop + scrollContainer.scrollTop,
behavior
});
} else {
this.pendingSlideChange = false;
}
});
}

render() {
Expand Down

0 comments on commit 97fcab8

Please sign in to comment.