Retrofit Geofencing #302
Retrofit Geofencing #302
Conversation
There was a problem hiding this comment.
@adaext In MainActivity.java the indent size changed from 4 spaces to 2 spaces, which doesn't match the format of the other files. The resulting diff is very large and doesn't reflect the changes specific to this PR. Can you please reset the indent to 4 spaces and reformat the file?
Ok, thanks. |
...encing/app/src/main/java/com/google/android/gms/location/sample/geofencing/MainActivity.java
Outdated
Show resolved
Hide resolved
...encing/app/src/main/java/com/google/android/gms/location/sample/geofencing/MainActivity.java
Outdated
Show resolved
Hide resolved
...encing/app/src/main/java/com/google/android/gms/location/sample/geofencing/MainActivity.java
Outdated
Show resolved
Hide resolved
| // Request permission. It's possible this can be auto answered if device policy | ||
| // sets the permission in a given state or the user denied the permission | ||
| // previously and checked "Never ask again". | ||
| ActivityCompat.requestPermissions( |
There was a problem hiding this comment.
Consider using registerForActivityResult with Permission Contract
https://developer.android.com/training/permissions/requesting#allow-system-manage-request-code
| // previously and checked "Never ask again". | ||
| ActivityCompat.requestPermissions( | ||
| MainActivity.this, | ||
| new String[] { |
There was a problem hiding this comment.
This is not allowed since Android 11.
You should first request COARSE + FINE and then BACKGROUND separately
| // Return a GeofencingRequest. | ||
| return builder.build(); | ||
| } | ||
| @Override |
There was a problem hiding this comment.
This goes against the best practices. I understand for the sample might be okay, but I would consider moving this logic under a button/CTA in the screen.
| R.string.permission_denied_explanation, | ||
| R.string.settings, | ||
| view -> { | ||
| // Build intent that displays the App settings screen. |
There was a problem hiding this comment.
AFAIK showing the app details in system settings isn't a pattern we recommend for developers.
Upgrade the Gradle and SDK to the latest version and add tristate support for location permission.