From fad6ed883bf66b026c0a0851512b7e3b997318e8 Mon Sep 17 00:00:00 2001 From: Lucca Boas <86315612+Luccacvb@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:12:22 -0300 Subject: [PATCH] fix(android): keep the visible page when pages change before it --- .../reactnativepagerview/ComposePagerView.kt | 25 +++-- .../issue-965-pages-before-current.e2e.ts | 96 +++++++++++++++++++ example/src/App.tsx | 5 + .../Issue965PagesBeforeCurrentRepro.tsx | 90 +++++++++++++++++ 4 files changed, 206 insertions(+), 10 deletions(-) create mode 100644 e2e/tests/issues/issue-965-pages-before-current.e2e.ts create mode 100644 example/src/gh-issues/Issue965PagesBeforeCurrentRepro.tsx diff --git a/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt b/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt index 814b7df3..068b4e1d 100644 --- a/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt +++ b/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt @@ -236,6 +236,9 @@ class ComposePagerView(context: Context) : FrameLayout(context) { if (child == null) { return } + // Page keys must be unique. If Fabric re-adds a view it never removed, move it like a + // ViewGroup would instead of listing it twice. + pages.remove(child) val insertionIndex = index.coerceIn(0, pages.size) pages.add(insertionIndex, child) } @@ -500,13 +503,6 @@ class ComposePagerView(context: Context) : FrameLayout(context) { currentPage = pagerState.currentPage dispatchPageSelected(pagerState.currentPage) } - - if (pagerState.currentPage >= pageCount) { - val target = pageCount - 1 - pagerState.scrollToPage(target) - currentPage = target - dispatchPageSelected(target) - } } LaunchedEffect(scrollCommandState.value, pageCount) { @@ -576,6 +572,9 @@ class ComposePagerView(context: Context) : FrameLayout(context) { val pageSpacing = pageMarginState.value.dp val beyondViewportPageCount = getBeyondViewportPageCount() + // Key pages by their React tag, so the visible page stays put and its new index is + // reported when React inserts or removes pages before it. + val pageKey: (Int) -> Any = { page -> pages[page].id } val overscrollConfiguration = if (overScrollModeState.value == OverScrollMode.Never) { null } else { @@ -590,7 +589,8 @@ class ComposePagerView(context: Context) : FrameLayout(context) { pageSpacing = pageSpacing, userScrollEnabled = scrollEnabledState.value, reverseLayout = false, - beyondViewportPageCount = beyondViewportPageCount + beyondViewportPageCount = beyondViewportPageCount, + key = pageKey ) { page -> PageHost(pages[page]) } @@ -601,7 +601,8 @@ class ComposePagerView(context: Context) : FrameLayout(context) { pageSpacing = pageSpacing, userScrollEnabled = scrollEnabledState.value, reverseLayout = false, - beyondViewportPageCount = beyondViewportPageCount + beyondViewportPageCount = beyondViewportPageCount, + key = pageKey ) { page -> PageHost(pages[page]) } @@ -621,7 +622,11 @@ class ComposePagerView(context: Context) : FrameLayout(context) { } }, update = { container -> - attachChild(container, child) + // Reads pages so this runs again when React moves a page: removePageAt detached it, + // and its keyed slot keeps the same view without recomposing. + if (child in pages) { + attachChild(container, child) + } } ) } diff --git a/e2e/tests/issues/issue-965-pages-before-current.e2e.ts b/e2e/tests/issues/issue-965-pages-before-current.e2e.ts new file mode 100644 index 00000000..ebd1c3a4 --- /dev/null +++ b/e2e/tests/issues/issue-965-pages-before-current.e2e.ts @@ -0,0 +1,96 @@ +import { freshAppBeforeEach, expect, test } from '../support/test.ts'; +import type { App, Screen } from '../support/test.ts'; +import { openExample } from '../support/app.ts'; + +freshAppBeforeEach(); + +// Android-only: iOS keeps the old index too, which #1078 addresses. +const options = { + tags: ['regression', 'issue-965', 'issue-808'], + platforms: ['android'], +}; + +/** Opens the repro and waits for the pager to report page C. */ +async function openRepro(app: App, screen: Screen): Promise { + await openExample(app, screen, 'Issue #965 Pages Before Current Repro'); + await expect(screen.getByTestId('issue-965-status')).toHaveText( + 'Selected: 2 (C) | Events: 1' + ); +} + +/** Taps a repro button and checks that page C is reported at `index`. */ +async function expectCAt( + screen: Screen, + button: string, + index: number +): Promise { + const status = `Selected: ${index} (C) | Events: 2`; + await screen.getByTestId(button).tap(); + await expect(screen.getByTestId('issue-965-status')).toHaveText(status); + await expect(screen.getByText('Page C')).toBeVisible(); + // No late second event. + await expect(screen.getByTestId('issue-965-status')).toHaveText(status); +} + +test( + 'inserting a page before the current one keeps it visible', + options, + async ({ app, screen }) => { + await openRepro(app, screen); + await expectCAt(screen, 'issue-965-insert-first', 3); + } +); + +test( + 'inserting a page at the current index keeps the current page', + options, + async ({ app, screen }) => { + await openRepro(app, screen); + await expectCAt(screen, 'issue-965-insert-before-c', 3); + } +); + +test( + 'removing a page before the current one keeps it visible', + options, + async ({ app, screen }) => { + await openRepro(app, screen); + await expectCAt(screen, 'issue-965-remove-first', 1); + } +); + +test( + 'removing pages up to the current index keeps it visible', + options, + async ({ app, screen }) => { + await openRepro(app, screen); + // C moves to index 0, and the old index 2 is now past the end. + await expectCAt(screen, 'issue-965-remove-first-two', 0); + } +); + +test( + 'moving the current page keeps it visible', + options, + async ({ app, screen }) => { + await openRepro(app, screen); + await expectCAt(screen, 'issue-965-move-c-first', 0); + } +); + +test( + 'moving a page next to the current one keeps it rendered', + options, + async ({ app, screen }) => { + await openRepro(app, screen); + // Fabric applies a move as a remove and an insert of the same view. + await expectCAt(screen, 'issue-965-move-c-last', 3); + await screen + .getByText('Page C') + .swipe({ direction: 'left', momentum: 'fast' }); + await expect(screen.getByTestId('issue-965-status')).toHaveText( + 'Selected: 2 (D) | Events: 3' + ); + await expect(screen.getByText('Page D')).toBeVisible(); + } +); diff --git a/example/src/App.tsx b/example/src/App.tsx index 4af71ee7..c86465f1 100644 --- a/example/src/App.tsx +++ b/example/src/App.tsx @@ -54,6 +54,7 @@ import { import { Issue1154PageChangeFocusRepro } from './gh-issues/Issue1154PageChangeFocusRepro'; import { Issue1156NestedHorizontalScrollRepro } from './gh-issues/Issue1156NestedHorizontalScrollRepro'; import { Issue424SwipeCancelsPressRepro } from './gh-issues/Issue424SwipeCancelsPressRepro'; +import { Issue965PagesBeforeCurrentRepro } from './gh-issues/Issue965PagesBeforeCurrentRepro'; import { colors, radius } from './theme'; function BasicPagerViewExampleScreen() { @@ -174,6 +175,10 @@ const sections: { title: string; data: Example[] }[] = [ component: Issue424SwipeCancelsPressRepro, name: 'Issue #424 Swipe Cancels Press Repro', }, + { + component: Issue965PagesBeforeCurrentRepro, + name: 'Issue #965 Pages Before Current Repro', + }, ], }, ]; diff --git a/example/src/gh-issues/Issue965PagesBeforeCurrentRepro.tsx b/example/src/gh-issues/Issue965PagesBeforeCurrentRepro.tsx new file mode 100644 index 00000000..a226fbe0 --- /dev/null +++ b/example/src/gh-issues/Issue965PagesBeforeCurrentRepro.tsx @@ -0,0 +1,90 @@ +/** + * Repro for #965 and #808 on Android: inserting, removing or moving pages + * before the current one must keep the current page visible and report its + * new index once through onPageSelected. The pager starts on page C. + */ +import React, { useRef, useState } from 'react'; +import { Button, StyleSheet, Text, View } from 'react-native'; +import PagerView from 'react-native-pager-view'; + +const INITIAL_PAGES = ['A', 'B', 'C', 'D']; + +export function Issue965PagesBeforeCurrentRepro() { + const [pages, setPages] = useState(INITIAL_PAGES); + const [selected, setSelected] = useState(0); + const [events, setEvents] = useState(0); + const insertedCount = useRef(0); + const nextPage = () => `N${insertedCount.current++}`; + const withoutC = (current: string[]) => + current.filter((page) => page !== 'C'); + const selectedPage = pages[selected] ?? '-'; + + return ( + + + {`Selected: ${selected} (${selectedPage}) | Events: ${events}`} + +