Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions __tests__/core/onScroll.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,31 +75,33 @@ describe("onScroll", () => {
expect(mockState.scroll).toBe(150);
});

it("normalizes negative horizontal offsets in RTL mode", () => {
it("clamps a negative (overscroll bounce) horizontal offset to the list end in RTL", () => {
mockState.props.horizontal = true;
I18nManager.isRTL = true;
mockState.scrollLength = 500;
mockScrollEvent.nativeEvent.contentSize.width = 1400;
mockScrollEvent.nativeEvent.contentSize.width = 1400; // maxOffset = 900
mockScrollEvent.nativeEvent.contentOffset.x = -120;

onScroll(mockCtx, mockScrollEvent);

expect(mockState.scrollPending).toBe(120);
expect(mockState.scroll).toBe(120);
expect(mockState.horizontalRTLScrollType).toBe("negative");
// Native RTL flips contentOffset; a negative raw is overscroll past the end, not a
// separate coordinate mode. Clamp to the end and keep the pinned "inverted" mode.
expect(mockState.scrollPending).toBe(900);
expect(mockState.scroll).toBe(900);
expect(mockState.horizontalRTLScrollType).toBe("inverted");
});

it("uses the rtl prop override when global I18nManager is false", () => {
mockState.props.horizontal = true;
mockState.props.rtl = true;
mockState.scrollLength = 500;
mockScrollEvent.nativeEvent.contentSize.width = 1400;
mockScrollEvent.nativeEvent.contentOffset.x = -75;
mockScrollEvent.nativeEvent.contentSize.width = 1400; // maxOffset = 900
mockScrollEvent.nativeEvent.contentOffset.x = 600; // inverted -> 300

onScroll(mockCtx, mockScrollEvent);

expect(mockState.scroll).toBe(75);
expect(mockState.horizontalRTLScrollType).toBe("negative");
expect(mockState.scroll).toBe(300);
expect(mockState.horizontalRTLScrollType).toBe("inverted");
});

it("respects rtl=false override when global I18nManager is true", () => {
Expand Down
4 changes: 3 additions & 1 deletion __tests__/core/syncMountedContainer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe("syncMountedContainer", () => {
expect(ctx.state.pendingDataComparison?.byIndex[1]).toBe(1);
});

it("mirrors horizontal rtl container positions into physical space", () => {
it("mirrors a prop-forced rtl container position on an LTR native tree", () => {
const ctx = createMockContext(
{
totalSize: 1000,
Expand All @@ -107,6 +107,8 @@ describe("syncMountedContainer", () => {

const result = syncMountedContainer(ctx, 0, 0);

// Prop-forced RTL (rtl prop, global I18nManager.isRTL === false): the native tree stays LTR
// so RN's left->start swap doesn't fire — the JS mirror is still needed here (1000-200-50).
expect(result.didChangePosition).toBe(true);
expect(peek$(ctx, "containerPosition0")).toBe(750);
});
Expand Down
49 changes: 42 additions & 7 deletions __tests__/utils/rtl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,45 @@ describe("rtl horizontal coordinate helpers", () => {
expect(isHorizontalRTL(state)).toBe(false);
});

it("normalizes negative native offsets into logical offsets", () => {
expect(toLogicalHorizontalOffset(state, -120, 1000)).toBe(120);
expect(state.horizontalRTLScrollType).toBe("negative");
it("treats a negative native offset as overscroll bounce and clamps it", () => {
// Was pinned to "negative" mode before; a transient bounce frame must not switch modes.
expect(toLogicalHorizontalOffset(state, -120, 1000)).toBe(700);
expect(state.horizontalRTLScrollType).toBe("inverted");
});

it("detects inverted native offsets from the first scroll sample", () => {
it("converts native offsets with the pinned inverted mode on native", () => {
expect(toLogicalHorizontalOffset(state, 700, 1000)).toBe(0);
expect(state.horizontalRTLScrollType).toBe("inverted");
});

it("uses the native RTL default before a first scroll sample classifies the mode", () => {
it("uses the native RTL default (inverted) deterministically", () => {
expect(toLogicalHorizontalOffset(state, 0, 1000)).toBe(700);
expect(state.horizontalRTLScrollType).toBe("inverted");
});

it("does not classify positive native offsets without content size", () => {
it("stays pinned to inverted across frames instead of reclassifying (regression: blank on scroll)", () => {
state.hasScrolled = true;
state.scroll = 0;
// A sequence that the old distance heuristic would have flipped to "normal" mid-scroll.
expect(toLogicalHorizontalOffset(state, 700, 1000)).toBe(0);
expect(toLogicalHorizontalOffset(state, 690, 1000)).toBe(10);
expect(toLogicalHorizontalOffset(state, 0, 1000)).toBe(700);
expect(state.horizontalRTLScrollType).toBe("inverted");
});

it("does not classify offsets without content size", () => {
Platform.OS = "android";

expect(toLogicalHorizontalOffset(state, 125, undefined)).toBe(125);
expect(state.horizontalRTLScrollType).toBeUndefined();
});

it("still normalizes negative scroll offsets on web (RTL scroll root reports negative scrollLeft)", () => {
Platform.OS = "web";
expect(toLogicalHorizontalOffset(state, -120, 1000)).toBe(120);
expect(state.horizontalRTLScrollType).toBe("negative");
});

it("uses platform defaults for native offsets before a scroll sample classifies the mode", () => {
Platform.OS = "android";
expect(toNativeHorizontalOffset(state, 100, 1000)).toBe(600);
Expand All @@ -83,7 +100,25 @@ describe("rtl horizontal coordinate helpers", () => {
expect(toNativeHorizontalOffset(state, 100, 1000)).toBe(-100);
});

it("mirrors logical item positions for physical horizontal rendering", () => {
it("does not mirror item positions when the native tree is actually RTL (RN swaps left->start)", () => {
I18nManager.isRTL = true;
// Native + global RTL: return the logical position unchanged; mirroring here double-mirrors (#477).
expect(toPhysicalHorizontalItemPosition(state, 200, 50, 1000)).toBe(200);

state.props.rtl = false;
expect(toPhysicalHorizontalItemPosition(state, 200, 50, 1000)).toBe(200);
});

it("mirrors item positions for a prop-forced rtl list on an LTR native tree", () => {
// rtl prop, global I18nManager.isRTL === false: no native swap, so the JS mirror is needed.
expect(toPhysicalHorizontalItemPosition(state, 200, 50, 1000)).toBe(750);

state.props.rtl = false;
expect(toPhysicalHorizontalItemPosition(state, 200, 50, 1000)).toBe(200);
});

it("mirrors item positions on web (container is forced direction: ltr there)", () => {
Platform.OS = "web";
expect(toPhysicalHorizontalItemPosition(state, 200, 50, 1000)).toBe(750);

state.props.rtl = false;
Expand Down
14 changes: 11 additions & 3 deletions src/core/checkFinishedScroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { initialScrollCompletion, initialScrollWatchdog } from "@/core/initialSc
import { Platform } from "@/platform/Platform";
import { getContentSize } from "@/state/getContentSize";
import type { StateContext } from "@/state/state";
import { toNativeHorizontalOffset } from "@/utils/rtl";

type ActiveScrollTarget = NonNullable<StateContext["state"]["scrollingTo"]>;
const INITIAL_SCROLL_MAX_FALLBACK_CHECKS = 20;
Expand Down Expand Up @@ -151,10 +152,17 @@ function checkFinishedScrollFrame(ctx: StateContext) {
}

function scrollToFallbackOffset(ctx: StateContext, offset: number) {
ctx.state.refScroller.current?.scrollTo({
const state = ctx.state;
const isHorizontal = !!state.props.horizontal;
// `offset` is a logical (LTR) offset. The normal doScrollTo dispatch converts it to the native
// RTL coordinate; do the same here so the watchdog/retry doesn't dispatch an unconverted offset
// and land on the RTL mirror index (fixes #476, and stops the retry from fighting the converting
// dispatch — the ping-pong shows up as flicker on horizontal RTL lists).
const x = isHorizontal ? toNativeHorizontalOffset(state, offset, getContentSize(ctx)) : 0;
state.refScroller.current?.scrollTo({
animated: false,
x: ctx.state.props.horizontal ? offset : 0,
y: ctx.state.props.horizontal ? 0 : offset,
x,
y: isHorizontal ? 0 : offset,
});
}

Expand Down
27 changes: 27 additions & 0 deletions src/utils/rtl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ export function toPhysicalHorizontalItemPosition(
return logicalPosition;
}

// When the native tree is actually RTL, RN's `doLeftAndRightSwapInRTL` (on by default) rewrites
// the item's `left` inset to `start`, which Yoga resolves from the right edge — so native layout
// already mirrors logical positions. Mirroring here as well double-mirrors every item off screen
// (blank list, #477 / #458). Gate on the global `I18nManager.isRTL`: a per-list `rtl` prop on an
// otherwise-LTR native tree gets no native swap, so it still needs the JS mirror. Web always keeps
// the JS mirror since Container forces `direction: ltr` there.
if (Platform.OS !== "web" && I18nManager.isRTL) {
return logicalPosition;
}

return Math.max(0, listSize - logicalPosition - itemSize);
}

Expand Down Expand Up @@ -133,6 +143,23 @@ export function toLogicalHorizontalOffset(

const maxOffset = getHorizontalMaxOffset(state, contentWidth);

// Native: the scroll coordinate space is deterministic — iOS/Android Fabric flip contentOffset in
// both directions with the self-inverse `maxOffset - x`, so JS reads a physical-left-based offset
// ("inverted"). Pin it instead of the per-frame distance heuristic, which could reclassify
// "inverted" -> "normal" on a single overscroll-bounce frame (a transient negative rawOffset first
// pinned "negative", then the next positive frame fell through to the heuristic) and mirror the
// visible-range math mid-scroll, blanking the whole list. A negative rawOffset is bounce: clamp it,
// never switch modes.
if (Platform.OS !== "web") {
if (maxOffset === undefined) {
return rawOffset < 0 ? -rawOffset : rawOffset;
}
state.horizontalRTLScrollType = "inverted";
return clampHorizontalOffset(maxOffset - rawOffset, maxOffset);
}

// Web: browsers report flow-relative scroll offsets that may be normal or, in an RTL scroll root
// (inherited dir="rtl" / direction: rtl), negative. Keep the existing adaptive classification.
if (rawOffset < 0) {
state.horizontalRTLScrollType = "negative";
return clampHorizontalOffset(-rawOffset, maxOffset);
Expand Down