From 31f2600816a117b92b5db919330c790867321dfa Mon Sep 17 00:00:00 2001 From: Alessandro Date: Mon, 25 Mar 2024 16:22:28 +0100 Subject: [PATCH] fix(carousel): synchronize slides after scroll (#1923) * fix(carousel): synchronize slides after scroll * chore: leftovers --- src/components/carousel/carousel.component.ts | 98 +++++++++---------- src/components/carousel/carousel.test.ts | 42 +++++++- 2 files changed, 82 insertions(+), 58 deletions(-) diff --git a/src/components/carousel/carousel.component.ts b/src/components/carousel/carousel.component.ts index 21f47bd7..2a2912bc 100644 --- a/src/components/carousel/carousel.component.ts +++ b/src/components/carousel/carousel.component.ts @@ -92,9 +92,6 @@ export default class SlCarousel extends ShoelaceElement { @state() dragging = false; private autoplayController = new AutoplayController(this, () => this.next()); - private intersectionObserver: IntersectionObserver; // determines which slide is displayed - // A map containing the state of all the slides - private readonly intersectionObserverEntries = new Map(); private readonly localize = new LocalizeController(this); private mutationObserver: MutationObserver; @@ -102,35 +99,10 @@ export default class SlCarousel extends ShoelaceElement { super.connectedCallback(); this.setAttribute('role', 'region'); this.setAttribute('aria-label', this.localize.term('carousel')); - - const intersectionObserver = new IntersectionObserver( - (entries: IntersectionObserverEntry[]) => { - entries.forEach(entry => { - // Store all the entries in a map to be processed when scrolling ends - this.intersectionObserverEntries.set(entry.target, entry); - - const slide = entry.target; - slide.toggleAttribute('inert', !entry.isIntersecting); - slide.classList.toggle('--in-view', entry.isIntersecting); - slide.setAttribute('aria-hidden', entry.isIntersecting ? 'false' : 'true'); - }); - }, - { - root: this, - threshold: 0.6 - } - ); - this.intersectionObserver = intersectionObserver; - - // Store the initial state of each slide - intersectionObserver.takeRecords().forEach(entry => { - this.intersectionObserverEntries.set(entry.target, entry); - }); } disconnectedCallback(): void { super.disconnectedCallback(); - this.intersectionObserver.disconnect(); this.mutationObserver.disconnect(); } @@ -291,26 +263,52 @@ export default class SlCarousel extends ShoelaceElement { this.scrolling = true; } + /** @internal Synchronizes the slides with the IntersectionObserver API. */ + private synchronizeSlides() { + const io = new IntersectionObserver( + entries => { + io.disconnect(); + + for (const entry of entries) { + const slide = entry.target; + slide.toggleAttribute('inert', !entry.isIntersecting); + slide.classList.toggle('--in-view', entry.isIntersecting); + slide.setAttribute('aria-hidden', entry.isIntersecting ? 'false' : 'true'); + } + + const firstIntersecting = entries.find(entry => entry.isIntersecting); + + if (firstIntersecting) { + if (this.loop && firstIntersecting.target.hasAttribute('data-clone')) { + const clonePosition = Number(firstIntersecting.target.getAttribute('data-clone')); + + // Scrolls to the original slide without animating, so the user won't notice that the position has changed + this.goToSlide(clonePosition, 'instant'); + } else { + const slides = this.getSlides(); + + // Update the current index based on the first visible slide + const slideIndex = slides.indexOf(firstIntersecting.target as SlCarouselItem); + // Set the index to the first "snappable" slide + this.activeSlide = Math.ceil(slideIndex / this.slidesPerMove) * this.slidesPerMove; + } + } + }, + { + root: this.scrollContainer, + threshold: 0.6 + } + ); + + this.getSlides({ excludeClones: false }).forEach(slide => { + io.observe(slide); + }); + } + private handleScrollEnd() { if (!this.scrolling || this.dragging) return; - const entries = [...this.intersectionObserverEntries.values()]; - - const firstIntersecting: IntersectionObserverEntry | undefined = entries.find(entry => entry.isIntersecting); - - if (this.loop && firstIntersecting?.target.hasAttribute('data-clone')) { - const clonePosition = Number(firstIntersecting.target.getAttribute('data-clone')); - - // Scrolls to the original slide without animating, so the user won't notice that the position has changed - this.goToSlide(clonePosition, 'instant'); - } else if (firstIntersecting) { - const slides = this.getSlides(); - - // Update the current index based on the first visible slide - const slideIndex = slides.indexOf(firstIntersecting.target as SlCarouselItem); - // Set the index to the first "snappable" slide - this.activeSlide = Math.ceil(slideIndex / this.slidesPerMove) * this.slidesPerMove; - } + this.synchronizeSlides(); this.scrolling = false; } @@ -337,14 +335,8 @@ export default class SlCarousel extends ShoelaceElement { @watch('loop', { waitUntilFirstUpdate: true }) @watch('slidesPerPage', { waitUntilFirstUpdate: true }) initializeSlides() { - const intersectionObserver = this.intersectionObserver; - - this.intersectionObserverEntries.clear(); - // Removes all the cloned elements from the carousel this.getSlides({ excludeClones: false }).forEach((slide, index) => { - intersectionObserver.unobserve(slide); - slide.classList.remove('--in-view'); slide.classList.remove('--is-active'); slide.setAttribute('aria-label', this.localize.term('slideNum', index + 1)); @@ -361,9 +353,7 @@ export default class SlCarousel extends ShoelaceElement { this.createClones(); } - this.getSlides({ excludeClones: false }).forEach(slide => { - intersectionObserver.observe(slide); - }); + this.synchronizeSlides(); // Because the DOM may be changed, restore the scroll position to the active slide this.goToSlide(this.activeSlide, 'auto'); diff --git a/src/components/carousel/carousel.test.ts b/src/components/carousel/carousel.test.ts index 01cfc8d1..18a7f342 100644 --- a/src/components/carousel/carousel.test.ts +++ b/src/components/carousel/carousel.test.ts @@ -1,21 +1,39 @@ import '../../../dist/shoelace.js'; +import { aTimeout, expect, fixture, html, nextFrame, oneEvent, waitUntil } from '@open-wc/testing'; import { clickOnElement, dragElement, moveMouseOnElement } from '../../internal/test.js'; -import { expect, fixture, html, nextFrame, oneEvent } from '@open-wc/testing'; import { map } from 'lit/directives/map.js'; import { range } from 'lit/directives/range.js'; import { resetMouse } from '@web/test-runner-commands'; import sinon from 'sinon'; +import type { SinonStub } from 'sinon'; import type SlCarousel from './carousel.js'; describe('', () => { const sandbox = sinon.createSandbox(); + const ioCallbacks = new Map(); + const intersectionObserverCallbacks = () => { + const callbacks = [...ioCallbacks.values()]; + return waitUntil(() => callbacks.every(callback => callback.called)); + }; + const OriginalIntersectionObserver = globalThis.IntersectionObserver; + + beforeEach(() => { + globalThis.IntersectionObserver = class IntersectionObserverMock extends OriginalIntersectionObserver { + constructor(callback: IntersectionObserverCallback, options?: IntersectionObserverInit) { + const stubCallback = sandbox.stub().callsFake(callback); + + super(stubCallback, options); + + ioCallbacks.set(this, stubCallback); + } + }; + }); afterEach(async () => { await resetMouse(); - }); - - afterEach(() => { sandbox.restore(); + globalThis.IntersectionObserver = OriginalIntersectionObserver; + ioCallbacks.clear(); }); it('should render a carousel with default configuration', async () => { @@ -311,6 +329,7 @@ describe('', () => { await clickOnElement(nextButton); await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); await el.updateComplete; // Assert @@ -337,13 +356,19 @@ describe('', () => { // Act await clickOnElement(nextButton); + await aTimeout(50); await clickOnElement(nextButton); + await aTimeout(50); await clickOnElement(nextButton); + await aTimeout(50); await clickOnElement(nextButton); + await aTimeout(50); await clickOnElement(nextButton); + await aTimeout(50); await clickOnElement(nextButton); await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); await el.updateComplete; // Assert @@ -502,6 +527,7 @@ describe('', () => { el.goToSlide(2, 'auto'); await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); await el.updateComplete; // Act @@ -537,6 +563,9 @@ describe('', () => { // wait scroll to actual item await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); + await el.updateComplete; + // Assert expect(nextButton).to.have.attribute('aria-disabled', 'false'); expect(el.activeSlide).to.be.equal(0); @@ -620,6 +649,8 @@ describe('', () => { // wait scroll to actual item await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); + // Assert expect(previousButton).to.have.attribute('aria-disabled', 'false'); expect(el.activeSlide).to.be.equal(2); @@ -673,6 +704,7 @@ describe('', () => { el.goToSlide(1); await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); await nextFrame(); sandbox.spy(el, 'goToSlide'); @@ -680,6 +712,7 @@ describe('', () => { // Act el.previous(); await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); const containerRect = el.scrollContainer.getBoundingClientRect(); const itemRect = expectedCarouselItem.getBoundingClientRect(); @@ -706,6 +739,7 @@ describe('', () => { // Act el.goToSlide(2); await oneEvent(el.scrollContainer, 'scrollend'); + await intersectionObserverCallbacks(); await el.updateComplete; // Assert