Conversation
Android back reached the navigator through the JS thread, so while a thread was loading or syncing, back did nothing until that work finished. - withAndroidNativeScreenBack adds an OnBackPressedCallback to MainActivity that runs ahead of React Native's. When the top screen of the innermost stack opted in, it dismisses it natively (the same dismissal an iOS swipe back uses) and JS catches up through onDismissed. Everything else still goes to JS. - native-stack: `unstable_nativeBackDismissalEnabled` sets the screen's nativeBackButtonDismissalEnabled instead of hardcoding false on Android. - The Thread screen opts in. The in-window ⋮ menu holds back for JS while it is open (jsBackHold), so back still closes it first.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes the default Android back behavior for thread screens through a new native callback and patched navigation stack, with coordination across native and JS code. The cross-layer production behavior and unresolved menu-dismissal timing race require human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
The menu's JS hold reached the screen's native dismissal setting two passive effects after the menu opened, so back pressed in between popped the thread instead of closing the menu. The in-window menu's overlay now carries a nativeID, and the native back callback hands back to JS while a view with that ID is on screen. The callback reads the view tree when back is pressed, so the check holds from the overlay's first frame. The JS hold store and the Thread screen's setOptions round trip are gone.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe mobile app adds a Kotlin Android back callback and enables native dismissal for the Thread route. It marks keyboard-visible anchored menus for JavaScript back handling. The native-stack patch also updates dismissal support and header item processing. ChangesAndroid back handling
Native-stack header processing
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant AndroidSystem
participant MainActivity
participant AndroidAnchoredMenu
participant NativeStack
participant ReactNativeBackDispatcher
AndroidAnchoredMenu->>MainActivity: Expose JS back-handler ID in the view hierarchy
AndroidSystem->>MainActivity: Invoke back callback
alt JS back-handler ID is present
MainActivity->>ReactNativeBackDispatcher: Delegate default back action
else Native dismissal is enabled for an eligible screen
MainActivity->>NativeStack: Dismiss top screen
end
Merge Risk: 🔵 Low · up to The PR is mergeable with a bounded follow-up: custom iOS center header items will not retain identifier-based transition matching when callers provide an identifier. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Native back is limited to opted-in screens, and no security-control bypass was established. Reconciliation after an interrupted dismissal and the presence of any parent-level navigation guard remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This branch has not been deployed
1 participant
What Changed
On Android, back from a thread now pops the screen natively instead of waiting for the JS thread.
withAndroidNativeScreenBack(new config plugin) adds anOnBackPressedCallbacktoMainActivity.onPostCreate, after React Native's own, so it runs first.ScreenStack. If that screen opted in and isn't the stack's root, it callsdismissFromContainer(). That is the native dismissal react-native-screens uses for the iOS swipe back, and JS catches up throughonDismissed.unstable_nativeBackDismissalEnabledoption sets the screen'snativeBackButtonDismissalEnabled. native-stack hardcodes it tofalseon Android.nativeID(JS_BACK_HANDLER_NATIVE_ID), and the native callback hands back to JS while a view with that ID is on screen. The check reads the view tree when back is pressed, so it holds from the overlay's first frame, and back still closes the menu first.Why
Android back reaches the navigator through the JS thread:
hardwareBackPress.BackHandlerlisteners.When JS is busy, back does nothing until that work finishes, for example while a thread is loading or syncing. That's what users see: back from a thread that's still loading waits until it has loaded. An earlier probe on my Pixel 9, during a heavy sync, measured the JS event loop blocked for 28 s out of 60.
The native dismissal pops on the UI thread, so it doesn't wait for JS.
Measurements
Pixel 9, real account, release builds installed in place. The builds are my test builds (main plus my other open mobile PRs), before and with this PR.
withAndroidNativeScreenBack.test.mjscovers the generatedMainActivity, including the menu check (against the JS constant) and chaining withwithAndroidPredictiveBackCompat.UI Changes
The JS thread is held busy for 3 s right after the thread opens, then back is swiped. On the left is the build before this PR: back waits for JS. On the right, with it, Home comes back right away. Real time (MP4).
Android performance series
These are separate PRs, each reviewable on its own:
Checklist
Summary by CodeRabbit