diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 4bb12959..5cf6634d 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -80,6 +80,8 @@ jobs: # Simulator.app attaches the Mac keyboard by default, which hides the # software keyboard and leaves no Return key for the keyboard tests. + # A fresh runner's own devices have never seen a Mac keyboard, so they do + # not need the per-device flags that e2e/README.md gives for local runs. - name: Detach hardware keyboard from simulator run: defaults write com.apple.iphonesimulator ConnectHardwareKeyboard -bool false diff --git a/README.md b/README.md index 6edcd9f9..23c9e543 100644 --- a/README.md +++ b/README.md @@ -89,23 +89,25 @@ For more examples, see the [example app](example/src): nested pagers, keyboard h ### Props -| Prop | Description | Platform | -| -------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | :------: | -| `initialPage: number` | Index of the page that is selected on mount. | both | -| `scrollEnabled: boolean` | Whether the user can swipe between pages. Defaults to `true`. | both | -| `orientation: 'horizontal' \| 'vertical'` | Scroll direction. Defaults to `horizontal`. Changing it after mount is not supported. | both | -| `layoutDirection: 'ltr' \| 'rtl' \| 'locale'` | Layout direction. Use `ltr` or `rtl` to set it explicitly, or `locale` to follow `I18nManager.isRTL`. Defaults to `locale`. | both | -| `pageMargin: number` | Blank space between pages. | both | -| `keyboardDismissMode: 'none' \| 'on-drag'` | Whether the keyboard is dismissed when the user drags the pager. Defaults to `none`. | both | -| `onPageScroll: (e: PagerViewOnPageScrollEvent) => void` | Called while moving between pages, either from a swipe or from an animated `setPage`. `e.nativeEvent` contains `position` and `offset`. | both | -| `onPageSelected: (e: PagerViewOnPageSelectedEvent) => void` | Called once the pager settles on a page, and once on mount for `initialPage`. `e.nativeEvent.position` is the selected index. | both | -| `onPageScrollStateChanged: (e: PageScrollStateChangedNativeEvent) => void` | Called when the scroll state changes. `e.nativeEvent.pageScrollState` is `idle`, `dragging` or `settling`. | both | -| `overdrag: boolean` | Allows bouncing past the first and last page. Defaults to `false`. | iOS | -| `offscreenPageLimit: number` | Number of pages kept composed on each side of the visible page. When unset, the Compose pager default is used. | Android | -| `overScrollMode: 'auto' \| 'always' \| 'never'` | Overscroll effect at the edges. `never` disables the effect. Defaults to `auto`. | Android | +| Prop | Description | Platform | +| -------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :------: | +| `initialPage: number` | Index of the page that is selected on mount. | both | +| `scrollEnabled: boolean` | Whether the user can swipe between pages. Defaults to `true`. | both | +| `orientation: 'horizontal' \| 'vertical'` | Scroll direction. Defaults to `horizontal`. Changing it after mount is not supported. | both | +| `layoutDirection: 'ltr' \| 'rtl' \| 'locale'` | Layout direction. Use `ltr` or `rtl` to set it explicitly, or `locale` to follow `I18nManager.isRTL`. Defaults to `locale`. | both | +| `pageMargin: number` | Blank space between pages. | both | +| `keyboardDismissMode: 'none' \| 'on-drag'` | Whether the keyboard is dismissed when the user drags the pager. Defaults to `none`. | both | +| `onPageScroll: (e: PagerViewOnPageScrollEvent) => void` | Called while moving between pages, either from a swipe or from an animated `setPage`. `e.nativeEvent` contains `position` and `offset`. | both | +| `onPageSelected: (e: PagerViewOnPageSelectedEvent) => void` | Called when the pager moves to a page: on Android once it settles, and on iOS before it does, as an animated `setPage` starts, as a swipe commits to a page, or at once for `setPageWithoutAnimation`. Also called on mount for `initialPage`. `e.nativeEvent.position` is the selected index. | both | +| `onPageScrollStateChanged: (e: PageScrollStateChangedNativeEvent) => void` | Called when the scroll state changes. `e.nativeEvent.pageScrollState` is `idle`, `dragging` or `settling`. | both | +| `overdrag: boolean` | Allows bouncing past the first and last page. Defaults to `false`. | iOS | +| `offscreenPageLimit: number` | Number of pages kept composed on each side of the visible page. When unset, the Compose pager default is used. | Android | +| `overScrollMode: 'auto' \| 'always' \| 'never'` | Overscroll effect at the edges. `never` disables the effect. Defaults to `auto`. | Android | `PagerView` also accepts all standard `View` props. +Leaving a page blurs an input focused on it, whatever `keyboardDismissMode` is set to. On iOS an animated `setPage` blurs the input before the slide starts, and a swipe blurs it once the pager comes to rest, so the keyboard never closes under the user's finger. A swipe reports coming to rest from iOS 16 on, so on iOS 15 and visionOS only `setPage` blurs. A handler that focuses an input from `onPageSelected` keeps it after a swipe, though the keyboard closes and reopens on the way; during an animated `setPage` iOS reports the selection before the page is on screen, so focusing there does nothing and the page ends with nothing focused. A focus sent in the same tick as a `setPage` that moves the pager back a page does not take effect on iOS either, so focus the new page's input once the pager reports `idle` instead. + ### Methods | Method | Description | Platform | diff --git a/e2e/README.md b/e2e/README.md index 464af1f6..9d09d5b0 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -40,6 +40,19 @@ bun run e2e:test:android From this directory, `bun run test:ios -- --tag smoke` runs only the smoke test, and `bun run list` prints every test without running it. +### Software keyboard on iOS + +The keyboard tests (`issue-1096-keyboard-shrink` and `issue-1154-page-change-focus`) need the simulator to show its own keyboard. A device that has seen a Mac keyboard once shows none again, across boots, and `Simulator.app`'s `ConnectHardwareKeyboard` preference does not reach a device the engine booted itself. Clear the device's own two flags, on every booted device, after building and before the run, since a build and launch can set them again: + +```bash +for udid in $(xcrun simctl list devices booted | grep -Eo '[0-9A-F-]{36}'); do + xcrun simctl spawn "$udid" defaults write com.apple.keyboard.preferences HardwareKeyboardLastSeen -bool false + xcrun simctl spawn "$udid" defaults write com.apple.keyboard.preferences AutomaticMinimizationEnabled -bool false +done +``` + +`AutomaticMinimizationEnabled` matters as much as the other: with it set, the keyboard stays minimised to the shortcut bar and React Native reports it hidden while an input is focused. + ### Several simulators at once With nothing pinned, the engine uses every booted simulator as the pool, so diff --git a/e2e/tests/issues/issue-1154-page-change-focus.e2e.ts b/e2e/tests/issues/issue-1154-page-change-focus.e2e.ts index ffcff23b..33ccae8b 100644 --- a/e2e/tests/issues/issue-1154-page-change-focus.e2e.ts +++ b/e2e/tests/issues/issue-1154-page-change-focus.e2e.ts @@ -1,24 +1,41 @@ import { freshAppBeforeEach, expect, test } from '../support/test.ts'; import type { App, Screen } from '../support/test.ts'; -import { openExample } from '../support/app.ts'; +import { openExample, restoreLtrAfterEach } from '../support/app.ts'; freshAppBeforeEach(); -// Android-only: on iOS the pager never kept focus on the page being left. -const options = { tags: ['regression', 'issue-1154'], platforms: ['android'] }; +const options = { tags: ['regression', 'issue-1154'] }; + +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +/** Longer than the 0.8s the default spring takes to settle. */ +const SETTLE_MS = 1000; /** - * Checks the selected page and the focused input. The keyboard part of the - * status line is not checked: agent-device swaps in its own headless IME for - * the run, so React Native never reports the keyboard as shown. + * Checks the selected page, the focused input, and the keyboard, which follows + * the focus. The keyboard is checked on iOS only: Android's headless IME never + * reports it. An iOS failure reading `Keyboard: hidden` means the simulator is + * showing no software keyboard, which the e2e README's device flags fix. */ async function expectStatus( screen: Screen, + platform: string, page: number, focused: string ): Promise { + const keyboard = + platform === 'ios' + ? ` Keyboard: ${focused === 'none' ? 'hidden' : 'visible'}` + : ''; await expect(screen.getByTestId('issue-1154-status')).toHaveText( - new RegExp(`^Selected: ${page} \\| Focused: ${focused} \\|`) + new RegExp(`^Selected: ${page} \\| Focused: ${focused} \\|${keyboard}`) + ); +} + +/** Waits for the pager to report that it has stopped on `page`. */ +async function expectIdleAt(screen: Screen, page: number): Promise { + await expect(screen.getByTestId('issue-1154-events')).toHaveText( + new RegExp(`state idle at ${page}`) ); } @@ -26,57 +43,267 @@ async function expectStatus( async function focusInputOnPage( app: App, screen: Screen, + platform: string, page: number ): Promise { await openExample(app, screen, 'Issue #1154 Page Change Focus Repro'); if (page !== 0) { await screen.getByTestId(`issue-1154-go-${page}`).tap(); + await expectIdleAt(screen, page); } await screen.getByTestId(`issue-1154-input-${page}`).tap(); - await expectStatus(screen, page, `input ${page}`); + await expectStatus(screen, platform, page, `input ${page}`); } test( 'leaving a page with setPage blurs its input', options, - async ({ app, screen }) => { - await focusInputOnPage(app, screen, 1); + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 1); // Before the fix the off-screen input kept focus. await screen.getByTestId('issue-1154-go-0').tap(); - await expectStatus(screen, 0, 'none'); + await expectStatus(screen, platform, 0, 'none'); + } +); + +test( + 'setPage with an input focused lands on the target page', + options, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // Blurring during the slide used to land the pager on page 2, so the page + // is checked once the pager reports coming to rest. + await screen.getByTestId('issue-1154-go-1').tap(); + await expectIdleAt(screen, 1); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 1, 'none'); + } +); + +test( + 'a setPage two pages away lands on the far page', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // Two pages at once, so the page being left can drop out of the window + // while the slide runs and UIKit resigns the input itself. + await screen.getByTestId('issue-1154-go-2').tap(); + await expectIdleAt(screen, 2); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 2, 'none'); + } +); + +test( + 'a setPage repeated during the slide changes nothing', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // The second command names the page already arriving, so it moves + // nothing and must not disturb the slide or the blur. + await screen.getByTestId('issue-1154-go-1-twice').tap(); + await expectIdleAt(screen, 1); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 1, 'none'); } ); test( 'leaving a page with a swipe blurs its input', options, - async ({ app, screen }) => { - await focusInputOnPage(app, screen, 0); + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); - // Swipe on the page title: a swipe that starts on the input is taken by - // the input instead of the pager. + // Swipe the title: a swipe starting on the input is taken by the input. await screen .getByTestId('issue-1154-label-0') - .swipe({ direction: 'right' }); - await expectStatus(screen, 1, 'none'); + .swipe({ direction: 'right', momentum: 'fast' }); + await expectStatus(screen, platform, 1, 'none'); } ); test( 'an input focused on the new page keeps focus', options, - async ({ app, screen }) => { - await focusInputOnPage(app, screen, 1); + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // Focused right after setPage, before the pager has settled. The status + // reads right as the slide starts, so the second check waits out the + // settle, where a late blur or an overshoot would show. The keyboard + // reopening ends this slide's scroll early, at page 0, so its idle report + // is no use as a signal here. + await screen.getByTestId('issue-1154-mode-onPress').tap(); + await screen.getByTestId('issue-1154-go-1').tap(); + await expectStatus(screen, platform, 1, 'input 1'); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 1, 'input 1'); + } +); + +test( + 'an input focused on the new page keeps focus going back', + { ...options, platforms: ['android'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 1); - // Focused right after setPage, while the pager is still animating. + // The same case backwards, which iOS drops: a focus sent in the same tick + // as the command does not reach the page the pager moves back to, with or + // without an input focused first. Forward it survives. await screen.getByTestId('issue-1154-mode-onPress').tap(); await screen.getByTestId('issue-1154-go-0').tap(); - await expectStatus(screen, 0, 'input 0'); + await expectStatus(screen, platform, 0, 'input 0'); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 0, 'input 0'); + } +); + +// A focus call from an `onPageSelected` handler is covered for a swipe only: +// iOS reports the selection as an animated `setPage` starts, when the +// destination page can still be out of the window, where focusing does nothing. + +test( + 'a swipe keeps the focus the new page takes from onPageSelected', + options, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + await screen.getByTestId('issue-1154-mode-onSelected').tap(); + await screen + .getByTestId('issue-1154-label-0') + .swipe({ direction: 'right', momentum: 'fast' }); + await expectStatus(screen, platform, 1, 'input 1'); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 1, 'input 1'); + } +); + +test( + 'a setPage that interrupts another lands on the page asked for last', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); - // Focused from onPageSelected, once the pager has settled. + // The second command goes back a page, so an overshoot lands on page 0 + // where this can see it. The interrupted slide's idle names whichever page + // it had reached by then, so the wait is the settle rather than that + // report. + await screen.getByTestId('issue-1154-go-2-then-1').tap(); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 1, 'none'); + } +); + +test( + 'a setPage that interrupts another still lands on the later page', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // The first command focuses page 1's input as it leaves page 0, and the + // second one redirects to page 2 while that slide runs, so the focus lands + // on a page the pager has already left. An overshoot lands on page 3. The + // landing of a redirected slide reports no idle, so the wait is the settle + // and the page comes from the selection, which an overshoot changes. + await screen.getByTestId('issue-1154-mode-onPress').tap(); + await screen.getByTestId('issue-1154-go-1-then-2').tap(); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 2, 'none'); + } +); + +test( + 'a setPage redirected once the focus has landed blurs it', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // The focus on page 1 lands before the redirect to page 2 arrives, so the + // blur before that page change is the one that resigns it, in the middle + // of a slide, where resigning used to break the slide. An overshoot lands + // on page 3, which the selection would report. + await screen.getByTestId('issue-1154-go-1-then-2-late').tap(); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 2, 'none'); + } +); + +test( + 'setPage keeps the focus the new page takes from onPageSelected', + { ...options, platforms: ['android'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // Android reports the selection once the pager has settled, so a handler + // can focus the new page there. iOS reports it as the slide starts, where + // the page can still be out of the window and focus does nothing. await screen.getByTestId('issue-1154-mode-onSelected').tap(); await screen.getByTestId('issue-1154-go-1').tap(); - await expectStatus(screen, 1, 'input 1'); + await expectStatus(screen, platform, 1, 'input 1'); + } +); + +test( + 'two setPage calls that cancel out land on the first page', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // Each setPage blurs before the pager moves, so the first one has already + // taken the focus by the time the second one sends the pager back. The + // pager never moves here, so there is no report of it coming to rest to + // wait for. + await screen.getByTestId('issue-1154-go-1-and-0').tap(); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 0, 'none'); + } +); + +test.describe('right to left', () => { + restoreLtrAfterEach(); + + test( + 'leaving a page with a swipe blurs its input', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await openExample( + app, + screen, + 'Issue #1154 Page Change Focus Repro', + 'rtl' + ); + await screen.getByTestId('issue-1154-input-0').tap(); + await expectStatus(screen, platform, 0, 'input 0'); + + // The pages are mirrored, so the gesture is reversed and the page the + // pager reports at rest counts from the other end. + await screen + .getByTestId('issue-1154-label-0') + .swipe({ direction: 'left', momentum: 'fast' }); + await expectStatus(screen, platform, 1, 'none'); + } + ); +}); + +test( + 'a page that grows with the keyboard still lands on the target page', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await openExample(app, screen, 'Issue #1154 Page Change Focus Repro'); + await screen.getByTestId('issue-1154-avoid').tap(); + await screen.getByTestId('issue-1154-input-0').tap(); + await expectStatus(screen, platform, 0, 'input 0'); + + // The blur closes the keyboard before the slide starts, so the page + // relayouts as the pager begins to move, which must not move the pager + // anywhere but page 1. + await screen.getByTestId('issue-1154-go-1').tap(); + await expectIdleAt(screen, 1); + await wait(SETTLE_MS); + await expectStatus(screen, platform, 1, 'none'); } ); diff --git a/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx b/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx index edad1eaa..db79c414 100644 --- a/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx +++ b/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx @@ -1,13 +1,14 @@ /** - * Repro for #1154: on Android, a focused TextInput must lose focus and hide - * the keyboard when its page is left, via setPage or a swipe. The mode buttons - * focus the new page's input from onPageSelected or right after setPage, which - * must keep working. + * Repro for #1154: a focused TextInput must lose focus and hide the keyboard + * when its page is left, via setPage or a swipe. The mode buttons focus the + * new page's input from onPageSelected or right after setPage, which must + * keep working. */ import React, { useEffect, useRef, useState } from 'react'; import { Button, Keyboard, + KeyboardAvoidingView, StyleSheet, Text, TextInput, @@ -16,11 +17,12 @@ import { } from 'react-native'; import PagerView from 'react-native-pager-view'; -const PAGES = ['Page 0', 'Page 1', 'Page 2']; -const PAGE_COLORS = ['#fde2e2', '#e2fde6', '#e2e8fd']; +const PAGES = ['Page 0', 'Page 1', 'Page 2', 'Page 3']; +const PAGE_COLORS = ['#fde2e2', '#e2fde6', '#e2e8fd', '#fdf3e2']; export function Issue1154PageChangeFocusRepro() { const pagerRef = useRef(null); + const followUpCommand = useRef<{ at: number; go: number } | null>(null); const [selectedPage, setSelectedPage] = useState(0); const [focusedInput, setFocusedInput] = useState(null); const [keyboardVisible, setKeyboardVisible] = useState(false); @@ -28,10 +30,37 @@ export function Issue1154PageChangeFocusRepro() { const [autoFocusMode, setAutoFocusMode] = useState< 'off' | 'onSelected' | 'onPress' >('off'); + const [avoidsKeyboard, setAvoidsKeyboard] = useState(false); const inputRefs = useRef>([]); + const lastRest = useRef(0); + const lateCommand = useRef | null>(null); const log = (event: string) => setEvents((current) => [...current.slice(-11), event]); + // Each page command starts a fresh log and drops a follow-up left armed, so + // a check waiting for the pager to report stopping cannot match a report + // from an earlier command. + const startCommand = () => { + setEvents([]); + followUpCommand.current = null; + if (lateCommand.current) { + clearTimeout(lateCommand.current); + lateCommand.current = null; + } + }; + + const goToPageThen = (first: number, second: number) => { + startCommand(); + // A command for the page already shown sends no event, so a follow-up + // armed for it would wait until some later visit and bounce the pager. + followUpCommand.current = + selectedPage === first ? null : { at: first, go: second }; + pagerRef.current?.setPage(first); + if (autoFocusMode === 'onPress') { + inputRefs.current[first]?.focus(); + } + }; + useEffect(() => { const show = Keyboard.addListener('keyboardDidShow', () => { setKeyboardVisible(true); @@ -44,6 +73,9 @@ export function Issue1154PageChangeFocusRepro() { return () => { show.remove(); hide.remove(); + if (lateCommand.current) { + clearTimeout(lateCommand.current); + } }; }, []); @@ -64,6 +96,7 @@ export function Issue1154PageChangeFocusRepro() { testID={`issue-1154-go-${i}`} title={`Go ${i}`} onPress={() => { + startCommand(); pagerRef.current?.setPage(i); if (autoFocusMode === 'onPress') { inputRefs.current[i]?.focus(); @@ -72,6 +105,48 @@ export function Issue1154PageChangeFocusRepro() { /> ))} + +