diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 0fffeedb..ae6d03ae 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -77,6 +77,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..b1ef8e7b 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ For more examples, see the [example app](example/src): nested pagers, keyboard h | `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 | +| `onPageSelected: (e: PagerViewOnPageSelectedEvent) => void` | Called when the pager moves to 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 | @@ -106,6 +106,8 @@ For more examples, see the [example app](example/src): nested pagers, keyboard h `PagerView` also accepts all standard `View` props. +Leaving a page blurs an input focused on it, whatever `keyboardDismissMode` is set to. On iOS the blur follows `onPageSelected`, including the one sent on mount, and waits for the pager to report that it has settled on the page it was heading for: an animated `setPage` blurs when its slide ends, and a swipe blurs once the gesture and the deceleration after it are over, so the keyboard never closes under the user's finger. Where that report never comes the blur runs about a second after the page change instead, which covers a `setPage` that interrupts one still running, and every animated `setPage` on iOS 15 and visionOS, where no scroll state is reported at all. Because the blur follows the event, it never takes the focus from an input an `onPageSelected` handler focused, though the keyboard can close and reopen on the way. On iOS that event arrives when the pager starts moving rather than when it settles, so a focus call made from the handler during an animated `setPage` can land on a page that is not on screen yet and do nothing; from iOS 16 on, focus from `onPageScrollStateChanged` once the state is `idle` where it has to hold, since iOS 15 and visionOS never report that state. + ### 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..69a88d3d 100644 --- a/e2e/tests/issues/issue-1154-page-change-focus.e2e.ts +++ b/e2e/tests/issues/issue-1154-page-change-focus.e2e.ts @@ -4,21 +4,38 @@ import { openExample } 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)); + +/** Matches the wait the iOS blur falls back on when nothing reports settling. */ +const PAGE_CHANGE_WAIT_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 scrolling. */ +async function expectIdle(screen: Screen): Promise { + await expect(screen.getByTestId('issue-1154-events')).toHaveText( + /state idle/ ); } @@ -26,6 +43,7 @@ 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'); @@ -33,50 +51,150 @@ async function focusInputOnPage( await screen.getByTestId(`issue-1154-go-${page}`).tap(); } 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. + await screen.getByTestId('issue-1154-go-1').tap(); + 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 expectIdle(screen); + await expectStatus(screen, platform, 2, 'none'); + } +); + +test( + 'a setPage repeated during the slide still blurs', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // The repeat used to cancel the wait, so the blur never ran at all. + await screen.getByTestId('issue-1154-go-1-twice').tap(); + 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, while the pager is still animating. + // Focused right after setPage, before the pager has settled, and checked + // again once it reports stopping: the first check holds a few milliseconds + // in, so a slide that then lands on page 2 would pass unseen. await screen.getByTestId('issue-1154-mode-onPress').tap(); - await screen.getByTestId('issue-1154-go-0').tap(); - await expectStatus(screen, 0, 'input 0'); + await screen.getByTestId('issue-1154-go-1').tap(); + await expectStatus(screen, platform, 1, 'input 1'); + await expectIdle(screen); + await expectStatus(screen, platform, 1, 'input 1'); + } +); + +// 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'); + } +); - // Focused from onPageSelected, once the pager has settled. +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); + + // The second command goes back a page, so an overshoot lands on page 0 + // where this can see it. The interrupted slide reports stopping for a page + // the pager is no longer heading to, so the blur comes from the wait + // running out and the status is checked after both. + await screen.getByTestId('issue-1154-go-2-then-1').tap(); + await expectIdle(screen); + await expectStatus(screen, platform, 1, '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 keep the focus', + { ...options, platforms: ['ios'] }, + async ({ app, screen, platform }) => { + await focusInputOnPage(app, screen, platform, 0); + + // The pager ends where it started, so it never moves and never reports + // settling. Nothing left page 0, so its input keeps focus, and the check + // comes after the wait has run out so the blur it schedules is included. + await screen.getByTestId('issue-1154-go-1-and-0').tap(); + await wait(PAGE_CHANGE_WAIT_MS + 300); + await expectStatus(screen, platform, 0, 'input 0'); } ); diff --git a/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx b/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx index edad1eaa..587412f6 100644 --- a/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx +++ b/example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx @@ -1,8 +1,8 @@ /** - * 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 { @@ -21,6 +21,7 @@ const PAGE_COLORS = ['#fde2e2', '#e2fde6', '#e2e8fd']; 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); @@ -32,6 +33,19 @@ export function Issue1154PageChangeFocusRepro() { const log = (event: string) => setEvents((current) => [...current.slice(-11), event]); + // Each page command starts a fresh log, so a check waiting for the pager to + // report stopping cannot match a report from an earlier command. + const startCommand = () => setEvents([]); + + 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); + }; + useEffect(() => { const show = Keyboard.addListener('keyboardDidShow', () => { setKeyboardVisible(true); @@ -64,6 +78,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 +87,27 @@ export function Issue1154PageChangeFocusRepro() { /> ))} + +