ref(android): Extract shared nav sample app infrastructure into common package - #6220
0xadam-brown wants to merge 1 commit into
Conversation
…on package Move shared navigation sample infrastructure into the io.sentry.samples.android.navigation.common package and isolate the existing Nav2 implementation under its own package. Keep Nav2 performance controls focused while wiring route-work support and launcher resources for the sample foundation. Lays the groundwork for the Nav3 sample app. Co-Authored-By: Codex <noreply@openai.com>
📲 Install BuildsAndroid
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ebef890. Configure here.
| scenario = Nav2Scenario.COMPOSE, | ||
| scenario = | ||
| if (currentRoute.startsWith("/${RouteNames.CUSTOM}")) Nav2Scenario.CUSTOM | ||
| else Nav2Scenario.COMPOSE, |
There was a problem hiding this comment.
Custom flow freezes top bar
Medium Severity
updateComposeNavigationUi treats only routes that start with /Custom as the Custom tab. Product List and later Custom-flow destinations are stored under Compose, and the header does not refresh while Custom stays selected, so the displayed route and back stack go stale.
Reviewed by Cursor Bugbot for commit ebef890. Configure here.
| } | ||
| } | ||
| else -> Unit | ||
| } |
There was a problem hiding this comment.
Custom tab drops Home root
Medium Severity
Entering the Custom tab navigates with popUpTo(Home) inclusive, which removes the shared graph root. Later Home-based resets and Compose tab switches cannot pop that destination, so the real NavController stack and the tracked list diverge. Opening Compose also re-pushes Home and emits an extra navigation event.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ebef890. Configure here.
| routeSpec, | ||
| arguments = | ||
| mapOf( | ||
| Nav2Args.PRODUCT_ID to productId, |
There was a problem hiding this comment.
Custom routes tagged as Compose
Medium Severity
TracedNav2ComposeRoute always calls tagCurrentNav2Scenario with Nav2Scenario.COMPOSE. On the Custom tab the current span is the bound custom transaction, so sample_nav2_scenario is overwritten from Custom to Compose.
Reviewed by Cursor Bugbot for commit ebef890. Configure here.
| var customTransactionMode by rememberSaveable { | ||
| mutableStateOf(Nav2CustomTransactionMode.PER_SCREEN) | ||
| } | ||
| var asyncBrowseProductsJob by rememberSaveable { mutableStateOf<Job?>(null) } |
There was a problem hiding this comment.
Job stored in rememberSaveable
Medium Severity
asyncBrowseProductsJob is held in rememberSaveable, but Job cannot be written to a Bundle. If onSaveInstanceState runs while the 250ms async browse job is active, state saving throws.
Reviewed by Cursor Bugbot for commit ebef890. Configure here.
This branch has not been deployed
1 participant
📜 Description
Move shared nav sample app infrastructure into the
io.sentry.samples.android.navigation.commonpackage and isolate the existing Nav2 implementation under its own package.💡 Motivation and Context
Lays the groundwork for the Nav3 sample app, which will make use of the
.commonclasses.💚 How did you test it?
Manually via sample app installation / interaction.
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps