Skip to content

Commit

Permalink
fix(carousel): fix issues with safari (#1748)
Browse files Browse the repository at this point in the history 
* fix(carousel): fix scrollend polyfill

* fix(carousel): refactor mouse dragging

* chore: revert original mouse dragging implementation

* fix: add workaround for safari

* chore: add unit tests

* chore: minor changes

* chore: revert change

* chore: skip test case

* chore: revert changes to docs

* chore: remove leftover
  • Loading branch information
@alenaksu
alenaksu committed Jan 23, 2024
1 parent b1908d7 commit 9f640aa
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 172 deletions.
130 changes: 110 additions & 20 deletions src/components/carousel/carousel.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ShoelaceElement from '../../internal/shoelace-element.js';
import SlIcon from '../icon/icon.component.js';
Expand Down Expand Up @@ -86,8 +86,11 @@ export default class SlCarousel extends ShoelaceElement {
// 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<Element, IntersectionObserverEntry>();
Expand Down Expand Up @@ -216,8 +219,80 @@ export default class SlCarousel extends ShoelaceElement {
}
}

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);
Expand All @@ -226,13 +301,17 @@ export default class SlCarousel extends ShoelaceElement {
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 SlCarouselItem);
// 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 SlCarouselItem {
Expand Down Expand Up @@ -350,11 +429,6 @@ export default class SlCarousel extends ShoelaceElement {
}
}

@watch('mouseDragging')
handleMouseDraggingChange() {
this.scrollController.mouseDragging = this.mouseDragging;
}

/**
* Move the carousel backward by `slides-per-move` slides.
*
Expand All @@ -380,7 +454,7 @@ export default class SlCarousel extends ShoelaceElement {
* @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 });
Expand All @@ -399,18 +473,31 @@ export default class SlCarousel extends ShoelaceElement {
const nextSlideIndex = clamp(index + (loop ? slidesPerPage : 0), 0, slidesWithClones.length - 1);
const nextSlide = slidesWithClones[nextSlideIndex];

this.scrollToSlide(nextSlide, prefersReducedMotion() ? 'auto' : behavior);
}

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

scrollContainer.scrollTo({
left: nextSlideRect.left - scrollContainerRect.left + scrollContainer.scrollLeft,
top: nextSlideRect.top - scrollContainerRect.top + scrollContainer.scrollTop,
behavior: prefersReducedMotion() ? 'auto' : behavior
});
const nextLeft = nextSlideRect.left - scrollContainerRect.left;
const nextTop = nextSlideRect.top - scrollContainerRect.top;

// If the slide is already in view, don't need to scroll
if (nextLeft !== scrollContainer.scrollLeft || nextTop !== scrollContainer.scrollTop) {
scrollContainer.scrollTo({
left: nextLeft + scrollContainer.scrollLeft,
top: nextTop + scrollContainer.scrollTop,
behavior
});

await waitForEvent(scrollContainer, 'scrollend');
}
}

render() {
const { scrollController, slidesPerMove } = this;
const { slidesPerMove, scrolling } = this;
const pagesCount = this.getPageCount();
const currentPage = this.getCurrentPage();
const prevEnabled = this.canScrollPrev();
Expand All @@ -425,13 +512,16 @@ export default class SlCarousel extends ShoelaceElement {
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}
>
<slot></slot>
Expand Down
4 changes: 1 addition & 3 deletions src/components/carousel/carousel.styles.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ export default css`
overflow-x: hidden;
}
.carousel__slides--dragging,
.carousel__slides--dropping {
scroll-snap-type: unset;
.carousel__slides--dragging {
}
:host([vertical]) ::slotted(sl-carousel-item) {
Expand Down
54 changes: 53 additions & 1 deletion src/components/carousel/carousel.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import '../../../dist/shoelace.js';
import { clickOnElement } from '../../internal/test.js';
import { clickOnElement, dragElement, moveMouseOnElement } from '../../internal/test.js';
import { expect, fixture, html, 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 SlCarousel from './carousel.js';

describe('<sl-carousel>', () => {
afterEach(async () => {
await resetMouse();
});

it('should render a carousel with default configuration', async () => {
// Arrange
const el = await fixture(html`
Expand Down Expand Up @@ -409,6 +414,53 @@ describe('<sl-carousel>', () => {
});
});

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<SlCarousel>(html`
<sl-carousel mouse-dragging>
<sl-carousel-item>Node 1</sl-carousel-item>
<sl-carousel-item>Node 2</sl-carousel-item>
<sl-carousel-item>Node 3</sl-carousel-item>
</sl-carousel>
`);

// 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<SlCarousel>(html`
<sl-carousel mouse-dragging>
<sl-carousel-item><button>click me</button></sl-carousel-item>
<sl-carousel-item>Node 2</sl-carousel-item>
<sl-carousel-item>Node 3</sl-carousel-item>
</sl-carousel>
`);
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 () => {
Expand Down

0 comments on commit 9f640aa

Please sign in to comment.