Merge pull request #787 from davidjerleke/bug/#784

[Bug]: Direction rtl reverses y axis direction

packages/embla-carousel-docs/src/components/CarouselGenerator/CarouselGeneratorBasicSettings.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect } from 'react'
+import React from 'react'
 import { InputRadioDefault } from 'components/Input/InputRadio'
 import { InputCheckboxDefault } from 'components/Input/InputCheckbox'
 import { useCarouselGenerator } from 'hooks/useCarouselGenerator'
@@ -45,13 +45,7 @@ const INPUT_ACCESSIBILITY: SandboxGeneratorCheckboxType<'accessibility'> = {
 }
 
 export const CarouselGeneratorBasicSettings = () => {
-  const { formData, onCheckboxChange, onRadioChange, onChange } =
-    useCarouselGenerator()
-  const axis = formData[SANDBOX_GENERATOR_FORM_FIELDS.AXIS]
-
-  useEffect(() => {
-    if (axis === 'y') onChange(SANDBOX_GENERATOR_FORM_FIELDS.DIRECTION, 'ltr')
-  }, [axis])
+  const { formData, onCheckboxChange, onRadioChange } = useCarouselGenerator()
 
   return (
     <>
@@ -71,26 +65,24 @@ export const CarouselGeneratorBasicSettings = () => {
         ))}
       </CarouselGeneratorFormItems>
 
-      {axis === 'x' && (
-        <CarouselGeneratorFormItems
-          role="radiogroup"
-          aria-label={INPUT_DIRECTION.ID}
-        >
-          {INPUT_DIRECTION.OPTIONS.map(({ VALUE, LABEL }) => (
-            <div key={VALUE}>
-              <InputRadioDefault
-                name={INPUT_DIRECTION.FIELD_NAME}
-                id={`${INPUT_DIRECTION.ID}-${VALUE}`}
-                value={VALUE}
-                checked={formData[INPUT_DIRECTION.FIELD_NAME] === VALUE}
-                onChange={onRadioChange}
-              >
-                {LABEL}
-              </InputRadioDefault>
-            </div>
-          ))}
-        </CarouselGeneratorFormItems>
-      )}
+      <CarouselGeneratorFormItems
+        role="radiogroup"
+        aria-label={INPUT_DIRECTION.ID}
+      >
+        {INPUT_DIRECTION.OPTIONS.map(({ VALUE, LABEL }) => (
+          <div key={VALUE}>
+            <InputRadioDefault
+              name={INPUT_DIRECTION.FIELD_NAME}
+              id={`${INPUT_DIRECTION.ID}-${VALUE}`}
+              value={VALUE}
+              checked={formData[INPUT_DIRECTION.FIELD_NAME] === VALUE}
+              onChange={onRadioChange}
+            >
+              {LABEL}
+            </InputRadioDefault>
+          </div>
+        ))}
+      </CarouselGeneratorFormItems>
 
       <CarouselGeneratorFormItem>
         <InputCheckboxDefault

packages/embla-carousel-docs/src/components/Sandbox/SandboxGeneratorExampleArrowButtons.tsx

