From 7321b40b525a66dfa5cb73a283b8dc1016134858 Mon Sep 17 00:00:00 2001 From: Ismail9k Date: Thu, 26 Dec 2024 12:24:02 +0300 Subject: [PATCH 1/2] fix: Carousel component's slide cloning logic and add tests for it --- src/components/Carousel/Carousel.ts | 12 ++-- tests/integration/carousel.spec.ts | 101 ++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 13 deletions(-) diff --git a/src/components/Carousel/Carousel.ts b/src/components/Carousel/Carousel.ts index 9da0c77..4c2afd1 100644 --- a/src/components/Carousel/Carousel.ts +++ b/src/components/Carousel/Carousel.ts @@ -661,15 +661,13 @@ export const Carousel = defineComponent({ } const slidesToClone = Math.ceil(config.itemsToShow) - const before = Math.min(slidesToClone, activeSlideIndex.value) - const after = Math.min( - slidesToClone, - slidesCount.value - activeSlideIndex.value - 1 - ) return { - before: Math.max(slidesToClone, before), - after: Math.max(slidesToClone, after), + before: Math.max(slidesToClone, slidesToClone - activeSlideIndex.value), + after: Math.max( + slidesToClone, + slidesToClone - (slidesCount.value - (activeSlideIndex.value + 1)) + ), } }) diff --git a/tests/integration/carousel.spec.ts b/tests/integration/carousel.spec.ts index c8a082a..83564a4 100644 --- a/tests/integration/carousel.spec.ts +++ b/tests/integration/carousel.spec.ts @@ -16,8 +16,8 @@ describe('Carousel.ts', () => { props: { slideNum: 5, modelValue: 0, - 'onUpdate:modelValue': (e) => wrapper.setProps({ modelValue: e }) - } + 'onUpdate:modelValue': (e) => wrapper.setProps({ modelValue: e }), + }, }) }) @@ -69,7 +69,7 @@ describe('Carousel.ts', () => { expect(wrapper.props('modelValue')).toBe(1) await triggerKeyEvent('ArrowDown') expect(wrapper.props('modelValue')).toBe(1) - await wrapper.setProps({dir: 'ttb'}) + await wrapper.setProps({ dir: 'ttb' }) await triggerKeyEvent('ArrowDown') expect(wrapper.props('modelValue')).toBe(2) await triggerKeyEvent('ArrowUp') @@ -79,7 +79,7 @@ describe('Carousel.ts', () => { await triggerKeyEvent('ArrowLeft') expect(wrapper.props('modelValue')).toBe(1) - await wrapper.setProps({dir: 'btt'}) + await wrapper.setProps({ dir: 'btt' }) await triggerKeyEvent('ArrowDown') expect(wrapper.props('modelValue')).toBe(0) await triggerKeyEvent('ArrowUp') @@ -89,7 +89,7 @@ describe('Carousel.ts', () => { await triggerKeyEvent('ArrowLeft') expect(wrapper.props('modelValue')).toBe(1) - await wrapper.setProps({dir: 'rtl'}) + await wrapper.setProps({ dir: 'rtl' }) await triggerKeyEvent('ArrowDown') expect(wrapper.props('modelValue')).toBe(1) await triggerKeyEvent('ArrowUp') @@ -155,6 +155,95 @@ describe('Wrap around Carousel.ts', () => { }) }) +describe('Carousel Slide Cloning', () => { + it('should not clone slides when wrapAround is false', async () => { + const wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: false, + itemsToShow: 3, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + const clonedSlides = wrapper.findAll('.carousel__slide--clone') + expect(slides.length).toBe(5) // Only original slides, no clones + expect(clonedSlides.length).toBe(0) + }) + + it('should clone correct number of slides with wrapAround enabled', async () => { + const wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 3, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (5) + cloned before (3) + cloned after (3) + expect(slides.length).toBe(11) + }) + + it('should adjust clone count based on activeSlideIndex', async () => { + const wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 2, + modelValue: 0, + }, + }) + + // Initial state (at index 0) + let slides = wrapper.findAll('.carousel__slide') + const initialCount = slides.length + + // Move to middle slide + await wrapper.setProps({ modelValue: 2 }) + slides = wrapper.findAll('.carousel__slide') + expect(slides.length).toBe(initialCount) // Should maintain same total count + + // Move to last slide + await wrapper.setProps({ modelValue: 5 }) + slides = wrapper.findAll('.carousel__slide') + // Original slides (5) + cloned before (2) + cloned after (3) + expect(slides.length).toBe(10) // Should maintain same total count + }) + + it('should handle decimal itemsToShow values', async () => { + const wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 2.5, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (5) + cloned before (3) + cloned after (3) + // Math.ceil(2.5) = 3 slides should be cloned on each side + expect(slides.length).toBe(11) + }) + + it('should handle edge case with single slide', async () => { + const wrapper = await mount(App, { + props: { + slideNum: 1, + wrapAround: true, + itemsToShow: 1, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slide (1) + cloned before (1) + cloned after (1) + expect(slides.length).toBe(3) + }) +}) + describe('SSR Carousel', () => { const consoleMock = vi.spyOn(console, 'warn').mockImplementation(() => undefined) @@ -164,7 +253,7 @@ describe('SSR Carousel', () => { const renderSSR = async >( component: T, - props: P = {} + props: P = {} as P ) => { const comp = { render() { From 168558ab33655cb148afd96c131eb314726bf207 Mon Sep 17 00:00:00 2001 From: Ismail9k Date: Thu, 26 Dec 2024 13:59:13 +0300 Subject: [PATCH 2/2] feat: enhance Carousel component with cloned slide offset adjustments and refactor index calculations --- src/components/Carousel/Carousel.css | 12 ++ src/components/Carousel/Carousel.ts | 65 +++--- src/utils/getScrolledIndex.ts | 35 ++-- .../__snapshots__/carousel.spec.ts.snap | 17 +- tests/integration/carousel.spec.ts | 191 ++++++++++-------- 5 files changed, 172 insertions(+), 148 deletions(-) diff --git a/src/components/Carousel/Carousel.css b/src/components/Carousel/Carousel.css index 7d1d87d..5735751 100644 --- a/src/components/Carousel/Carousel.css +++ b/src/components/Carousel/Carousel.css @@ -49,3 +49,15 @@ flex-direction: column-reverse; } } + +.carousel.is-vertical { + .carousel__slide--clone:first-child { + margin-block-start: var(--vc-trk-cloned-offset); + } +} + +.carousel:not(.is-vertical) { + .carousel__slide--clone:first-child { + margin-inline-start: var(--vc-trk-cloned-offset); + } +} diff --git a/src/components/Carousel/Carousel.ts b/src/components/Carousel/Carousel.ts index 4c2afd1..e176f87 100644 --- a/src/components/Carousel/Carousel.ts +++ b/src/components/Carousel/Carousel.ts @@ -402,7 +402,13 @@ export const Carousel = defineComponent({ effectiveSlideSize: effectiveSlideSize.value, }) - activeSlideIndex.value = currentSlideIndex.value + draggedSlides + activeSlideIndex.value = config.wrapAround + ? currentSlideIndex.value + draggedSlides + : getNumberInRange({ + val: currentSlideIndex.value + draggedSlides, + max: maxSlideIndex.value, + min: minSlideIndex.value, + }) // Emit a drag event for further customization if needed emit('drag', { deltaX: dragged.x, deltaY: dragged.y }) @@ -473,14 +479,22 @@ export const Carousel = defineComponent({ } let targetIndex = slideIndex + let mappedIndex = slideIndex + + prevSlideIndex.value = currentSlideIndex.value - // Calculate shortest path in wrap-around mode if (!config.wrapAround) { targetIndex = getNumberInRange({ val: targetIndex, max: maxSlideIndex.value, min: minSlideIndex.value, }) + } else { + mappedIndex = mapNumberToRange({ + val: targetIndex, + max: maxSlideIndex.value, + min: 0, + }) } emit('slide-start', { @@ -491,29 +505,20 @@ export const Carousel = defineComponent({ }) stopAutoplay() - prevSlideIndex.value = currentSlideIndex.value - - const mappedNumber = config.wrapAround - ? mapNumberToRange({ - val: targetIndex, - max: maxSlideIndex.value, - min: 0, - }) - : targetIndex - isSliding.value = true - currentSlideIndex.value = targetIndex - if (mappedNumber !== targetIndex) { + currentSlideIndex.value = targetIndex + if (mappedIndex !== targetIndex) { modelWatcher.pause() } - emit('update:modelValue', mappedNumber) + emit('update:modelValue', mappedIndex) - transitionTimer = setTimeout((): void => { + const transitionCallback = (): void => { if (config.wrapAround) { - if (mappedNumber !== targetIndex) { + if (mappedIndex !== targetIndex) { modelWatcher.resume() - currentSlideIndex.value = mappedNumber + + currentSlideIndex.value = mappedIndex emit('loop', { currentSlideIndex: currentSlideIndex.value, slidingToIndex: slideIndex, @@ -529,7 +534,9 @@ export const Carousel = defineComponent({ isSliding.value = false resetAutoplay() - }, config.transition) + } + + transitionTimer = setTimeout(transitionCallback, config.transition) } function next(skipTransition = false): void { @@ -660,17 +667,19 @@ export const Carousel = defineComponent({ return { before: 0, after: 0 } } - const slidesToClone = Math.ceil(config.itemsToShow) + const slidesToClone = Math.ceil(config.itemsToShow + (config.itemsToScroll - 1)) + const before = slidesToClone - activeSlideIndex.value + const after = slidesToClone - (slidesCount.value - (activeSlideIndex.value + 1)) return { - before: Math.max(slidesToClone, slidesToClone - activeSlideIndex.value), - after: Math.max( - slidesToClone, - slidesToClone - (slidesCount.value - (activeSlideIndex.value + 1)) - ), + before: Math.max(0, before), + after: Math.max(0, after), } }) + const clonedSlidesOffset = computed( + () => clonedSlidesCount.value.before * effectiveSlideSize.value * -1 + ) const trackTransform: ComputedRef = computed(() => { const directionMultiplier = isReversed.value ? 1 : -1 const translateAxis = isVertical.value ? 'Y' : 'X' @@ -679,13 +688,10 @@ export const Carousel = defineComponent({ const scrolledOffset = scrolledIndex.value * effectiveSlideSize.value * directionMultiplier - const clonedSlidesOffset = - clonedSlidesCount.value.before * effectiveSlideSize.value * -1 - // Include user drag interaction offset const dragOffset = isVertical.value ? dragged.y : dragged.x - const totalOffset = scrolledOffset + clonedSlidesOffset + dragOffset + const totalOffset = scrolledOffset + dragOffset return `translate${translateAxis}(${totalOffset}px)` }) @@ -695,6 +701,7 @@ export const Carousel = defineComponent({ 'transition-duration': isSliding.value ? `${config.transition}ms` : undefined, gap: config.gap > 0 ? `${config.gap}px` : undefined, '--vc-trk-height': trackHeight.value, + '--vc-trk-cloned-offset': `${clonedSlidesOffset.value}px`, })) return () => { diff --git a/src/utils/getScrolledIndex.ts b/src/utils/getScrolledIndex.ts index d005866..aca1b35 100644 --- a/src/utils/getScrolledIndex.ts +++ b/src/utils/getScrolledIndex.ts @@ -1,7 +1,6 @@ import { CarouselConfig } from '@/shared' import { getNumberInRange } from './getNumberInRange' -import { mapNumberToRange } from './mapNumberToRange' type GetScrolledIndexArgs = { config: Pick @@ -12,11 +11,16 @@ type GetScrolledIndexArgs = { export const calculateOffset = (snapAlign: string, itemsToShow: number): number => { switch (snapAlign) { default: - case 'start': return 0 - case 'center': return (itemsToShow - 1) / 2 - case 'center-odd': return (itemsToShow - 1) / 2 - case 'center-even': return (itemsToShow - 2) / 2 - case 'end': return itemsToShow - 1 + case 'start': + return 0 + case 'center': + return (itemsToShow - 1) / 2 + case 'center-odd': + return (itemsToShow - 1) / 2 + case 'center-even': + return (itemsToShow - 2) / 2 + case 'end': + return itemsToShow - 1 } } @@ -31,17 +35,12 @@ export function getScrolledIndex({ const offset = calculateOffset(snapAlign, itemsToShow) // Compute the index with or without wrapAround - if (!wrapAround) { - return getNumberInRange({ - val: currentSlide - offset, - max: slidesCount - itemsToShow, - min: 0, - }) - } else { - return mapNumberToRange({ - val: currentSlide - offset, - max: slidesCount + itemsToShow, - min: 0 - itemsToShow, - }) + if (wrapAround) { + return currentSlide - offset } + return getNumberInRange({ + val: currentSlide - offset, + max: slidesCount - itemsToShow, + min: 0, + }) } diff --git a/tests/integration/__snapshots__/carousel.spec.ts.snap b/tests/integration/__snapshots__/carousel.spec.ts.snap index 7a37214..6c7d322 100644 --- a/tests/integration/__snapshots__/carousel.spec.ts.snap +++ b/tests/integration/__snapshots__/carousel.spec.ts.snap @@ -4,16 +4,13 @@ exports[`SSR Carousel > renders server side properly 1`] = ` "
-
    - +
    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
      6. - -
