fix(samples): Add back navigation to secondary Android sample screens#5729
fix(samples): Add back navigation to secondary Android sample screens#5729runningcode wants to merge 1 commit into
Conversation
Secondary screens in the Android sample had no way to navigate back. XML AppCompat activities now enable the ActionBar up arrow (setDisplayHomeAsUpEnabled + onSupportNavigateUp); Compose activities get a Material3 TopAppBar with a back arrow. Also fix SecondActivity: it (and MainActivity's launcher) called finish() before startActivity, removing MainActivity from the back stack so system back exited the app. It now stays on the stack and back returns to Main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3d205d0 | 352.15 ms | 432.53 ms | 80.38 ms |
| e59e22a | 368.02 ms | 432.00 ms | 63.98 ms |
| 806307f | 357.85 ms | 424.64 ms | 66.79 ms |
| d364ace | 384.53 ms | 453.51 ms | 68.98 ms |
| 4c04bb8 | 307.93 ms | 362.34 ms | 54.41 ms |
| bb0ff41 | 312.86 ms | 363.78 ms | 50.92 ms |
| 91bb874 | 310.68 ms | 359.24 ms | 48.56 ms |
| 11f90db | 314.26 ms | 372.43 ms | 58.17 ms |
| f6cdbf0 | 314.19 ms | 357.59 ms | 43.40 ms |
| 8687935 | 332.52 ms | 362.23 ms | 29.71 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3d205d0 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| e59e22a | 1.58 MiB | 2.20 MiB | 635.34 KiB |
| 806307f | 1.58 MiB | 2.10 MiB | 533.42 KiB |
| d364ace | 1.58 MiB | 2.11 MiB | 539.75 KiB |
| 4c04bb8 | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| 91bb874 | 1.58 MiB | 2.13 MiB | 559.07 KiB |
| 11f90db | 0 B | 0 B | 0 B |
| f6cdbf0 | 0 B | 0 B | 0 B |
| 8687935 | 1.58 MiB | 2.19 MiB | 619.17 KiB |
runningcode
requested review from
0xadam-brown,
adinauer,
markushi and
romtsn
as code owners
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| Column( | ||
| modifier = | ||
| Modifier.fillMaxWidth() | ||
| .statusBarsPadding() |
There was a problem hiding this comment.
Bug: Redundant .statusBarsPadding() is applied within a Scaffold that already handles insets via innerPadding, causing double padding at the top of the screen.
Severity: LOW
Suggested Fix
Remove the redundant .statusBarsPadding() modifier from the child composables inside the Scaffold content. The innerPadding from the Scaffold should be the only modifier handling system bar insets for the main content area. For example, in SQLiteActivity.kt, remove .statusBarsPadding() from the Column modifier chain.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/sqlite/SQLiteActivity.kt#L318
Potential issue: The refactoring to use `Scaffold` with a `TopAppBar` introduces a
layout bug. The `innerPadding` provided by the `Scaffold` lambda already includes the
necessary insets for the status bar. However, child composables within the `Scaffold`
content, such as the `Column` in `SQLiteActivity.kt` and the root `Box` in
`UiLoadScreen.kt` (used by `UiLoadActivity`), are also applying `.statusBarsPadding()`.
This results in the status bar padding being applied twice, creating unnecessary extra
space at the top of the screen and pushing the content down.
Also affects:
sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/sqlite/UiLoadActivity.kt:68~70
Did we get this right? 👍 / 👎 to inform future reviews.
![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 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4d8253d. Configure here.
| detach(tabB) | ||
| } | ||
| } | ||
| supportActionBar?.setDisplayHomeAsUpEnabled(true) |
There was a problem hiding this comment.
NoActionBar hides up affordance
Medium Severity
DetachAttachTabsActivity enables the ActionBar up affordance, but the manifest assigns @style/AppTheme.Main (NoActionBar), so supportActionBar is null and no on-screen back arrow appears despite the PR’s AppCompat pattern.
Reviewed by Cursor Bugbot for commit 4d8253d. Configure here.
There was a problem hiding this comment.
that's probably valid -- maybe we could adjust the layout so the buttons show up under the action bar as opposed to using NoActionBar theme
| modifier = | ||
| Modifier.fillMaxWidth() | ||
| .statusBarsPadding() | ||
| .padding(16.dp) |
There was a problem hiding this comment.
Scaffold plus status bar padding
Low Severity
After wrapping the screen in a Scaffold with a TopAppBar, the content Column still uses statusBarsPadding() on top of innerPadding, which double-applies status-bar insets and adds extra empty space below the app bar.
Reviewed by Cursor Bugbot for commit 4d8253d. Configure here.
| } | ||
| ) { innerPadding -> | ||
| Box(modifier = Modifier.padding(innerPadding)) { | ||
| UiLoadScreen(status = status, onClose = ::finish) |
There was a problem hiding this comment.
Scaffold plus status bar padding
Low Severity
UiLoadActivity now pads content with Scaffold innerPadding, but UiLoadScreen still applies statusBarsPadding(), so status-bar insets are applied twice and the centered UI sits too low.
Reviewed by Cursor Bugbot for commit 4d8253d. Configure here.
2 participants
📜 Description
Secondary screens in the Android sample had no way to navigate back — once you opened one (e.g. from the Tracing or Integrations tabs) there was no visible back affordance, and
SecondActivityactively broke the system back button.This adds idiomatic back navigation per screen type:
SecondActivity,GesturesActivity,PermissionsActivity,DetachAttachTabsActivity,ThirdActivityFragment,CameraXActivity,TriggerHttpRequestActivity): enable the ActionBar up arrow viasetDisplayHomeAsUpEnabled(true)+onSupportNavigateUp()→onBackPressedDispatcher.FrameDataForSpansActivity,ReplayAnimationsActivity,ComposeActivity,SQLiteActivity,UiLoadActivity): add a Material3TopAppBarwith a back arrow and threadinnerPaddingso content is no longer drawn under the status bar.It also fixes a back-stack bug:
MainActivity's "Open Second Activity" andSecondActivity's "Back to Main" both calledfinish()beforestartActivity(...), removingMainActivityfrom the back stack so the system back button exited the app instead of returning. They now keep the activity on the stack.CustomTabsActivityis intentionally left unchanged — it launches a Chrome Custom Tab and finishes immediately, so it has no UI of its own.💡 Motivation and Context
Makes the sample navigable — you can now get back from every secondary screen (both via the on-screen back button and the system back gesture).
💚 How did you test it?
Built and installed the sample on an emulator:
SecondActivity: up arrow present; system back now returns toMainActivity(previously exited the app).FrameDataForSpansActivity(Compose): TopAppBar back arrow present, returns toMainActivity, and the title no longer overlaps the status bar.compileDebugKotlin/compileDebugJavaWithJavac) andspotlessApply/apiDumpare clean.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
None.
Ref: JAVA-631
#skip-changelog