@@ -73,14 +73,14 @@ export const PrevButton: React.FC<PropType> = (props) => {
       {...restProps}
     >
       <svg className="embla__button__svg" viewBox="0 0 532 532">
-        {isRightToLeft && (
+        {!isVertical && isRightToLeft && (
           <path
             fill="currentColor"
             d="M176.34 520.646c-13.793 13.805-36.208 13.805-50.001 0-13.785-13.804-13.785-36.238 0-50.034L330.78 266 126.34 61.391c-13.785-13.805-13.785-36.239 0-50.044 13.793-13.796 36.208-13.796 50.002 0 22.928 22.947 206.395 206.507 229.332 229.454a35.065 35.065 0 0 1 10.326 25.126c0 9.2-3.393 18.26-10.326 25.2-45.865 45.901-206.404 206.564-229.332 229.52Z"
           />
         )}
 
-        {!isRightToLeft && !isVertical && (
+        {!isVertical && !isRightToLeft && (
           <path
             fill="currentColor"
             d="M355.66 11.354c13.793-13.805 36.208-13.805 50.001 0 13.785 13.804 13.785 36.238 0 50.034L201.22 266l204.442 204.61c13.785 13.805 13.785 36.239 0 50.044-13.793 13.796-36.208 13.796-50.002 0a5994246.277 5994246.277 0 0 0-229.332-229.454 35.065 35.065 0 0 1-10.326-25.126c0-9.2 3.393-18.26 10.326-25.2C172.192 194.973 332.731 34.31 355.66 11.354Z"
@@ -109,14 +109,14 @@ export const NextButton: React.FC<PropType> = (props) => {
       {...restProps}
     >
       <svg className="embla__button__svg" viewBox="0 0 532 532">
-        {isRightToLeft && (
+        {!isVertical && isRightToLeft && (
           <path
             fill="currentColor"
             d="M355.66 11.354c13.793-13.805 36.208-13.805 50.001 0 13.785 13.804 13.785 36.238 0 50.034L201.22 266l204.442 204.61c13.785 13.805 13.785 36.239 0 50.044-13.793 13.796-36.208 13.796-50.002 0a5994246.277 5994246.277 0 0 0-229.332-229.454 35.065 35.065 0 0 1-10.326-25.126c0-9.2 3.393-18.26 10.326-25.2C172.192 194.973 332.731 34.31 355.66 11.354Z"
           />
         )}
 
-        {!isRightToLeft && !isVertical && (
+        {!isVertical && !isRightToLeft && (
           <path
             fill="currentColor"
             d="M176.34 520.646c-13.793 13.805-36.208 13.805-50.001 0-13.785-13.804-13.785-36.238 0-50.034L330.78 266 126.34 61.391c-13.785-13.805-13.785-36.239 0-50.044 13.793-13.796 36.208-13.796 50.002 0 22.928 22.947 206.395 206.507 229.332 229.454a35.065 35.065 0 0 1 10.326 25.126c0 9.2-3.393 18.26-10.326 25.2-45.865 45.901-206.404 206.564-229.332 229.52Z"

packages/embla-carousel-docs/src/components/Sandbox/sandboxGeneratorCreateOptions.ts

@@ -18,7 +18,7 @@ export const sandboxGeneratorCreateOptions = (
     ...(axis !== 'x' && { axis }),
     ...(align !== 'center' && { align }),
     ...(dragFree && { dragFree }),
-    ...(direction !== 'ltr' && axis === 'x' && { direction }),
+    ...(direction !== 'ltr' && { direction }),
     ...(!loop && !containScroll && { containScroll: false }),
     ...(loop && { loop }),
     ...(slidesToScroll !== 1 && { slidesToScroll })

packages/embla-carousel/src/__tests__/axis-vertical.test.ts

@@ -2,7 +2,7 @@ import EmblaCarousel from '../components/EmblaCarousel'
 import { mockTestElements } from './mocks'
 import { FIXTURE_AXIS_Y } from './fixtures/axis-vertical.fixture'
 
-describe('➡️  Axis - Vertical', () => {
+describe('➡️  Axis - Vertical LTR', () => {
   describe('Translates correctly when direction is:', () => {
     test('Vertical', () => {
       const emblaApi = EmblaCarousel(mockTestElements(FIXTURE_AXIS_Y), {
@@ -16,3 +16,17 @@ describe('➡️  Axis - Vertical', () => {
     })
   })
 })
+
+describe('➡️  Axis - Vertical RTL', () => {
+  test('Translates correctly', () => {
+    const emblaApi = EmblaCarousel(mockTestElements(FIXTURE_AXIS_Y), {
+      containScroll: false,
+      direction: 'rtl',
+      axis: 'y'
+    })
+
+    expect(emblaApi.containerNode().style.transform).toBe(
+      'translate3d(0px,100px,0px)'
+    )
+  })
+})

packages/embla-carousel/src/components/Axis.ts

@@ -1,7 +1,7 @@
-import { DirectionOptionType } from './Direction'
 import { NodeRectType } from './NodeRects'
 
 export type AxisOptionType = 'x' | 'y'
+export type AxisDirectionOptionType = 'ltr' | 'rtl'
 type AxisEdgeType = 'top' | 'right' | 'bottom' | 'left'
 
 export type AxisType = {
@@ -10,38 +10,47 @@ export type AxisType = {
   startEdge: AxisEdgeType
   endEdge: AxisEdgeType
   measureSize: (nodeRect: NodeRectType) => number
+  direction: (n: number) => number
 }
 
 export function Axis(
   axis: AxisOptionType,
-  direction: DirectionOptionType
+  contentDirection: AxisDirectionOptionType
 ): AxisType {
-  const scroll = axis === 'y' ? 'y' : 'x'
-  const cross = axis === 'y' ? 'x' : 'y'
+  const isRightToLeft = contentDirection === 'rtl'
+  const isVertical = axis === 'y'
+  const scroll = isVertical ? 'y' : 'x'
+  const cross = isVertical ? 'x' : 'y'
+  const sign = !isVertical && isRightToLeft ? -1 : 1
   const startEdge = getStartEdge()
   const endEdge = getEndEdge()
 
   function measureSize(nodeRect: NodeRectType): number {
-    const { width, height } = nodeRect
-    return scroll === 'x' ? width : height
+    const { height, width } = nodeRect
+    return isVertical ? height : width
   }
 
   function getStartEdge(): AxisEdgeType {
-    if (scroll === 'y') return 'top'
-    return direction === 'rtl' ? 'right' : 'left'
+    if (isVertical) return 'top'
+    return isRightToLeft ? 'right' : 'left'
   }
 
   function getEndEdge(): AxisEdgeType {
-    if (scroll === 'y') return 'bottom'
-    return direction === 'rtl' ? 'left' : 'right'
+    if (isVertical) return 'bottom'
+    return isRightToLeft ? 'left' : 'right'
+  }
+
+  function direction(n: number): number {
+    return n * sign
   }
 
   const self: AxisType = {
     scroll,
     cross,
     startEdge,
     endEdge,
-    measureSize
+    measureSize,
+    direction
   }
   return self
 }

packages/embla-carousel/src/components/DragHandler.ts

@@ -1,7 +1,6 @@
 import { EmblaCarouselType } from './EmblaCarousel'
 import { AnimationsType } from './Animations'
 import { CounterType } from './Counter'
-import { DirectionType } from './Direction'
 import { DragTrackerType, PointerEventType } from './DragTracker'
 import { EventHandlerType } from './EventHandler'
 import { AxisType } from './Axis'
@@ -37,7 +36,6 @@ export type DragHandlerType = {
 
 export function DragHandler(
   axis: AxisType,
-  direction: DirectionType,
   rootNode: HTMLElement,
   ownerDocument: Document,
   ownerWindow: WindowType,
@@ -57,7 +55,7 @@ export function DragHandler(
   baseFriction: number,
   watchDrag: DragHandlerOptionType
 ): DragHandlerType {
-  const { cross: crossAxis } = axis
+  const { cross: crossAxis, direction } = axis
   const focusNodes = ['INPUT', 'SELECT', 'TEXTAREA']
   const nonPassiveEvent = { passive: false }
   const initEvents = EventStore()
@@ -164,15 +162,15 @@ export function DragHandler(
 
     scrollBody.useFriction(0.3).useDuration(1)
     animation.start()
-    target.add(direction.apply(diff))
+    target.add(direction(diff))
     evt.preventDefault()
   }
 
   function up(evt: PointerEventType): void {
     const currentLocation = scrollTarget.byDistance(0, false)
     const targetChanged = currentLocation.index !== index.get()
     const rawForce = dragTracker.pointerUp(evt) * forceBoost()
-    const force = allowedForce(direction.apply(rawForce), targetChanged)
+    const force = allowedForce(direction(rawForce), targetChanged)
     const forceFactor = factorAbs(rawForce, force)
     const speed = baseSpeed - 10 * forceFactor
     const friction = baseFriction + forceFactor / 50

packages/embla-carousel/src/components/Engine.ts

@@ -7,7 +7,6 @@ import {
 } from './Animations'
 import { Axis, AxisType } from './Axis'
 import { Counter, CounterType } from './Counter'
-import { Direction, DirectionType } from './Direction'
 import { DragHandler, DragHandlerType } from './DragHandler'
 import { DragTracker } from './DragTracker'
 import { EventHandlerType } from './EventHandler'
@@ -42,7 +41,6 @@ export type EngineType = {
   ownerWindow: WindowType
   eventHandler: EventHandlerType
   axis: AxisType
-  direction: DirectionType
   animation: AnimationsType
   scrollBounds: ScrollBoundsType
   scrollLooper: ScrollLooperType
@@ -88,7 +86,7 @@ export function Engine(
   const {
     align,
     axis: scrollAxis,
-    direction: contentDirection,
+    direction,
     startIndex,
     loop,
     duration,
@@ -108,8 +106,7 @@ export function Engine(
   const nodeRects = NodeRects()
   const containerRect = nodeRects.measure(container)
   const slideRects = slides.map(nodeRects.measure)
-  const direction = Direction(contentDirection)
-  const axis = Axis(scrollAxis, contentDirection)
+  const axis = Axis(scrollAxis, direction)
   const viewSize = axis.measureSize(containerRect)
   const percentOfView = PercentOfView(viewSize)
   const alignment = Alignment(align, viewSize)
@@ -125,7 +122,6 @@ export function Engine(
   )
   const slidesToScroll = SlidesToScroll(
     axis,
-    direction,
     viewSize,
     groupSlides,
     loop,
@@ -272,10 +268,8 @@ export function Engine(
     slideRects,
     animation,
     axis,
-    direction,
     dragHandler: DragHandler(
       axis,
-      direction,
       root,
       ownerDocument,
       ownerWindow,
@@ -332,7 +326,6 @@ export function Engine(
     scrollTo,
     slideLooper: SlideLooper(
       axis,
-      direction,
       viewSize,
       contentSize,
       slideSizes,
@@ -349,7 +342,7 @@ export function Engine(
     slideRegistry,
     slidesToScroll,
     target,
-    translate: Translate(axis, direction, container)
+    translate: Translate(axis, container)
   }
 
   return engine

packages/embla-carousel/src/components/Options.ts

@@ -1,7 +1,6 @@
 import { AlignmentOptionType } from './Alignment'
-import { AxisOptionType } from './Axis'
+import { AxisDirectionOptionType, AxisOptionType } from './Axis'
 import { SlidesToScrollOptionType } from './SlidesToScroll'
-import { DirectionOptionType } from './Direction'
 import { ScrollContainOptionType } from './ScrollContain'
 import { DragHandlerOptionType } from './DragHandler'
 import { ResizeHandlerOptionType } from './ResizeHandler'
@@ -25,7 +24,7 @@ export type OptionsType = CreateOptionsType<{
   container: string | HTMLElement | null
   slides: string | HTMLElement[] | NodeListOf<HTMLElement> | null
   containScroll: ScrollContainOptionType
-  direction: DirectionOptionType
+  direction: AxisDirectionOptionType
   slidesToScroll: SlidesToScrollOptionType
   dragFree: boolean
   dragThreshold: number

packages/embla-carousel/src/components/SlideLooper.ts

@@ -2,7 +2,6 @@ import { AxisType } from './Axis'
 import { arrayKeys } from './utils'
 import { Vector1D, Vector1DType } from './Vector1d'
 import { Translate, TranslateType } from './Translate'
-import { DirectionType } from './Direction'
 
 type SlideBoundType = {
   start: number
@@ -26,7 +25,6 @@ export type SlideLooperType = {
 
 export function SlideLooper(
   axis: AxisType,
-  direction: DirectionType,
   viewSize: number,
   contentSize: number,
   slideSizes: number[],
@@ -78,7 +76,7 @@ export function SlideLooper(
         index,
         loopPoint,
         slideLocation: Vector1D(-1),
-        translate: Translate(axis, direction, slides[index]),
+        translate: Translate(axis, slides[index]),
         target: () => (offsetLocation.get() > loopPoint ? initial : altered)
       }
     })