Merge pull request #1096 from yasuhiro-yamamoto/fix/issue-899

[Bug]: Offset by 1 issue for canScrollNext when tab size has fractional width
davidjerleke · Jan 6, 2025 · 1b01939 · 1b01939
2 parents ba2c14c + 266ef1b
commit 1b01939
Show file tree

Hide file tree

Showing 7 changed files with 186 additions and 4 deletions.
diff --git a/packages/embla-carousel/src/__tests__/containScroll-ltr.test.ts b/packages/embla-carousel/src/__tests__/containScroll-ltr.test.ts
@@ -7,7 +7,8 @@ import {
   FIXTURE_CONTAIN_SCROLL_LTR_4,
   FIXTURE_CONTAIN_SCROLL_LTR_5,
   FIXTURE_CONTAIN_SCROLL_LTR_6,
-  FIXTURE_CONTAIN_SCROLL_LTR_7
+  FIXTURE_CONTAIN_SCROLL_LTR_7,
+  FIXTURE_CONTAIN_SCROLL_LTR_8
 } from './fixtures/containScroll-ltr.fixture'
 
 const FIRST_SNAP_INDEX = 0
@@ -250,6 +251,20 @@ describe('➡️  ContainScroll - Horizontal LTR', () => {
 
       expect(engine.slideRegistry).toEqual([[0, 1], [2], [3], [4, 5]])
     })
+
+    test('Unexpected 1 pixel snaps are NOT present at the end of the carousel', () => {
+      const emblaApi = EmblaCarousel(
+        mockTestElements(FIXTURE_CONTAIN_SCROLL_LTR_8),
+        { align: 'start' }
+      )
+
+      const engine = emblaApi.internalEngine()
+      const expectedScrollSnaps = [0, -400, -800]
+      expect(engine.scrollSnaps).toEqual(expectedScrollSnaps)
+      expect(engine.location.get()).toBe(expectedScrollSnaps[FIRST_SNAP_INDEX])
+
+      expect(engine.slideRegistry).toEqual([[0], [1], [2, 3]])
+    })
   })
 
   describe('"keepSnaps" is correct for slides WITHOUT MARGINS and ALIGN is:', () => {

diff --git a/packages/embla-carousel/src/__tests__/containScroll-rtl.test.ts b/packages/embla-carousel/src/__tests__/containScroll-rtl.test.ts
@@ -7,7 +7,8 @@ import {
   FIXTURE_CONTAIN_SCROLL_RTL_4,
   FIXTURE_CONTAIN_SCROLL_RTL_5,
   FIXTURE_CONTAIN_SCROLL_RTL_6,
-  FIXTURE_CONTAIN_SCROLL_RTL_7
+  FIXTURE_CONTAIN_SCROLL_RTL_7,
+  FIXTURE_CONTAIN_SCROLL_RTL_8
 } from './fixtures/containScroll-rtl.fixture'
 
 const FIRST_SNAP_INDEX = 0
@@ -255,6 +256,20 @@ describe('➡️  ContainScroll - Horizontal RTL', () => {
 
       expect(engine.slideRegistry).toEqual([[0, 1], [2], [3], [4, 5]])
     })
+
+    test('Unexpected 1 pixel snaps are NOT present at the end of the carousel', () => {
+      const emblaApi = EmblaCarousel(
+        mockTestElements(FIXTURE_CONTAIN_SCROLL_RTL_8),
+        { direction: 'rtl', align: 'start' }
+      )
+
+      const engine = emblaApi.internalEngine()
+      const expectedScrollSnaps = [0, -399, -799]
+      expect(engine.scrollSnaps).toEqual(expectedScrollSnaps)
+      expect(engine.location.get()).toBe(expectedScrollSnaps[FIRST_SNAP_INDEX])
+
+      expect(engine.slideRegistry).toEqual([[0], [1], [2, 3]])
+    })
   })
 
   describe('"keepSnaps" is correct for slides WITHOUT MARGINS and ALIGN is:', () => {

diff --git a/packages/embla-carousel/src/__tests__/containScroll-vertical.test.ts b/packages/embla-carousel/src/__tests__/containScroll-vertical.test.ts
@@ -7,7 +7,8 @@ import {
   FIXTURE_CONTAIN_SCROLL_Y_4,
   FIXTURE_CONTAIN_SCROLL_Y_5,
   FIXTURE_CONTAIN_SCROLL_Y_6,
-  FIXTURE_CONTAIN_SCROLL_Y_7
+  FIXTURE_CONTAIN_SCROLL_Y_7,
+  FIXTURE_CONTAIN_SCROLL_Y_8
 } from './fixtures/containScroll-vertical.fixture'
 
 const FIRST_SNAP_INDEX = 0
@@ -253,6 +254,20 @@ describe('➡️  ContainScroll - Vertical', () => {
 
       expect(engine.slideRegistry).toEqual([[0, 1], [2], [3], [4, 5]])
     })
+
+    test('Unexpected 1 pixel snaps are NOT present at the end of the carousel', () => {
+      const emblaApi = EmblaCarousel(
+        mockTestElements(FIXTURE_CONTAIN_SCROLL_Y_8),
+        { axis: 'y', align: 'start' }
+      )
+
+      const engine = emblaApi.internalEngine()
+      const expectedScrollSnaps = [0, -400, -800]
+      expect(engine.scrollSnaps).toEqual(expectedScrollSnaps)
+      expect(engine.location.get()).toBe(expectedScrollSnaps[FIRST_SNAP_INDEX])
+
+      expect(engine.slideRegistry).toEqual([[0], [1], [2, 3]])
+    })
   })
 
   describe('"keepSnaps" is correct for slides WITHOUT MARGINS and ALIGN is:', () => {

diff --git a/packages/embla-carousel/src/__tests__/fixtures/containScroll-ltr.fixture.ts b/packages/embla-carousel/src/__tests__/fixtures/containScroll-ltr.fixture.ts
@@ -405,3 +405,49 @@ export const FIXTURE_CONTAIN_SCROLL_LTR_7: TestElementDimensionsType = {
     value: 0
   }
 }
+
+/*
+Fixture 8
+
+- Horizontal
+- LTR
+- No slide margins
+*/
+export const FIXTURE_CONTAIN_SCROLL_LTR_8: TestElementDimensionsType = {
+  containerOffset: {
+    offsetWidth: 799,
+    offsetHeight: 190,
+    offsetTop: 176,
+    offsetLeft: 0
+  },
+  slideOffsets: [
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: 0
+    },
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: 400
+    },
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: 799
+    },
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: 1199
+    }
+  ],
+  endMargin: {
+    property: 'marginRight',
+    value: 0
+  }
+}
diff --git a/packages/embla-carousel/src/__tests__/fixtures/containScroll-rtl.fixture.ts b/packages/embla-carousel/src/__tests__/fixtures/containScroll-rtl.fixture.ts
@@ -405,3 +405,49 @@ export const FIXTURE_CONTAIN_SCROLL_RTL_7: TestElementDimensionsType = {
     value: 0
   }
 }