" `; -exports[`SSR Carousel > renders server side properly 2`] = `"
  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
Item 2 of 5
"`; +exports[`SSR Carousel > renders server side properly 2`] = `"
  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
Item 2 of 5
"`; exports[`SSR Carousel > renders slotted server side properly 1`] = ` "
-
    +
    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
      6. -
Item 1 of 5
@@ -55,15 +51,12 @@ exports[`SSR Carousel > renders slotted server side properly 1`] = `
" `; -exports[`SSR Carousel > renders slotted server side properly 2`] = `"
  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
Item 1 of 5
"`; +exports[`SSR Carousel > renders slotted server side properly 2`] = `"
  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
Item 1 of 5
"`; exports[`Wrap around Carousel.ts > renders wrapAround correctly 1`] = ` "
-
    - - - +
    1. 1
    2. 2
    3. 3
      4. diff --git a/tests/integration/carousel.spec.ts b/tests/integration/carousel.spec.ts index 83564a4..9beb256 100644 --- a/tests/integration/carousel.spec.ts +++ b/tests/integration/carousel.spec.ts @@ -155,95 +155,6 @@ describe('Wrap around Carousel.ts', () => { }) }) -describe('Carousel Slide Cloning', () => { - it('should not clone slides when wrapAround is false', async () => { - const wrapper = await mount(App, { - props: { - slideNum: 5, - wrapAround: false, - itemsToShow: 3, - }, - }) - - const slides = wrapper.findAll('.carousel__slide') - const clonedSlides = wrapper.findAll('.carousel__slide--clone') - expect(slides.length).toBe(5) // Only original slides, no clones - expect(clonedSlides.length).toBe(0) - }) - - it('should clone correct number of slides with wrapAround enabled', async () => { - const wrapper = await mount(App, { - props: { - slideNum: 5, - wrapAround: true, - itemsToShow: 3, - modelValue: 0, - }, - }) - - const slides = wrapper.findAll('.carousel__slide') - // Original slides (5) + cloned before (3) + cloned after (3) - expect(slides.length).toBe(11) - }) - - it('should adjust clone count based on activeSlideIndex', async () => { - const wrapper = await mount(App, { - props: { - slideNum: 5, - wrapAround: true, - itemsToShow: 2, - modelValue: 0, - }, - }) - - // Initial state (at index 0) - let slides = wrapper.findAll('.carousel__slide') - const initialCount = slides.length - - // Move to middle slide - await wrapper.setProps({ modelValue: 2 }) - slides = wrapper.findAll('.carousel__slide') - expect(slides.length).toBe(initialCount) // Should maintain same total count - - // Move to last slide - await wrapper.setProps({ modelValue: 5 }) - slides = wrapper.findAll('.carousel__slide') - // Original slides (5) + cloned before (2) + cloned after (3) - expect(slides.length).toBe(10) // Should maintain same total count - }) - - it('should handle decimal itemsToShow values', async () => { - const wrapper = await mount(App, { - props: { - slideNum: 5, - wrapAround: true, - itemsToShow: 2.5, - modelValue: 0, - }, - }) - - const slides = wrapper.findAll('.carousel__slide') - // Original slides (5) + cloned before (3) + cloned after (3) - // Math.ceil(2.5) = 3 slides should be cloned on each side - expect(slides.length).toBe(11) - }) - - it('should handle edge case with single slide', async () => { - const wrapper = await mount(App, { - props: { - slideNum: 1, - wrapAround: true, - itemsToShow: 1, - modelValue: 0, - }, - }) - - const slides = wrapper.findAll('.carousel__slide') - // Original slide (1) + cloned before (1) + cloned after (1) - expect(slides.length).toBe(3) - }) -}) - describe('SSR Carousel', () => { const consoleMock = vi.spyOn(console, 'warn').mockImplementation(() => undefined) @@ -332,3 +243,105 @@ describe('SSR Carousel', () => { expect(slidesSSR.length).toBe(1) }) }) + +describe('Carousel Clone Count Logic', () => { + let wrapper: ReturnType> + + it('should not clone slides when wrapAround is disabled', async () => { + wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: false, + itemsToShow: 3, + itemsToScroll: 1, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + expect(slides.length).toBe(5) // Only original slides + }) + + it('should calculate correct clone counts at start position', async () => { + wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 3, + itemsToScroll: 2, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (5) + cloned slides before (4) + cloned slides after (0) + // slidesToClone = Math.ceil(3 + (2 - 1)) = 4 + // -> before: Math.max(0, 4-0) = 4 + // -> after: Math.max(0, 4-(5-(0+1))) = 0 + expect(slides.length).toBe(9) + }) + + it('should adjust clone counts when scrolling to middle', async () => { + wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 2, + itemsToScroll: 1, + modelValue: 2, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (5) only + expect(slides.length).toBe(5) + }) + + it('should handle edge case at last slide', async () => { + wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 2, + itemsToScroll: 1, + modelValue: 4, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (5) + cloned slides after (2) + expect(slides.length).toBe(7) + }) + + it('should handle decimal itemsToShow with itemsToScroll', async () => { + wrapper = await mount(App, { + props: { + slideNum: 5, + wrapAround: true, + itemsToShow: 2.5, + itemsToScroll: 2, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (5) + ceil(2.5 + (2 - 1)) = 4 + expect(slides.length).toBe(9) + }) + + it('should handle minimal setup with a single item', async () => { + wrapper = await mount(App, { + props: { + slideNum: 1, + wrapAround: true, + itemsToShow: 1, + itemsToScroll: 1, + modelValue: 0, + }, + }) + + const slides = wrapper.findAll('.carousel__slide') + // Original slides (1) + cloned slides before (1) + cloned slides after (1) + expect(slides.length).toBe(3) + }) +})