Repository navigation
fix(iOS, Stack): remove the navigation controller's view when a nested stack is recycled - #4997
Conversation
… is recycled Co-authored-by: intergalacticspacehighway <23293248+intergalacticspacehighway@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Co-authored-by: intergalacticspacehighway <23293248+intergalacticspacehighway@users.noreply.github.com>
…arance was forwarded early (#51923) # Why Fixes #51288. From the second `fullScreenModal` presentation of a screen with a nested native stack, a `Host` that mounts while the modal still animates gets no safe area, so its content sits under the status bar and the title. ## Root cause - UIKit gives a view controller its safe area only after it has appeared. - A child view controller appears with its parent. A child added while the parent is appearing is included. A child added after the parent appeared, with no transition running, appears right away. But a child added after the parent appeared **while a transition is still running** gets no appearance calls, then or later. It never appears, so its safe area stays `{0, 0, 0, 0}`. - Normally that window doesn't exist: a presented screen appears when the presentation ends. With react-native-screens it does. `RNSScreenStack prepareForRecycle` removes the navigation controller from its parent but leaves its view in the stack view. When a recycled stack is reused in the next modal, that view enters the window with no parent controller, so UIKit makes the navigation controller and its screen appear right away, while the modal is still animating. Fix proposed upstream in software-mansion/react-native-screens#4997. - In the repro, the loaded `Host` replaces the loading one 400 ms in, inside that window, so its hosting controller never appears. Evidence: UIKit's private `_appearState` at attach was parent 1 (appearing) and child 1 → 2 on the first presentation, and parent 2 (appeared) and child 0 forever on later ones. lldb traces show the inner `RNSScreen` getting `viewDidAppear` from `transitionDidFinish:` the first time and from `viewDidMoveToWindow:shouldAppearOrDisappear:` later. A plain UIKit app breaks the same way when a container makes its child appear before its presentation ends. # How This is a workaround until software-mansion/react-native-screens#4997 ships (the code has a TODO to remove it). `HostingView` uses a small `UIHostingController` subclass that records `viewWillAppear`. If the hosting controller is attached during a transition and UIKit didn't forward `viewWillAppear`, `HostingView` forwards it with `beginAppearanceTransition(true, animated: false)` + `endAppearanceTransition()`. Otherwise nothing changes. It also keeps `Host` correct inside any other container with this timing. With the react-native-screens fix alone, the repro also works. # Test Plan The issue's repro ([brentvatne/expo-ui-host-safe-area-repro](https://github.com/brentvatne/expo-ui-host-safe-area-repro)) on SDK 58, `expo-modules-core` built from source, iOS 27.0 simulator, `-sequence a,b,a`: - Before: from the second presentation on, Row 1 is under the status bar and the title. - After: Row 1 is below the title. A 30 fps recording of the second and third presentations shows the list in place from its first frame, with no jump. Expo UI measure tests (`ExpoUIMeasurement`, `ExpoUIHostSize`, `ExpoUIRNHostViewSize`) in `bare-expo`: iOS 26/26 passed. | Before (from the issue) | After | | :---: | :---: | | <img src="https://github.com/brentvatne/expo-ui-host-safe-area-repro/raw/3b635b8/pr-evidence/host-safe-area/third.png" width="220"> | <img src="https://raw.githubusercontent.com/expo/expo/ea74840d1b264fe2be1f1f03dedbab053ee439ac/after.png" width="220"> | # Checklist - [x] I added a changeset and followed [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --------- Co-authored-by: expo-tuft[bot] <288127324+expo-tuft[bot]@users.noreply.github.com> Co-authored-by: intergalacticspacehighway <23293248+intergalacticspacehighway@users.noreply.github.com>
…arance was forwarded early (#51923) # Why Fixes #51288. From the second `fullScreenModal` presentation of a screen with a nested native stack, a `Host` that mounts while the modal still animates gets no safe area, so its content sits under the status bar and the title. ## Root cause - UIKit gives a view controller its safe area only after it has appeared. - A child view controller appears with its parent. A child added while the parent is appearing is included. A child added after the parent appeared, with no transition running, appears right away. But a child added after the parent appeared **while a transition is still running** gets no appearance calls, then or later. It never appears, so its safe area stays `{0, 0, 0, 0}`. - Normally that window doesn't exist: a presented screen appears when the presentation ends. With react-native-screens it does. `RNSScreenStack prepareForRecycle` removes the navigation controller from its parent but leaves its view in the stack view. When a recycled stack is reused in the next modal, that view enters the window with no parent controller, so UIKit makes the navigation controller and its screen appear right away, while the modal is still animating. Fix proposed upstream in software-mansion/react-native-screens#4997. - In the repro, the loaded `Host` replaces the loading one 400 ms in, inside that window, so its hosting controller never appears. Evidence: UIKit's private `_appearState` at attach was parent 1 (appearing) and child 1 → 2 on the first presentation, and parent 2 (appeared) and child 0 forever on later ones. lldb traces show the inner `RNSScreen` getting `viewDidAppear` from `transitionDidFinish:` the first time and from `viewDidMoveToWindow:shouldAppearOrDisappear:` later. A plain UIKit app breaks the same way when a container makes its child appear before its presentation ends. # How This is a workaround until software-mansion/react-native-screens#4997 ships (the code has a TODO to remove it). `HostingView` uses a small `UIHostingController` subclass that records `viewWillAppear`. If the hosting controller is attached during a transition and UIKit didn't forward `viewWillAppear`, `HostingView` forwards it with `beginAppearanceTransition(true, animated: false)` + `endAppearanceTransition()`. Otherwise nothing changes. It also keeps `Host` correct inside any other container with this timing. With the react-native-screens fix alone, the repro also works. # Test Plan The issue's repro ([brentvatne/expo-ui-host-safe-area-repro](https://github.com/brentvatne/expo-ui-host-safe-area-repro)) on SDK 58, `expo-modules-core` built from source, iOS 27.0 simulator, `-sequence a,b,a`: - Before: from the second presentation on, Row 1 is under the status bar and the title. - After: Row 1 is below the title. A 30 fps recording of the second and third presentations shows the list in place from its first frame, with no jump. Expo UI measure tests (`ExpoUIMeasurement`, `ExpoUIHostSize`, `ExpoUIRNHostViewSize`) in `bare-expo`: iOS 26/26 passed. | Before (from the issue) | After | | :---: | :---: | | <img src="https://github.com/brentvatne/expo-ui-host-safe-area-repro/raw/3b635b8/pr-evidence/host-safe-area/third.png" width="220"> | <img src="https://raw.githubusercontent.com/expo/expo/ea74840d1b264fe2be1f1f03dedbab053ee439ac/after.png" width="220"> | # Checklist - [x] I added a changeset and followed [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --------- Co-authored-by: expo-tuft[bot] <288127324+expo-tuft[bot]@users.noreply.github.com> Co-authored-by: intergalacticspacehighway <23293248+intergalacticspacehighway@users.noreply.github.com> (cherry picked from commit 0937842)
kligarski
left a comment
There was a problem hiding this comment.
Looks good to me, thank you!
|
CI fails are unrelated. |
Description
When a native stack nested in a modal is recycled and then reused, its screen appears before the modal has finished presenting. UIKit doesn't forward appearance to a child view controller that is added after its parent appeared while a transition is still running, so such a child never appears and never gets a safe area.
This breaks
Hostfrom@expo/uiin expo/expo#51288: from the secondfullScreenModalpresentation of a screen with a nested stack, aHostthat mounts during the presentation lays out its content under the status bar and the large title.Cause
prepareForRecycleremoves_controllerfrom its parent view controller but leaves_controller.viewinside the stack view.viewDidMoveToWindow:shouldAppearOrDisappear:), while the modal is still animating.maybeAddToParentAndUpdateContaineradds it to the parent only afterwards.reactAddControllerToClosestParentadds the controller to its parent before adding its view, so it appears with the modal when the presentation finishes.Breakpoints on
-[RNSScreen viewDidAppear:]confirm this: on the first presentation it's called from-[UIPresentationController transitionDidFinish:], on later ones from the window-move path above, before the modal finishes.Changes
prepareForRecyclealso removes_controller.view, so a reused stack attaches the controller before its view, as on first use.Test plan
Repro: https://github.com/brentvatne/expo-ui-host-safe-area-repro (expo-router
fullScreenModalroutes with a nestedStack, and a SwiftUIHostthat mounts 400 ms after the screen). Open A, close, open B, close, open A.Tested on an iOS 27.0 simulator (Xcode 27.1). The changed code is the same on
main.Checklist