+
+/*
+Fixture 8
+
+- Horizontal
+- RTL
+- No slide margins
+*/
+export const FIXTURE_CONTAIN_SCROLL_RTL_8: TestElementDimensionsType = {
+  containerOffset: {
+    offsetWidth: 799,
+    offsetHeight: 190,
+    offsetTop: 176,
+    offsetLeft: 0
+  },
+  slideOffsets: [
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: 400
+    },
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: 0
+    },
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: -399
+    },
+    {
+      offsetWidth: 400,
+      offsetHeight: 190,
+      offsetTop: 0,
+      offsetLeft: -799
+    }
+  ],
+  endMargin: {
+    property: 'marginLeft',
+    value: 0
+  }
+}
diff --git a/packages/embla-carousel/src/__tests__/fixtures/containScroll-vertical.fixture.ts b/packages/embla-carousel/src/__tests__/fixtures/containScroll-vertical.fixture.ts
@@ -398,3 +398,48 @@ export const FIXTURE_CONTAIN_SCROLL_Y_7: TestElementDimensionsType = {
     value: 0
   }
 }
+
+/*
+Fixture 8
+
+- Vertical
+- No slide margins
+*/
+export const FIXTURE_CONTAIN_SCROLL_Y_8: TestElementDimensionsType = {
+  containerOffset: {
+    offsetWidth: 799,
+    offsetHeight: 799,
+    offsetTop: 0,
+    offsetLeft: 0
+  },
+  slideOffsets: [
+    {
+      offsetWidth: 799,
+      offsetHeight: 400,
+      offsetTop: 0,
+      offsetLeft: 0
+    },
+    {
+      offsetWidth: 799,
+      offsetHeight: 400,
+      offsetTop: 400,
+      offsetLeft: 0
+    },
+    {
+      offsetWidth: 799,
+      offsetHeight: 400,
+      offsetTop: 799,
+      offsetLeft: 0
+    },
+    {
+      offsetWidth: 799,
+      offsetHeight: 400,
+      offsetTop: 1199,
+      offsetLeft: 0
+    }
+  ],
+  endMargin: {
+    property: 'marginBottom',
+    value: 0
+  }
+}
diff --git a/packages/embla-carousel/src/components/ScrollContain.ts b/packages/embla-carousel/src/components/ScrollContain.ts
@@ -21,7 +21,7 @@ export function ScrollContain(
   const snapsContained = measureContained()
 
   function usePixelTolerance(bound: number, snap: number): boolean {
-    return deltaAbs(bound, snap) < 1
+    return deltaAbs(bound, snap) <= 1
   }
 
   function findScrollContainLimit(): LimitType {