Repository navigation
fix(ios): clear focus and hide the keyboard when leaving a page - #1193
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Native responder and run-loop timing warrant human verification across supported iOS versions.
No open issues 🎉
What changed in this PR
Addresses the iOS counterpart of Android’s page-change focus fix, preventing off-screen inputs from retaining focus and keyboard input.
Changes:
- Defers focus cleanup after page selection while preserving focus on the selected page.
- Extends regression tests to iOS, including keyboard and refocus checks.
- Documents focus behavior and simulator keyboard setup.
| File | Description |
|---|---|
| README.md | Documents page-change blur behavior. |
| ios/PagerViewProvider.swift | Adds deferred responder cleanup and delegate forwarding. |
| example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx | Updates the reproduction comment to cover both platforms. |
| e2e/tests/issues/issue-1154-page-change-focus.e2e.ts | Adds iOS keyboard assertions and swipe refocus coverage. |
| e2e/README.md | Documents simulator software-keyboard setup. |
Review effort: Balanced
|
Hi @TaduJR, thanks for submitting the PR! While reviewing, I've found one potential regression connected to your changes. The reported issue seems to be fixed, but when testing imperative changes to the pages with "Go ..." buttons, it seems to cause some errors- a blank screen on one of the pages, or the wrong animation (fade-in instead of slide-in)
Seems to work fine with just swipes. Could you take a look at that issue & resolve the conflicts? Happy to proceed with the merge afterwards. |
|
Thanks for reviewing and catching the regression. Indeed it was from this PR. Addressed it. Please feel free to review it again. Thanks! |
|
@TaduJR could you resolve the conflicts? 🙏 |
Hi @TaduJR, thanks for the update. Unfortunately I was able to recreate the issue again, also it feels a bit laggy with the new timeout. Please let me know if it behaves differently on your end. Also there seem to be some failing checks on this PR. after.fix.2.bug.mp4 |
MarchewkaMatthew
left a comment
There was a problem hiding this comment.
Found one regression on the newest version of the PR
Replace the wait for the idle report and the one-second fallback with two fixed points: setPage blurs before it changes the selection, and a swipe blurs once its deceleration ends. Neither runs during the TabView slide, so the slide stays intact and the keyboard hides as the page changes.
|
@MarchewkaMatthew @TaduJR I'd like to propose alternative solution to this problem which doesn't rely on timers. Timers cause the lag Matthew reported, and they make blank page issue more frequent. Alternative is clearing focus right before the page change, so nothing happens mid-animation. The side effect is losing ability to keep input focused when changing page and immediately going back but this doesn't seem like a big deal. I've opened a PR targeting this branch from @TaduJR because I've reused all the e2e tests written and helpers in this PR and want to keep the commit crediting the author. |
…-page # Conflicts: # ios/PagerViewProvider.swift
|
Thanks, I've adopted it. The lag is gone @MarchewkaMatthew the blank page in your video is #1202 Could you take another look. TIA. |
|
Thanks @TaduJR, it looks good on my end! The blank page issue is gone, and the input keyboard is correctly blurred when changing the page. The only regression I've noticed is that imperative switching of pages with the keyboard open leads to no animation during page changes. @troZee @krozniata, could you take a look if it's a blocker for merging this change? after.fix.no.blank.bad.animation.mp4 |
|
@TaduJR I've looked into the missing animation. It happens when the pager resizes while it's sliding. That bug already exists on A middle ground is to blur only once the pager comes to rest, for
I haven't found a fix with no side effects yet. Happy to dig deeper if you'd like. |
|
@apex-sentinel-staging review |
KisaneNeko
left a comment
There was a problem hiding this comment.
LGTM, the issues with animation are related to keyboard dismiss itself, not with the changes in the PR. This PR will make it more apparent but I'd suggest creating follow-up issue to investigate the actual root cause instead of trying to handle it within this PR.
Apex SentinelVerdict: 4 finding(s): 2 minor, 2 nit. Verdict: approve. reached max rounds (1). 🟡 Minor (2)
🔵 Nit (2)
Reply under an inline comment with |
There was a problem hiding this comment.
Apex Sentinel: 4 new findings on fb0be73. The verdict and the full list are in the summary comment.
| scrollDelegate.delegate = delegate | ||
| // VTabView-style rotation preserves TabView's horizontal collection view. | ||
| scrollDelegate.orientation = .horizontal | ||
| scrollDelegate.onRest = { [props] restedPage in |
There was a problem hiding this comment.
🟡 Minor · Rest detection blurs an input that is not on the page being left (no previously-selected page tracked)
onRest only compares the resting page against the current selection before blurring everything else: guard page == props.currentPage else { return } then props.resignFirstResponder(outsidePage: page). There is no check that a page was actually left, and nothing on the rest path mirrors the index != props.currentPage guard that PagerViewProvider.setPage has. So any focus taken on a page other than the selected one is killed the next time the pager settles on a boundary. Concretely, with keyboardDismissMode: 'interactive' the user drags the pager a few points — the selection changes as the swipe commits — then pulls back to the same page and lifts the finger: scrollViewDidEndDragging(willDecelerate: false) calls notifyRestOnPageBoundary(fromScrollEnd: true), which fires unconditionally (the fromScrollEnd argument bypasses the lastRestedPage dedupe), the guard passes because the pager rests on the page the selection now names, and the input the user is typing into on the previous page is resigned and the keyboard closes even though the pager is back where it started. The same happens for setPage(i) to the page already shown followed by any rest event, which the setPage guard deliberately allows. The pre-change code never blurred on rest, so this is new behavior introduced by these lines.
Suggested change
Remember the page the blur last acted for (both in setPage and in onRest) and skip the rest-path blur when the pager comes to rest on that same page — i.e. blur on rest only when the selected page actually changed since the last rest, so a committed-then-cancelled swipe or a setPage to the current page does not take focus away.
Details
Also raised here
Also raised at this location by swift-ios: setPageWithoutAnimation blurs twice — For a non-animated command, setPage sets props.currentPage directly and SwiftUI jumps the collection view, so UIKit delivers scrollViewDidEndScrollingAnimation for the programmatic offset change; emitIdleWithCleanOffset already exists at that point, and now notifyRestOnPageBoundary(scrollView, fromScrollEnd: true) runs right after it. Because the fromScrollEnd argument skips the lastRestedPage != page dedupe, onRest fires a second time for the same page and props.resignFirstResponder(outsidePage:) runs again a second after the blur in setPage (in the same example: the issue-1154-go-1-and-0 case). React also receives a second idle/scroll pair, which was already the case, so this adds a second native resign on top. …
persona: swift-ios
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| lastRestedPage = nil | ||
| return | ||
| } | ||
| let page = Int(round(contentOffset / pageSize)) |
There was a problem hiding this comment.
🟡 Minor · lastRestedPage is never reset when a new slide starts, so the arrival can go unreported
lastRestedPage is only cleared in notifyRestOnPageBoundary, and only when a scroll event happens to land between pages. The guard fromScrollEnd || lastRestedPage != page else { return } dedupe then applies to a page reached by a different command: if a setPage slide ends exactly on the page that was already reported by the previous command's rest (a command that redirects back a page, a no-op setPage to the page already resting — the issue-1154-go-1-and-0 and go-1-twice cases), the scrollViewDidScroll events of the interrupted slide all reported page X as the resting point, and the final scrollViewDidEndScrollingAnimation for that same X is dropped by the guard. With it dropped, the pending blur held by scrollDelegate.onRest/PagerView never runs and the input on the page just left keeps focus — the bug this PR fixes. Resetting lastRestedPage = nil in scrollViewWillBeginDragging (and at the entry of the programmatic-scroll path, e.g. scrollViewWillBeginDecelerating) would scope the dedupe to one slide.
Suggested change
Scope the lastRestedPage dedupe to a single slide: clear it when a new interaction or programmatic scroll begins (scrollViewWillBeginDragging / scrollViewWillBeginDecelerating), so the scroll-end callback of a later command to the same page is not silenced.
Details
persona: maintainability
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
| guard children.indices.contains(page) else { | ||
| return | ||
| } | ||
| for (index, child) in children.enumerated() where index != page { |
There was a problem hiding this comment.
🔵 Nit · resignFirstResponder(outsidePage:) stops at the first page holding a first responder, so an input on a later page is left focused
The loop returns as soon as it meets the first in-window page that holds the first responder, whether or not that responder is an input. If that responder is a non-text first responder — the very case the comment carves out ("A view that takes no text, such as one holding key commands, is not an input") — the function bails out instead of continuing, so a focused TextInput on a later neighbour page is never resigned. The paged TabView keeps neighbours in the window (the doc comment says so), so with page 0 leaving, a key-command/focus-engine first responder on page 1 and a focused TextInput on page 2, the keyboard stays up and the off-screen input keeps focus — exactly the bug this PR fixes. continue after the non-input check (and return only once a responder has been resigned, if the intent is one-per-call) keeps the "only inputs are affected" rule while still scanning the remaining pages.
| for (index, child) in children.enumerated() where index != page { | |
| for (index, child) in children.enumerated() where index != page { | |
| guard child.view.window != nil, | |
| let responder = child.view.firstResponderInHierarchy else { | |
| continue | |
| } | |
| // A view that takes no text, such as one holding key commands, is not an | |
| // input, and one that brings its own keyboard is. | |
| guard responder is UIKeyInput | |
| || responder.inputView != nil | |
| || responder.inputViewController != nil else { | |
| continue | |
| } | |
| responder.resignFirstResponder() | |
| return | |
| } |
Suggested change
Scan every page other than the target rather than stopping at the first page that happens to hold a first responder: continue when the responder is not an input, so a focused input on a later neighbour page is still blurred.
Details
Also raised here
Also raised at this location by maintainability: resignFirstResponder(outsidePage:) stops at the first child that has a window, so a responder deeper in the list is never found — The return at the end of the loop body ends the walk as soon as one child other than page satisfies both child.view.window != nil and a non-nil firstResponderInHierarchy. The two conditions are checked as one guard, so the first visible page that contains the first responder ends the search even when that responder is a non-text responder (a container that is firstResponder for key commands) — the very case the UIKeyInput check below is written to leave alone. In RTL the enumeration order (children, i.e. declaration order) is the reverse of the physical page order page was derived from, so which visible page is visited first changes with layout direction. Walking all children and only breaking after a real input was resigned keeps the "only inputs are touched" promise independent of child order.
Also raised at this location by javascript: resignFirstResponder(outsidePage:) scans every sibling page for the first responder on every setPage, defeating its own "leave non-inputs alone" rule when an input is on a later page — PagerViewProps.resignFirstResponder(outsidePage:) walks children and stops (return inside the loop body, line 56) at the first child that is in a window and has any first responder in its hierarchy — not at the first child holding a text input. The loop therefore only ever inspects one child, and which one it inspects is the lowest-index page whose subtree happens to hold UIKit's first responder, while the UIKeyInput / inputView / inputViewController check on lines 51-53 only decides whether that one candidate is resigned. …
persona: maintainability, javascript, senior-developer
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
|
|
||
| /// A slide cut short by another command stops between pages, which is not a | ||
| /// rest: the pager is still on its way somewhere. | ||
| private func notifyRestOnPageBoundary(_ scrollView: UIScrollView, fromScrollEnd: Bool = false) { |
There was a problem hiding this comment.
🔵 Nit · Rest detection keys page size off scrollView.frame.width, which only matches the cell pitch by coincidence (pageMargin is applied in JS)
notifyRestOnPageBoundary (and the existing scrollViewDidScroll) computes pageSize as scrollView.frame.width/frame.height and derives the resting page from contentOffset.truncatingRemainder(dividingBy: pageSize) plus Int(round(contentOffset / pageSize)). The new code makes that assumption load-bearing for a blur decision rather than just for a reported offset: it decides whether the pager "came to rest on a page boundary" and therefore whether focus is dropped. pageMargin is not applied natively at all — grep over ios/ finds no pageMargin; it is emulated in JS (src/PagerView.tsx:156-169 and src/utils.ios.tsx:6-14 add marginHorizontal: ±pageMargin/2 to the page content), so the collection view's cell pitch only happens to equal the scroll view's own width while pageMargin is left at its default of 0. As soon as the cell pitch diverges from frame.width (a native pageMargin implementation, a change to the JS margin emulation, or any SwiftUI page inset), offsetInPage < 1 || pageSize - offsetInPage < 1 fails at every genuine rest, lastRestedPage is reset to nil on line 81, and onRest never fires — so a swipe silently stops blurring the input and the keyboard stays up again, which is exactly the regression this PR is fixing, with no assertion or test that pins the invariant (the RTL/blur e2e cases assert the behaviour only for the default margin). Read the pitch from the layout (e.g. the collection view's page item size / collectionViewLayout or contentSize / pageCount) instead of the scroll view's frame, or assert the equality.
Suggested change
Derive the page pitch from the pager's layout (item size / contentSize divided by page count) rather than scrollView.frame, so the boundary test stays correct if pageMargin or the page insets stop being emulated in JavaScript.
Details
Also raised here
Also raised at this location by maintainability: Page-boundary arithmetic ignores pageMargin and assumes a single-width stride — pageMargin is a documented, both-platform prop: any non-zero margin makes the stride pageSize + pageMargin, so the remainder test never matches and onRest never fires for that pager — a silent loss of the swipe blur rather than a wrong page. The same assumption is made silently in the RTL mirror in PagerView.swift.
persona: maintainability, javascript
Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.
Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.
…hen-leaving-a-page # Conflicts: # README.md
# Conflicts: # example/src/gh-issues/Issue1154PageChangeFocusRepro.tsx
Summary
Closes #1192. This is the iOS half of the problem that was fixed for Android in #1155 and shipped in 9.0.8: an input kept focus when the pager left its page, so the keyboard stayed up and typing went into a field that was no longer on screen.
setPagenow clears the focus just before it changes the selection. Clearing it afterwards does not work: resigning the input forces a layout pass, and SwiftUI then applies the pending selection twice, so the pager lands a page past the target. That was a real regression in an earlier version of this PR, and it returns even if the blur is deferred by a single pass of the main queue, so the order is deliberate.A swipe clears the focus once the pager stops, so the keyboard never closes while a finger is still on the screen. "Stopped" means the scroll offset is resting on a page boundary and that page is the one currently selected, so a boundary the pager is only passing through does not count. This reads the offset instead of waiting for a notification because iOS does not send a usable one when a second
setPageinterrupts a slide: the last end-of-scroll callback arrives while the pager is still tens of points short of its target, and nothing follows once it lands.Only inputs are affected, so a view that cannot take text, such as one holding key commands, is left alone.
Documented limits, all in
README.md:setPageclears focus, since a swipe's stop is not reported there.onPageSelectedduring an animatedsetPagedoes nothing, because the page is not on screen yet.setPagethat moves the pager back a page does not take effect.mainlands that focus today, so this is the one behaviour change here focus from theidlereport instead. I could not find a placement for the blur that keeps both this and the immediate dismissal, so if you would rather keep the focus, I will move it.The
onPageSelectedrow also said the event fires once the pager settles. That holds on Android but not on iOS, where it arrives before the pager settles, so the row now states both.Test Plan
What's required for testing (prerequisites)?
Or
bun run e2e:ios, which bundles, installs the release app and runs the iOS suite in one go.The keyboard assertions need the simulator to show its own keyboard. A device that has seen a Mac keyboard once will not, so clear both flags on every booted device after building (also written up in
e2e/README.md):CI runners boot fresh devices and do not need this there is a comment in
ios.ymlsaying so.What are the steps to reproduce (after prerequisites)?
By hand, in the example app, on Issue #1154 Page Change Focus Repro:
Focused: input 0; now it readsSelected: 1 | Focused: none | Keyboard: hidden.Automated:
15 cases, 13 of which run on iOS, covering: leaving a page by
setPageand by swipe, landing on the target one and two pages away, a repeated command during a slide, two commands that cancel out, one command interrupting another in both directions, a redirect arriving after the focus has landed, focus kept when the new page takes it, a right-to-left swipe, and a page that grows with the keyboard so its layout changes exactly as the slide starts.Results on an iPhone 18 Pro simulator, iOS 27, React Native 0.88: 13 of 13 on three consecutive runs. The full iOS suite is 34 of 35. The failure is
issue-1096-keyboard-shrink, where the test engine refuses to tap a target behind a visible keyboard. It passes in other runs of the same build and fails the same way with this change reverted.Compatibility
Android needs nothing: the same fix shipped there in 9.0.8 via #1155.
Checklist
README.md