Skip to content

Commit

Permalink
fix(carousel): remove check for scrolling (#1862)
Browse files Browse the repository at this point in the history
  • Loading branch information
alenaksu authored Feb 9, 2024
1 parent dafb35c commit 7e38e93
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 33 deletions.
17 changes: 6 additions & 11 deletions src/components/carousel/carousel.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,24 +476,19 @@ export default class SlCarousel extends ShoelaceElement {
this.scrollToSlide(nextSlide, prefersReducedMotion() ? 'auto' : behavior);
}

private async scrollToSlide(slide: HTMLElement, behavior: ScrollBehavior = 'smooth') {
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 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');
}
scrollContainer.scrollTo({
left: nextLeft + scrollContainer.scrollLeft,
top: nextTop + scrollContainer.scrollTop,
behavior
});
}

render() {
Expand Down
67 changes: 45 additions & 22 deletions src/components/carousel/carousel.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
import '../../../dist/shoelace.js';
import { clickOnElement, dragElement, moveMouseOnElement } from '../../internal/test.js';
import { expect, fixture, html, oneEvent } from '@open-wc/testing';
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 SlCarousel from './carousel.js';

describe('<sl-carousel>', () => {
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`
Expand All @@ -34,15 +40,11 @@ describe('<sl-carousel>', () => {
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<SlCarousel>(html`
Expand All @@ -52,7 +54,7 @@ describe('<sl-carousel>', () => {
<sl-carousel-item>Node 3</sl-carousel-item>
</sl-carousel>
`);
sinon.stub(el, 'next');
sandbox.stub(el, 'next');

await el.updateComplete;

Expand All @@ -73,7 +75,7 @@ describe('<sl-carousel>', () => {
<sl-carousel-item>Node 3</sl-carousel-item>
</sl-carousel>
`);
sinon.stub(el, 'next');
sandbox.stub(el, 'next');

await el.updateComplete;

Expand All @@ -96,7 +98,7 @@ describe('<sl-carousel>', () => {
<sl-carousel-item>Node 3</sl-carousel-item>
</sl-carousel>
`);
sinon.stub(el, 'next');
sandbox.stub(el, 'next');

await el.updateComplete;

Expand Down Expand Up @@ -183,7 +185,7 @@ describe('<sl-carousel>', () => {
<sl-carousel-item>Node 3</sl-carousel-item>
</sl-carousel>
`);
sinon.stub(el, 'goToSlide');
sandbox.stub(el, 'goToSlide');
await el.updateComplete;

// Act
Expand Down Expand Up @@ -473,7 +475,7 @@ describe('<sl-carousel>', () => {
</sl-carousel>
`);
const nextButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--next')!;
sinon.stub(el, 'next');
sandbox.stub(el, 'next');

await el.updateComplete;

Expand All @@ -496,7 +498,7 @@ describe('<sl-carousel>', () => {
</sl-carousel>
`);
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');
Expand Down Expand Up @@ -560,7 +562,7 @@ describe('<sl-carousel>', () => {
await el.updateComplete;

const previousButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--previous')!;
sinon.stub(el, 'previous');
sandbox.stub(el, 'previous');

await el.updateComplete;

Expand All @@ -584,7 +586,7 @@ describe('<sl-carousel>', () => {
`);

const previousButton: HTMLElement = el.shadowRoot!.querySelector('.carousel__navigation-button--previous')!;
sinon.stub(el, 'previous');
sandbox.stub(el, 'previous');
await el.updateComplete;

// Act
Expand Down Expand Up @@ -632,39 +634,60 @@ describe('<sl-carousel>', () => {
it('should scroll the carousel to the next slide', async () => {
// Arrange
const el = await fixture<SlCarousel>(html`
<sl-carousel slides-per-page="2" slides-per-move="2">
<sl-carousel>
<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>
`);
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);
});
});

describe('#previous', () => {
it('should scroll the carousel to the previous slide', async () => {
// Arrange
const el = await fixture<SlCarousel>(html`
<sl-carousel slides-per-page="2" slides-per-move="2">
<sl-carousel>
<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>
`);
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);
});
});

Expand Down

0 comments on commit 7e38e93

Please sign in to comment.