Fixes for Android 13 and newer#1606
Conversation
- when the device runs Android 16 or newer, the insets for the status bar and navigation bar are applied to the main view. - when the device runs Android 13 or newer, the "back" gesture is handled by an custom dispatcher. - minimum SDK version is updated from 27 to 30 since the web view does not work in older versions.
There was a problem hiding this comment.
Pull request overview
This pull request addresses UI and navigation issues on Android 13+ devices, specifically fixing bugs where the UI is drawn under the status bar and the back gesture closes the app instead of navigating within it.
Changes:
- Adds window insets handling for Android 16+ to prevent UI from being drawn under system bars
- Implements modern back gesture handling using OnBackInvokedCallback for Android 13+
- Updates minimum SDK version from 27 to 30 due to WebView compatibility issues
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| android/app/src/main/java/gallery/memories/MainActivity.kt | Adds insets listener for Android 16+, implements OnBackInvokedCallback for Android 13+, conditionalizes legacy onKeyDown back handling for Android 12 and older, and standardizes SDK version checks to use SDK_INT |
| android/app/build.gradle | Increases minimum SDK version from 27 (Android 8.1) to 30 (Android 11) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onBackInvokedDispatcher.registerOnBackInvokedCallback( | ||
| OnBackInvokedDispatcher.PRIORITY_DEFAULT | ||
| ) { | ||
| if (binding.webview.canGoBack()) { | ||
| binding.webview.goBack() | ||
| } else { | ||
| finish() | ||
| } | ||
| } |
There was a problem hiding this comment.
The back gesture callback is registered in onCreate but never unregistered. This could lead to a memory leak if the activity is recreated (e.g., on configuration changes). The callback should be stored as a field and unregistered in onDestroy using onBackInvokedDispatcher.unregisterOnBackInvokedCallback(callback).
| // Apply the insets as a margin to the view. This solution sets | ||
| // only the bottom, left, and right dimensions, but you can apply whichever | ||
| // insets are appropriate to your layout. You can also update the view padding | ||
| // if that's more appropriate. |
There was a problem hiding this comment.
The comment states "only the bottom, left, and right dimensions" but the code actually applies all four margins including topMargin (line 96). This comment appears to be copied from documentation and is misleading. Consider updating the comment to accurately reflect the implementation, or clarify that all insets are being applied.
| // Apply the insets as a margin to the view. This solution sets | |
| // only the bottom, left, and right dimensions, but you can apply whichever | |
| // insets are appropriate to your layout. You can also update the view padding | |
| // if that's more appropriate. | |
| // Apply the insets as margins to the view. This implementation sets | |
| // all four margins (top, bottom, left, and right) based on the system bar insets, | |
| // but you can apply whichever insets are appropriate to your layout or adjust padding | |
| // instead if that's more appropriate. |
| if (SDK_INT >= 33) { | ||
| onBackInvokedDispatcher.registerOnBackInvokedCallback( | ||
| OnBackInvokedDispatcher.PRIORITY_DEFAULT | ||
| ) { | ||
| if (binding.webview.canGoBack()) { | ||
| binding.webview.goBack() | ||
| } else { | ||
| finish() | ||
| } | ||
| } |
There was a problem hiding this comment.
For the OnBackInvokedCallback to work properly on Android 13+, the application needs to opt-in by setting android:enableOnBackInvokedCallback="true" in the application tag of AndroidManifest.xml. Without this flag, the back gesture handler won't be triggered, and the issue described in #1605 won't be fully resolved.
| // Enable insets for Android 16 or newer | ||
| if (SDK_INT >= 36) { |
There was a problem hiding this comment.
The insets fix is only applied for Android 16 (SDK_INT >= 36), but according to issue #1605, the UI is drawn under the status bar on Android 16. However, this issue may also affect earlier versions of Android. Consider applying this fix for a broader range of Android versions (e.g., Android 13+, SDK_INT >= 33) to ensure compatibility with more devices experiencing this issue, unless there's a specific reason it should only apply to Android 16+.
| @@ -136,19 +176,24 @@ class MainActivity : AppCompatActivity() { | |||
| releasePlayer() | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The @SuppressLint("GestureBackNavigation") annotation suppresses a lint warning about using the deprecated onKeyDown method for handling back navigation. However, this annotation is only necessary when the deprecated approach is used. Since the code is now conditionally using the modern OnBackInvokedCallback for Android 13+ (SDK_INT >= 33) and only falls back to onKeyDown for Android 12 and older (SDK_INT < 33), this suppression is justified. Consider adding a comment explaining why this suppression is needed to maintain backward compatibility.
| // Suppress GestureBackNavigation because onKeyDown is only used for Android 12 and older (SDK < 33); | |
| // Android 13+ uses OnBackInvokedCallback for back navigation. |
| defaultConfig { | ||
| applicationId "gallery.memories" | ||
| minSdk 27 | ||
| minSdk 30 |
There was a problem hiding this comment.
The PR description states "minimum SDK version is updated from 27 to 30 since the web view does not work in older versions." However, this is a significant change that will prevent users with Android 9 (API 28) and Android 10 (API 29) from installing or updating the app. Ensure this decision is well-justified and documented. If there are specific WebView issues on Android 9-10, consider adding more context in the commit message or PR description about what those issues are and why they cannot be worked around.
| // Enable insets for Android 16 or newer | ||
| if (SDK_INT >= 36) { | ||
| binding.webview.setOnApplyWindowInsetsListener { v, windowInsets -> | ||
| val insets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars()) |
There was a problem hiding this comment.
Type mismatch in the insets listener. The parameter windowInsets is of type android.view.WindowInsets (platform type), but getInsets() is being called with WindowInsetsCompat.Type.systemBars() which is from the AndroidX compatibility library.
The platform WindowInsets class has a getInsets(int type) method that expects constants from android.view.WindowInsets.Type, not WindowInsetsCompat.Type. Either use WindowInsets.Type.systemBars() instead of WindowInsetsCompat.Type.systemBars(), or convert the platform WindowInsets to WindowInsetsCompat using WindowInsetsCompat.toWindowInsetsCompat(windowInsets).
| val insets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars()) | |
| val insets = windowInsets.getInsets(WindowInsets.Type.systemBars()) |
|
Thank you! |
|
There are a few things left which should be fixed before publishing a new version. I will provide this as a second PR. |
This will fix #1605