fix(ios): improve focus requests and focus-state handling - #126
SynurDevelopers wants to merge 9 commits into
Conversation
…try on attach UIKit honors setNeedsFocusUpdate only when the environment contains the currently focused item, so requests on nearest-ancestor controllers were silently dropped (nested controllers, react-native-screens). All focus requests now go through RNCEKVKeyboardFocusService, preferring the key window root. Imperative focus() on a detached view records a pending request replayed on window attach. Also collapses the duplicated autofocus branches in didMoveToWindow, revalidates the deferred autofocus block against a generation counter after recycling, and enables the directional order guides via a descendant check instead of a first-subview pointer comparison. The text-input wrapper additionally gains the hasOnFocusChanged emission gate ahead of the base-class gate removal in the focus-event commit; the interim tree is double-gated with unchanged behavior.
All focus delegates held plain-strong back-pointers to their host views while the views strongly own the delegates, making every keyboard view immortal; the back-pointers are now zeroing-weak. The view controller's custom focus view association is stored through a weak holder, skipped and self-cleared when off-window, so controllers no longer retain unmounted subtrees or steer later focus updates to stale views. Group entry/exit boundary views latched in the order-linking singleton are now weak.
FocusTrap now diffs forceLock/lockDisabled and requests focus only when the lock becomes active, so prop commits and disable transitions no longer steal keyboard or VoiceOver focus. Tabbing into an ordered group suppresses UIKit's default move instead of double-focusing, and latched entry/exit boundaries are revalidated against the window before use. The native focus handler chain (context-menu registration) no longer depends on a JS listener being attached — only event emission is gated. Blur is emitted even after the tracked focused child deallocates, and moves between descendants of one wrapper no longer emit duplicate focus events.
Hydrates the dormant ExternalKeyboardExampleTests target with unit tests covering focus change events, focus delegates, keyboard focus service, lock view, and retain cycles. Adds a setup script to enable coverage on the test run and updates the Podfile/project to wire the target into the example workspace.
… window root - Readiness for imperative keyboard focus is now window-based: the request base parks when the controller or window is missing, and the text-input wrapper additionally parks until its native child exists; both replay from didMoveToWindow. - RNCEKVKeyboardFocusService resolves the routing controller from the target view's own window root first, then the key-window root, then the supplied fallback, and returns the controller it routed to. - Detach and cleanReferences now clear the controller's preferred-focus target when it still points at the view's own request, so recycled or navigated-away views cannot be revived as stale preferred targets. - screenReaderFocus gets the same park-and-replay as keyboard focus, so both halves of the JS focus() call survive a pre-attach request. - A same-generation autofocus that is skipped while detached returns its attempt, so the next attach retries instead of losing autofocus. - The text-input wrapper now inherits the focus delegate's tracked focus state machine (single focus per wrapper-level entry, blur after the tracked child deallocates) instead of the descendant-only checks.
…roup endpoint cleanup - Both lock-view request guards now reject any inactive or disabled state (matching onAccessibilityFocusChanged:), and the Fabric prop diff applies lockDisabled before forceLock so a compound activate-and-disable commit never passes through a momentarily-active state. didMoveToWindow doubles as the attach replay for an active trap whose early request had no controller. - Directional-guide enablement uses nearest-wrapper ownership: a nested order-group wrapper (or a focused wrapper itself) owns its focus, so a parent's guides no longer activate for a nested wrapper's focus. - RNCEKVOrderRelationship.clear also nils its entry/exit endpoints, so emptying a group releases its cached boundaries on the unlink path.
The branch was developed and validated with a full XCTest suite (97 unit tests over the changed focus-path methods, plus coverage and mutation scoring). It lives in history: 6d0b46a adds the suite and test target, 1bfc9c9 and 94115dc extend it alongside the fixes they verify. Since the project has no existing native test infrastructure, this commit removes the suite, the test-target project changes, and the Podfile/Podfile.lock edits from the PR tip to keep the reviewable diff limited to the library sources. Revert this commit to restore the complete, passing test setup.
Why the test suite is not in the final diffThe repository does not currently include a runnable iOS unit-test suite for this focus path. To validate these changes, I temporarily configured the existing XCTest target and added 97 native tests. All 97 tests passed on an iOS Simulator under Fabric before I removed the test setup. I also ran 69 targeted mutation checks; all 69 caused the tests to fail as expected. The test setup required changes to the Xcode project, The tests remain available in the branch history:
These tests cover the native focus-path logic. They do not exercise UIKit’s live focus engine, observe a physical focus ring, or cover the Legacy Bridge configuration. No device testing was performed. |
Suggested manual testingThese checks are intended for an iOS app that already uses Test this PR in an existing app
Core checks
Optional deeper checks
When reporting results, please include the device or Simulator model, iOS version, React Native version, and whether the app used Fabric or the Legacy Bridge. |
|
Dear @SynurDevelopers Thanks for putting together the PR. The changes make sense, and I'll try to research around the information you've provided, including the known limitations and the suggested manual test checklist. This falls outside my current focus for the moment, so I won't be able to get to it right away. But I'll return to this as I have some spare time. |
Scenario Lab demo appA runnable manual-test harness for this PR is available in SynurDevelopers/react-native-external-keyboard#1. The demo branch is stacked on To run it: git clone https://github.com/SynurDevelopers/react-native-external-keyboard.git
cd react-native-external-keyboard
git checkout demo/scenario-lab
yarn
cd example/ios
pod install
cd ../..
yarn example iosOpen Scenario Lab from the example app home screen. Device, Simulator, VoiceOver, and Instruments requirements are documented in the Scenario Lab guide. |
…eviewed] A focus request parked while the view was partially ready was never cleared by a later successful focus, causing a stale replay on the next window attach.
Addresses #125
Summary
This PR improves physical-keyboard focus on iOS, especially during navigation, native view attachment, and Fabric recycling.
It routes focus requests through a more reliable focus environment, retains requests until views are ready, and fixes related ownership, focus-locking, ordering, and event-state problems found while investigating #125.
The final diff changes 17 files under
ios/. It does not change the JavaScript API, TypeScript source, codegen specifications, or Android implementation.The shared native paths used by Fabric and the Legacy Bridge were updated. Local native tests covered Fabric; the Legacy Bridge configuration was not compiled or run.
What changed
Focus requests and autofocus
RNCEKVKeyboardFocusServiceas the shared entry point for programmatic keyboard focus.Ownership and ordering
orderIndexgroup reports the redirected movement as handled, preventing UIKit from performing a second movement.Focus locking and focus state
FocusTrapnow avoids repeating focus side effects when unchanged props are reapplied.onFocusChangelistener.enableContextMenucan therefore follow native focus without requiringonFocusChange.Known limitations
Several uncommon lifecycle cases remain:
RCTKeyWindow().FocusTrapprop updates can still pass through a briefly active state. The public JavaScriptFocusTrapavoids most of these transitions by rendering a plainViewwhile inactive.orderIdcan still leave subscribers registered after unmount.Validation
Completed:
git diff --checkpasses.The temporary test infrastructure was removed from the final diff in
8ef3723to keep the PR focused on the library changes. The test history, commit references, and restoration instructions are documented in this PR comment.Not completed: