From cb1c423aea8717aae34be32d43cea26ac4f7e8f2 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Fri, 9 Feb 2024 10:05:14 -0500 Subject: [PATCH] backport 1862 --- docs/src/content/docs/resources/changelog.md | 1 + src/components/carousel/carousel.component.ts | 142 +++++++++++++----- src/components/carousel/carousel.test.ts | 123 ++++++++++++--- 3 files changed, 208 insertions(+), 58 deletions(-) diff --git a/docs/src/content/docs/resources/changelog.md b/docs/src/content/docs/resources/changelog.md index 552bfc098..30b6577f3 100644 --- a/docs/src/content/docs/resources/changelog.md +++ b/docs/src/content/docs/resources/changelog.md @@ -27,6 +27,7 @@ New versions of Web Awesome are released as-needed and generally occur when a cr - Added help text to `` [#1860] - Added help text to `` [#1800] - Fixed a bug in `` that caused HTML tags to be included in `getTextLabel()` +- Fixed a bug in `` that caused slides to not switch correctly [#1862] ## 2.13.1 diff --git a/src/components/carousel/carousel.component.ts b/src/components/carousel/carousel.component.ts index 56748dcc7..4a8671649 100644 --- a/src/components/carousel/carousel.component.ts +++ b/src/components/carousel/carousel.component.ts @@ -3,13 +3,13 @@ import '../../internal/scrollend-polyfill.js'; import { AutoplayController } from './autoplay-controller.js'; import { clamp } from '../../internal/math.js'; import { classMap } from 'lit/directives/class-map.js'; +import { eventOptions, property, query, state } from 'lit/decorators.js'; import { html } from 'lit'; import { LocalizeController } from '../../utilities/localize.js'; import { map } from 'lit/directives/map.js'; import { prefersReducedMotion } from '../../internal/animate.js'; -import { property, query, state } from 'lit/decorators.js'; import { range } from 'lit/directives/range.js'; -import { ScrollController } from './scroll-controller.js'; +import { waitForEvent } from '../../internal/event.js'; import { watch } from '../../internal/watch.js'; import styles from './carousel.styles.js'; import WaIcon from '../icon/icon.component.js'; @@ -41,15 +41,12 @@ import type WaCarouselItem from '../carousel-item/carousel-item.component.js'; * @csspart navigation-button--previous - Applied to the previous button. * @csspart navigation-button--next - Applied to the next button. * + * @cssproperty --slide-gap - The space between each slide. * @cssproperty [--aspect-ratio=16/9] - The aspect ratio of each slide. - * @cssproperty --navigation-color - The color of the navigation buttons. - * @cssproperty --pagination-color-activated - The color of the pagination dot for the current item. - * @cssproperty --pagination-color-resting - The color of the pagination dots for inactive items. * @cssproperty --scroll-hint - The amount of padding to apply to the scroll area, allowing adjacent slides to become * partially visible as a scroll hint. - * @cssproperty --slide-gap - The space between each slide. */ -export default class WaCarousel extends WebAwesomeElement { +export default class SlCarousel extends WebAwesomeElement { static styles: CSSResultGroup = styles; static dependencies = { 'wa-icon': WaIcon }; @@ -89,8 +86,11 @@ export default class WaCarousel extends WebAwesomeElement { // The index of the active slide @state() activeSlide = 0; + @state() scrolling = false; + + @state() dragging = false; + private autoplayController = new AutoplayController(this, () => this.next()); - private scrollController = new ScrollController(this); private intersectionObserver: IntersectionObserver; // determines which slide is displayed // A map containing the state of all the slides private readonly intersectionObserverEntries = new Map(); @@ -142,7 +142,7 @@ export default class WaCarousel extends WebAwesomeElement { }); } - protected willUpdate(changedProperties: PropertyValueMap | Map): void { + protected willUpdate(changedProperties: PropertyValueMap | Map): void { // Ensure the slidesPerMove is never higher than the slidesPerPage if (changedProperties.has('slidesPerMove') || changedProperties.has('slidesPerPage')) { this.slidesPerMove = Math.min(this.slidesPerMove, this.slidesPerPage); @@ -219,8 +219,80 @@ export default class WaCarousel extends WebAwesomeElement { } } + private handleMouseDragStart(event: PointerEvent) { + const canDrag = this.mouseDragging && event.button === 0; + if (canDrag) { + event.preventDefault(); + + document.addEventListener('pointermove', this.handleMouseDrag, { capture: true, passive: true }); + document.addEventListener('pointerup', this.handleMouseDragEnd, { capture: true, once: true }); + } + } + + private handleMouseDrag = (event: PointerEvent) => { + if (!this.dragging) { + // Start dragging if it hasn't yet + this.scrollContainer.style.setProperty('scroll-snap-type', 'none'); + this.dragging = true; + } + + this.scrollContainer.scrollBy({ + left: -event.movementX, + top: -event.movementY, + behavior: 'instant' + }); + }; + + private handleMouseDragEnd = () => { + const scrollContainer = this.scrollContainer; + + document.removeEventListener('pointermove', this.handleMouseDrag, { capture: true }); + + // get the current scroll position + const startLeft = scrollContainer.scrollLeft; + const startTop = scrollContainer.scrollTop; + + // remove the scroll-snap-type property so that the browser will snap the slide to the correct position + scrollContainer.style.removeProperty('scroll-snap-type'); + + // fix(safari): forcing a style recalculation doesn't seem to immediately update the scroll + // position in Safari. Setting "overflow" to "hidden" should force this behavior. + scrollContainer.style.setProperty('overflow', 'hidden'); + + // get the final scroll position to the slide snapped by the browser + const finalLeft = scrollContainer.scrollLeft; + const finalTop = scrollContainer.scrollTop; + + // restore the scroll position to the original one, so that it can be smoothly animated if needed + scrollContainer.style.removeProperty('overflow'); + scrollContainer.style.setProperty('scroll-snap-type', 'none'); + scrollContainer.scrollTo({ left: startLeft, top: startTop, behavior: 'instant' }); + + requestAnimationFrame(async () => { + if (startLeft !== finalLeft || startTop !== finalTop) { + scrollContainer.scrollTo({ + left: finalLeft, + top: finalTop, + behavior: prefersReducedMotion() ? 'auto' : 'smooth' + }); + await waitForEvent(scrollContainer, 'scrollend'); + } + + scrollContainer.style.removeProperty('scroll-snap-type'); + + this.dragging = false; + this.handleScrollEnd(); + }); + }; + + @eventOptions({ passive: true }) + private handleScroll() { + this.scrolling = true; + } + private handleScrollEnd() { - const slides = this.getSlides(); + if (!this.scrolling || this.dragging) return; + const entries = [...this.intersectionObserverEntries.values()]; const firstIntersecting: IntersectionObserverEntry | undefined = entries.find(entry => entry.isIntersecting); @@ -229,13 +301,17 @@ export default class WaCarousel extends WebAwesomeElement { 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, 'auto'); + 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 WaCarouselItem); // Set the index to the first "snappable" slide this.activeSlide = Math.ceil(slideIndex / this.slidesPerMove) * this.slidesPerMove; } + + this.scrolling = false; } private isCarouselItem(node: Node): node is WaCarouselItem { @@ -353,11 +429,6 @@ export default class WaCarousel extends WebAwesomeElement { } } - @watch('mouseDragging') - handleMouseDraggingChange() { - this.scrollController.mouseDragging = this.mouseDragging; - } - /** * Move the carousel backward by `slides-per-move` slides. * @@ -383,7 +454,7 @@ export default class WaCarousel extends WebAwesomeElement { * @param behavior - The behavior used for scrolling. */ goToSlide(index: number, behavior: ScrollBehavior = 'smooth') { - const { slidesPerPage, loop, scrollContainer } = this; + const { slidesPerPage, loop } = this; const slides = this.getSlides(); const slidesWithClones = this.getSlides({ excludeClones: false }); @@ -402,18 +473,26 @@ export default class WaCarousel extends WebAwesomeElement { const nextSlideIndex = clamp(index + (loop ? slidesPerPage : 0), 0, slidesWithClones.length - 1); const nextSlide = slidesWithClones[nextSlideIndex]; + this.scrollToSlide(nextSlide, prefersReducedMotion() ? 'auto' : behavior); + } + + private scrollToSlide(slide: HTMLElement, behavior: ScrollBehavior = 'smooth') { + const scrollContainer = this.scrollContainer; const scrollContainerRect = scrollContainer.getBoundingClientRect(); - const nextSlideRect = nextSlide.getBoundingClientRect(); + const nextSlideRect = slide.getBoundingClientRect(); + + const nextLeft = nextSlideRect.left - scrollContainerRect.left; + const nextTop = nextSlideRect.top - scrollContainerRect.top; scrollContainer.scrollTo({ - left: nextSlideRect.left - scrollContainerRect.left + scrollContainer.scrollLeft, - top: nextSlideRect.top - scrollContainerRect.top + scrollContainer.scrollTop, - behavior: prefersReducedMotion() ? 'auto' : behavior + left: nextLeft + scrollContainer.scrollLeft, + top: nextTop + scrollContainer.scrollTop, + behavior }); } render() { - const { scrollController, slidesPerMove } = this; + const { slidesPerMove, scrolling } = this; const pagesCount = this.getPageCount(); const currentPage = this.getCurrentPage(); const prevEnabled = this.canScrollPrev(); @@ -428,13 +507,16 @@ export default class WaCarousel extends WebAwesomeElement { class="${classMap({ carousel__slides: true, 'carousel__slides--horizontal': this.orientation === 'horizontal', - 'carousel__slides--vertical': this.orientation === 'vertical' + 'carousel__slides--vertical': this.orientation === 'vertical', + 'carousel__slides--dragging': this.dragging })}" style="--slides-per-page: ${this.slidesPerPage};" - aria-busy="${scrollController.scrolling ? 'true' : 'false'}" + aria-busy="${scrolling ? 'true' : 'false'}" aria-atomic="true" tabindex="0" @keydown=${this.handleKeyDown} + @mousedown="${this.handleMouseDragStart}" + @scroll="${this.handleScroll}" @scrollend=${this.handleScrollEnd} > @@ -456,11 +538,7 @@ export default class WaCarousel extends WebAwesomeElement { @click=${prevEnabled ? () => this.previous() : null} > - + @@ -477,11 +555,7 @@ export default class WaCarousel extends WebAwesomeElement { @click=${nextEnabled ? () => this.next() : null} > - + diff --git a/src/components/carousel/carousel.test.ts b/src/components/carousel/carousel.test.ts index 4641beb99..5323767fa 100644 --- a/src/components/carousel/carousel.test.ts +++ b/src/components/carousel/carousel.test.ts @@ -1,12 +1,23 @@ -import '../../../dist/webawesome.js'; -import { clickOnElement } from '../../internal/test.js'; -import { expect, fixture, html, oneEvent } from '@open-wc/testing'; +import '../../../dist/shoelace.js'; +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 WaCarousel from './carousel.js'; describe('', () => { + const sandbox = sinon.createSandbox(); + + afterEach(async () => { + await resetMouse(); + }); + + afterEach(() => { + sandbox.restore(); + }); + it('should render a carousel with default configuration', async () => { // Arrange const el = await fixture(html` @@ -29,15 +40,11 @@ describe('', () => { let clock: sinon.SinonFakeTimers; beforeEach(() => { - clock = sinon.useFakeTimers({ + clock = sandbox.useFakeTimers({ now: new Date() }); }); - afterEach(() => { - clock.restore(); - }); - it('should scroll forwards every `autoplay-interval` milliseconds', async () => { // Arrange const el = await fixture(html` @@ -47,7 +54,7 @@ describe('', () => { Node 3 `); - sinon.stub(el, 'next'); + sandbox.stub(el, 'next'); await el.updateComplete; @@ -68,7 +75,7 @@ describe('', () => { Node 3 `); - sinon.stub(el, 'next'); + sandbox.stub(el, 'next'); await el.updateComplete; @@ -91,7 +98,7 @@ describe('', () => { Node 3 `); - sinon.stub(el, 'next'); + sandbox.stub(el, 'next'); await el.updateComplete; @@ -178,7 +185,7 @@ describe('', () => { Node 3 `); - sinon.stub(el, 'goToSlide'); + sandbox.stub(el, 'goToSlide'); await el.updateComplete; // Act @@ -409,6 +416,53 @@ describe('', () => { }); }); + describe('when `mouse-dragging` attribute is provided', () => { + // TODO(alenaksu): skipping because failing in webkit, PointerEvent.movementX and PointerEvent.movementY seem to return incorrect values + it.skip('should be possible to drag the carousel using the mouse', async () => { + // Arrange + const el = await fixture(html` + + Node 1 + Node 2 + Node 3 + + `); + + // Act + await dragElement(el, -Math.round(el.offsetWidth * 0.75)); + await oneEvent(el.scrollContainer, 'scrollend'); + await dragElement(el, -Math.round(el.offsetWidth * 0.75)); + await oneEvent(el.scrollContainer, 'scrollend'); + + await el.updateComplete; + + // Assert + expect(el.activeSlide).to.be.equal(2); + }); + + it('should be possible to interact with clickable elements', async () => { + // Arrange + const el = await fixture(html` + + + Node 2 + Node 3 + + `); + const button = el.querySelector('button')!; + + const clickSpy = sinon.spy(); + button.addEventListener('click', clickSpy); + + // Act + await moveMouseOnElement(button); + await clickOnElement(button); + + // Assert + expect(clickSpy).to.have.been.called; + }); + }); + describe('Navigation controls', () => { describe('when the user clicks the next button', () => { it('should scroll to the next slide', async () => { @@ -421,7 +475,7 @@ describe('', () => { `); const nextButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--next')!; - sinon.stub(el, 'next'); + sandbox.stub(el, 'next'); await el.updateComplete; @@ -444,7 +498,7 @@ describe('', () => { `); const nextButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--next')!; - sinon.stub(el, 'next'); + sandbox.stub(el, 'next'); el.goToSlide(2, 'auto'); await oneEvent(el.scrollContainer, 'scrollend'); @@ -508,7 +562,7 @@ describe('', () => { await el.updateComplete; const previousButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--previous')!; - sinon.stub(el, 'previous'); + sandbox.stub(el, 'previous'); await el.updateComplete; @@ -532,7 +586,7 @@ describe('', () => { `); const previousButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--previous')!; - sinon.stub(el, 'previous'); + sandbox.stub(el, 'previous'); await el.updateComplete; // Act @@ -580,19 +634,27 @@ describe('', () => { it('should scroll the carousel to the next slide', async () => { // Arrange const el = await fixture(html` - + Node 1 Node 2 Node 3 `); - sinon.stub(el, 'goToSlide'); - await el.updateComplete; + sandbox.spy(el, 'goToSlide'); + const expectedCarouselItem: HTMLElement = el.querySelector('sl-carousel-item:nth-child(2)')!; // Act el.next(); + await oneEvent(el.scrollContainer, 'scrollend'); + await el.updateComplete; - expect(el.goToSlide).to.have.been.calledWith(2); + const containerRect = el.scrollContainer.getBoundingClientRect(); + const itemRect = expectedCarouselItem.getBoundingClientRect(); + + // Assert + expect(el.goToSlide).to.have.been.calledWith(1); + expect(itemRect.top).to.be.equal(containerRect.top); + expect(itemRect.left).to.be.equal(containerRect.left); }); }); @@ -600,19 +662,32 @@ describe('', () => { it('should scroll the carousel to the previous slide', async () => { // Arrange const el = await fixture(html` - + Node 1 Node 2 Node 3 `); - sinon.stub(el, 'goToSlide'); - await el.updateComplete; + const expectedCarouselItem: HTMLElement = el.querySelector('sl-carousel-item:nth-child(1)')!; + + el.goToSlide(1); + + await oneEvent(el.scrollContainer, 'scrollend'); + await nextFrame(); + + sandbox.spy(el, 'goToSlide'); // Act el.previous(); + await oneEvent(el.scrollContainer, 'scrollend'); - expect(el.goToSlide).to.have.been.calledWith(-2); + const containerRect = el.scrollContainer.getBoundingClientRect(); + const itemRect = expectedCarouselItem.getBoundingClientRect(); + + // Assert + expect(el.goToSlide).to.have.been.calledWith(0); + expect(itemRect.top).to.be.equal(containerRect.top); + expect(itemRect.left).to.be.equal(containerRect.left); }); });