fix(virtual-core): restore programmatic scrolling in RTL elements

.changeset/tidy-rtl-scroll.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/virtual-core': patch
+---
+
+Fix programmatic scrolling in horizontal RTL elements by converting logical offsets, including measurement adjustments, to negative DOM scroll offsets.

packages/marko-virtual/e2e/app/e2e/option-gates.spec.ts

@@ -141,6 +141,18 @@ test('isRtl: a right-to-left horizontal list advances as the user scrolls left',
   expect(idx[idx.length - 1]!).toBeGreaterThan(50)
 })
 
+test('isRtl: scrollToIndex reaches a column using the default scroll function', async ({
+  page,
+}) => {
+  await page.goto('/rtl')
+  await page.waitForSelector('[data-index="0"]')
+  await page.getByTestId('scroll-50').click()
+
+  const scroller = page.getByTestId('scroller')
+  await expect.poll(() => scroller.evaluate((el) => el.scrollLeft)).toBe(-5000)
+  await expect(page.locator('[data-index="50"]')).toBeInViewport()
+})
+
 test('custom measureElement: item sizes come from the custom measurer (+10)', async ({
   page,
 }) => {

packages/marko-virtual/e2e/app/src/routes/rtl/+page.marko

@@ -5,15 +5,18 @@
 <html lang="en">
   <head><meta charset="UTF-8"/><title>gate: isRtl</title></head>
   <body>
+    <virtualizer/v
+      count=10000
+      estimateSize=() => 100
+      overscan=5
+      horizontal=true
+      isRtl=true
+      getScrollElement=(): HTMLDivElement | null => scrollEl()
+    />
+    <button data-testid="scroll-50" onClick() { v.scrollToIndex(50, { align: "start" }) }>
+      Scroll to column 50
+    </button>
     <div/scrollEl dir="rtl" data-testid="scroller" style="width: 500px; height: 120px; overflow-x: auto; position: relative">
-      <virtualizer/v
-        count=10000
-        estimateSize=() => 100
-        overscan=5
-        horizontal=true
-        isRtl=true
-        getScrollElement=() => scrollEl()
-      />
       <div style=`width: ${v.totalSize}px; height: 100%; position: relative`>
         <for|item| of=v.virtualItems>
           <div

packages/virtual-core/src/index.ts

@@ -300,16 +300,29 @@ export const measureElement = <TItemElement extends Element>(
   ]
 }
 
-const scrollWithAdjustments = (
+const scrollWithAdjustments = <
+  TScrollElement extends Element | Window,
+  TItemElement extends Element,
+>(
   offset: number,
   {
     adjustments = 0,
     behavior,
   }: { adjustments?: number; behavior?: ScrollBehavior },
-  instance: Virtualizer<any, any>,
+  instance: Virtualizer<TScrollElement, TItemElement>,
 ) => {
-  instance.scrollElement?.scrollTo?.({
-    [instance.options.horizontal ? 'left' : 'top']: offset + adjustments,
+  const { scrollElement, options } = instance
+  const adjustedOffset = offset + adjustments
+  const isRtlElement =
+    options.horizontal &&
+    options.isRtl &&
+    scrollElement !== null &&
+    'ownerDocument' in scrollElement
+
+  scrollElement?.scrollTo?.({
+    [options.horizontal ? 'left' : 'top']: isRtlElement
+      ? -adjustedOffset
+      : adjustedOffset,
     behavior,
   })
 }

packages/virtual-core/tests/dom-observers.test.ts

@@ -49,6 +49,61 @@ test('elementScroll: uses left when horizontal is true', () => {
   expect(scrollTo).toHaveBeenCalledWith({ left: 100, behavior: 'auto' })
 })
 
+// In a right-to-left element scrollLeft runs from 0 down to negative values,
+// which is why observeElementOffset negates it under isRtl. The write has to
+// convert back, or the browser clamps the positive value to 0.
+test.each([
+  { adjustments: 0, left: -100 },
+  { adjustments: 25, left: -125 },
+  { adjustments: -25, left: -75 },
+])(
+  'elementScroll: writes a negative left when horizontal and isRtl (adjustments: $adjustments)',
+  ({ adjustments, left }) => {
+    const scrollEl = document.createElement('div')
+    const scrollTo = vi.fn()
+    scrollEl.scrollTo = scrollTo
+    elementScroll(
+      100,
+      { adjustments, behavior: 'auto' },
+      makeBaseInstance(scrollEl, { horizontal: true, isRtl: true }) as any,
+    )
+    expect(scrollTo).toHaveBeenCalledWith({ left, behavior: 'auto' })
+  },
+)
+
+test('elementScroll: isRtl does not change a vertical write', () => {
+  const scrollEl = document.createElement('div')
+  const scrollTo = vi.fn()
+  scrollEl.scrollTo = scrollTo
+  elementScroll(
+    100,
+    { behavior: 'auto' },
+    makeBaseInstance(scrollEl, { horizontal: false, isRtl: true }) as any,
+  )
+  expect(scrollTo).toHaveBeenCalledWith({ top: 100, behavior: 'auto' })
+})
+
+test('elementScroll: reading back its own RTL write gives the written offset', () => {
+  const scrollEl = document.createElement('div')
+  scrollEl.scrollTo = ((options: ScrollToOptions) => {
+    // A dir="rtl" scroller clamps scrollLeft to [-(scrollWidth - clientWidth), 0].
+    scrollEl.scrollLeft = Math.min(0, Math.max(-9500, options.left!))
+    scrollEl.dispatchEvent(new Event('scroll'))
+  }) as typeof scrollEl.scrollTo
+  const instance = makeObserveInstance(scrollEl, {
+    horizontal: true,
+    isRtl: true,
+  })
+  const cb = vi.fn()
+  const cleanup = observeElementOffset(instance as any, cb)
+
+  elementScroll(5000, { behavior: 'auto' }, instance as any)
+
+  expect(scrollEl.scrollLeft).toBe(-5000)
+  expect(cb).toHaveBeenLastCalledWith(5000, true)
+  cleanup?.()
+})
+
 test('windowScroll: calls scrollTo with top + behavior on the window', () => {
   const scrollTo = vi.fn()
   const win = { scrollTo }
@@ -67,6 +122,18 @@ test('windowScroll: applies adjustments + horizontal', () => {
   expect(scrollTo).toHaveBeenCalledWith({ left: 240, behavior: 'auto' })
 })
 
+// observeWindowOffset reads scrollX as is, so the window write stays positive.
+test('windowScroll: isRtl does not change a horizontal write', () => {
+  const scrollTo = vi.fn()
+  const win = { scrollTo }
+  windowScroll(
+    250,
+    { behavior: 'auto' },
+    makeBaseInstance(win, { horizontal: true, isRtl: true }) as any,
+  )
+  expect(scrollTo).toHaveBeenCalledWith({ left: 250, behavior: 'auto' })
+})
+
 test('elementScroll / windowScroll: no-op when scrollElement is null', () => {
   expect(() =>
     elementScroll(100, {}, makeBaseInstance(null) as any),

packages/virtual-core/tests/rtl-scroll.test.ts

@@ -0,0 +1,44 @@
+import { expect, test } from 'vitest'
+import { Virtualizer, elementScroll, observeElementOffset } from '../src/index'
+
+test.each([
+  { horizontal: true, isRtl: true, adjustments: 0 },
+  { horizontal: true, isRtl: true, adjustments: 25 },
+  { horizontal: true, isRtl: true, adjustments: -25 },
+  { horizontal: true, isRtl: false, adjustments: 25 },
+  { horizontal: false, isRtl: true, adjustments: 25 },
+])(
+  'element scrolling preserves logical offsets: %o',
+  ({ horizontal, isRtl, adjustments }) => {
+    const element = document.createElement('div')
+    element.scrollTo = (offsets) => {
+      if (typeof offsets === 'object') {
+        element.scrollLeft = offsets.left ?? element.scrollLeft
+        element.scrollTop = offsets.top ?? element.scrollTop
+      }
+    }
+    const virtualizer = new Virtualizer<HTMLDivElement, HTMLDivElement>({
+      horizontal,
+      isRtl,
+      count: 100,
+      getScrollElement: () => element,
+      estimateSize: () => 100,
+      scrollToFn: elementScroll,
+      observeElementRect: () => {},
+      observeElementOffset,
+    })
+    virtualizer.scrollElement = element
+    virtualizer.targetWindow = window
+    let observedOffset = 0
+    const cleanup = observeElementOffset(virtualizer, (offset) => {
+      observedOffset = offset
+    })
+    try {
+      elementScroll(100, { adjustments }, virtualizer)
+      element.dispatchEvent(new Event('scroll'))
+      expect(observedOffset).toBe(100 + adjustments)
+    } finally {
+      cleanup?.()
+    }
+  },
+)

packages/virtual-core/tests/scroll-commands.test.ts

@@ -1,9 +1,11 @@
 import { expect, test, vi } from 'vitest'
 import {
   Virtualizer,
+  elementScroll,
   observeElementOffset,
   observeElementRect,
 } from '../src/index'
+import { makeMockScrollElement } from './helpers'
 import type { VirtualizerOptions } from '../src/index'
 
 test('should not throw when component unmounts during scrollToIndex rAF loop', () => {
@@ -1946,3 +1948,81 @@ test('scrollToIndex reaches an index added since the last render once the sizer
   expect(el.scrollTop).toBe(5500)
   expect(v.scrollState).toBeNull()
 })
+
+// A dir="rtl" horizontal scroller as browsers implement it: scrollLeft is 0 at
+// the start (the right edge) and negative towards the end, and a write outside
+// [-(scrollWidth - clientWidth), 0] is clamped. The scroll event arrives in
+// the next frame.
+function setupRtlScroller() {
+  const rafCallbacks: Array<FrameRequestCallback> = []
+  const mockWindow = {
+    requestAnimationFrame: (cb: FrameRequestCallback) => rafCallbacks.push(cb),
+    cancelAnimationFrame: () => {},
+    setTimeout: () => 0,
+    clearTimeout: () => {},
+  }
+  const el = makeMockScrollElement({
+    scrollTop: 0,
+    scrollLeft: 0,
+    scrollWidth: 10000,
+    clientWidth: 500,
+    scrollHeight: 120,
+    clientHeight: 120,
+    offsetWidth: 500,
+    offsetHeight: 120,
+    ownerDocument: { defaultView: mockWindow },
+  })
+  el.scrollTo = ({ left }: ScrollToOptions) => {
+    el.scrollLeft = Math.min(
+      0,
+      Math.max(left!, el.clientWidth - el.scrollWidth),
+    )
+    rafCallbacks.push(() => el._dispatch('scroll'))
+  }
+  const v = new Virtualizer<HTMLDivElement, HTMLDivElement>({
+    count: 100,
+    horizontal: true,
+    isRtl: true,
+    estimateSize: () => 100,
+    getScrollElement: () => el,
+    scrollToFn: elementScroll,
+    observeElementRect: (_inst, cb) => {
+      cb({ width: 500, height: 120 })
+      return () => {}
+    },
+    observeElementOffset,
+  })
+  v._willUpdate()
+  v.getVirtualItems()
+  const settle = () => {
+    for (let i = 0; i < 10 && rafCallbacks.length > 0; i++) {
+      rafCallbacks.splice(0).forEach((cb) => cb(0))
+    }
+  }
+  return { v, el, settle }
+}
+
+test.each([
+  {
+    command: 'scrollToIndex',
+    run: (v: Virtualizer<any, any>) => v.scrollToIndex(50, { align: 'start' }),
+  },
+  {
+    command: 'scrollToOffset',
+    run: (v: Virtualizer<any, any>) => v.scrollToOffset(5000),
+  },
+  { command: 'scrollBy', run: (v: Virtualizer<any, any>) => v.scrollBy(5000) },
+])(
+  'isRtl: $command moves a horizontal element with the default elementScroll',
+  ({ run }) => {
+    const { v, el, settle } = setupRtlScroller()
+
+    run(v)
+    settle()
+
+    expect(el.scrollLeft).toBe(-5000)
+    expect(v.scrollOffset).toBe(5000)
+    expect(v.getVirtualItems()[0]!.index).toBeLessThanOrEqual(50)
+    expect(v.getVirtualItems().at(-1)!.index).toBeGreaterThanOrEqual(54)
+  },
+)