From 80b31293565c1ae5397ad31776cfec8c4354e510 Mon Sep 17 00:00:00 2001
From: Marcus Hoffmann <bubu@bubu1.eu>
Date: Tue, 9 Nov 2021 03:08:01 +0100
Subject: [PATCH] improve background location request flow

Instead of directly jumping to the settings screen (which is what the
"Request background location access" amounts to) and leaving the user
there without a clue what to do, we update the label, icon and button
with new text explaining what the user needs to do next.
---
 .../ExposureNotificationsConfirmActivity.kt   | 92 +++++++++++++------
 .../res/drawable/ic_outline_location_on.xml   | 13 +++
 ...xposure_notifications_confirm_activity.xml | 48 ++++++++++
 .../src/main/res/values-de/strings.xml        |  4 +
 .../src/main/res/values/strings.xml           |  2 +
 5 files changed, 133 insertions(+), 26 deletions(-)
 create mode 100644 play-services-nearby-core-ui/src/main/res/drawable/ic_outline_location_on.xml

diff --git a/play-services-nearby-core-ui/src/main/kotlin/org/microg/gms/nearby/core/ui/ExposureNotificationsConfirmActivity.kt b/play-services-nearby-core-ui/src/main/kotlin/org/microg/gms/nearby/core/ui/ExposureNotificationsConfirmActivity.kt
index 225a0e02..d2b831a6 100644
--- a/play-services-nearby-core-ui/src/main/kotlin/org/microg/gms/nearby/core/ui/ExposureNotificationsConfirmActivity.kt
+++ b/play-services-nearby-core-ui/src/main/kotlin/org/microg/gms/nearby/core/ui/ExposureNotificationsConfirmActivity.kt
@@ -82,6 +82,9 @@ class ExposureNotificationsConfirmActivity : AppCompatActivity() {
         findViewById<Button>(R.id.grant_permission_button).setOnClickListener {
             requestPermissions()
         }
+        findViewById<Button>(R.id.grant_background_location_button).setOnClickListener {
+            requestBackgroundLocation()
+        }
         findViewById<Button>(R.id.enable_bluetooth_button).setOnClickListener {
             requestBluetooth()
         }
@@ -98,50 +101,87 @@ class ExposureNotificationsConfirmActivity : AppCompatActivity() {
     }
 
     private fun updateButton() {
-        findViewById<Button>(android.R.id.button1).isEnabled = !permissionNeedsHandling && !bluetoothNeedsHandling && !locationNeedsHandling
+        findViewById<Button>(android.R.id.button1).isEnabled =
+            !permissionNeedsHandling && !backgroundLocationNeedsHandling && !bluetoothNeedsHandling && !locationNeedsHandling
     }
 
     // Permissions
     private var permissionNeedsHandling: Boolean = false
+    private var backgroundLocationNeedsHandling: Boolean = false
     private var permissionRequestCode = 33
     private val permissions by lazy {
-        if (Build.VERSION.SDK_INT >= 31){
-            arrayOf("android.permission.BLUETOOTH_ADVERTISE", "android.permission.BLUETOOTH_SCAN", "android.permission.ACCESS_BACKGROUND_LOCATION", "android.permission.ACCESS_COARSE_LOCATION", "android.permission.ACCESS_FINE_LOCATION")
-
-        }
-        else if (Build.VERSION.SDK_INT >= 29) {
-            arrayOf("android.permission.ACCESS_BACKGROUND_LOCATION", "android.permission.ACCESS_COARSE_LOCATION", "android.permission.ACCESS_FINE_LOCATION")
-        } else {
-            arrayOf("android.permission.ACCESS_COARSE_LOCATION", "android.permission.ACCESS_FINE_LOCATION")
+        when {
+            Build.VERSION.SDK_INT >= 31 -> {
+                // We shouldn't be needing the LOCATION permissions on 31+ anymore, at least when
+                // apps making use of this target 31+ as well, but this needs more testing. See
+                // https://developer.android.com/guide/topics/connectivity/bluetooth/permissions#assert-never-for-location
+                arrayOf(
+                    "android.permission.BLUETOOTH_ADVERTISE",
+                    "android.permission.BLUETOOTH_SCAN",
+                    "android.permission.ACCESS_COARSE_LOCATION",
+                    "android.permission.ACCESS_FINE_LOCATION"
+                )
+            }
+            Build.VERSION.SDK_INT == 29 -> {
+                // We only can directly request background location permission on 29.
+                // We need it on 30 (and possibly later) as well, but it has to be requested in a two
+                // step process, see https://fosstodon.org/@utf8equalsX/104359649537615235
+                arrayOf(
+                    "android.permission.ACCESS_BACKGROUND_LOCATION",
+                    "android.permission.ACCESS_COARSE_LOCATION",
+                    "android.permission.ACCESS_FINE_LOCATION"
+                )
+            }
+            else -> {
+                // Below 29 or equals 30
+                arrayOf(
+                    "android.permission.ACCESS_COARSE_LOCATION",
+                    "android.permission.ACCESS_FINE_LOCATION"
+                )
+            }
         }
     }
+
     private fun checkPermissions() {
-        permissionNeedsHandling = Build.VERSION.SDK_INT >= 23 && permissions.any { ContextCompat.checkSelfPermission(this, it) != PackageManager.PERMISSION_GRANTED }
-        findViewById<View>(R.id.grant_permission_view).visibility = if (permissionNeedsHandling) View.VISIBLE else View.GONE
+        permissionNeedsHandling = Build.VERSION.SDK_INT >= 23 && permissions.any {
+            ContextCompat.checkSelfPermission(
+                this,
+                it
+            ) != PackageManager.PERMISSION_GRANTED
+        }
+
+        backgroundLocationNeedsHandling = Build.VERSION.SDK_INT >= 30
+                && ContextCompat.checkSelfPermission(
+            this,
+            "android.permission.ACCESS_FINE_LOCATION"
+        ) == PackageManager.PERMISSION_GRANTED
+                && ContextCompat.checkSelfPermission(
+            this,
+            "android.permission.ACCESS_BACKGROUND_LOCATION"
+        ) != PackageManager.PERMISSION_GRANTED
+
+        findViewById<View>(R.id.grant_permission_view).visibility =
+            if (permissionNeedsHandling) View.VISIBLE else View.GONE
+        findViewById<View>(R.id.grant_background_location_view).visibility =
+            if (!permissionNeedsHandling && backgroundLocationNeedsHandling) View.VISIBLE else View.GONE
         updateButton()
     }
 
     private fun requestPermissions() {
-        when {
-            Build.VERSION.SDK_INT >= 30 -> requestPermissions(
-                permissions.toSet().minus("android.permission.ACCESS_BACKGROUND_LOCATION").toTypedArray(), ++permissionRequestCode
-            )
-            Build.VERSION.SDK_INT >= 23 -> requestPermissions(permissions, ++permissionRequestCode)
+        if (Build.VERSION.SDK_INT >= 23) {
+            requestPermissions(permissions, ++permissionRequestCode)
+        }
+    }
+
+    private fun requestBackgroundLocation() {
+        if (Build.VERSION.SDK_INT >= 23) {
+            requestPermissions(arrayOf("android.permission.ACCESS_BACKGROUND_LOCATION"), ++permissionRequestCode)
         }
     }
 
     override fun onRequestPermissionsResult(requestCode: Int, permissions: Array<out String>, grantResults: IntArray) {
         super.onRequestPermissionsResult(requestCode, permissions, grantResults)
-        if (requestCode == this.permissionRequestCode) {
-            when {
-                Build.VERSION.SDK_INT >= 30 && permissions.contains("android.permission.ACCESS_FINE_LOCATION") ->
-                    requestPermissions(
-                        arrayOf("android.permission.ACCESS_BACKGROUND_LOCATION"),
-                        ++permissionRequestCode
-                    )
-                else -> checkPermissions()
-            }
-        }
+        if (requestCode == this.permissionRequestCode) checkPermissions()
     }
 
     // Bluetooth
diff --git a/play-services-nearby-core-ui/src/main/res/drawable/ic_outline_location_on.xml b/play-services-nearby-core-ui/src/main/res/drawable/ic_outline_location_on.xml
new file mode 100644
index 00000000..55063d15
--- /dev/null
+++ b/play-services-nearby-core-ui/src/main/res/drawable/ic_outline_location_on.xml
@@ -0,0 +1,13 @@
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+    android:width="24dp"
+    android:height="24dp"
+    android:viewportWidth="24"
+    android:viewportHeight="24"
+    android:tint="?attr/colorAccent">
+  <path
+      android:fillColor="#000"
+      android:pathData="M12,2C8.13,2 5,5.13 5,9c0,5.25 7,13 7,13s7,-7.75 7,-13c0,-3.87 -3.13,-7 -7,-7zM7,9c0,-2.76 2.24,-5 5,-5s5,2.24 5,5c0,2.88 -2.88,7.19 -5,9.88C9.92,16.21 7,11.85 7,9z"/>
+  <path
+      android:fillColor="#000"
+      android:pathData="M12,9m-2.5,0a2.5,2.5 0,1 1,5 0a2.5,2.5 0,1 1,-5 0"/>
+</vector>
diff --git a/play-services-nearby-core-ui/src/main/res/layout/exposure_notifications_confirm_activity.xml b/play-services-nearby-core-ui/src/main/res/layout/exposure_notifications_confirm_activity.xml
index f6d2d359..f13888d9 100644
--- a/play-services-nearby-core-ui/src/main/res/layout/exposure_notifications_confirm_activity.xml
+++ b/play-services-nearby-core-ui/src/main/res/layout/exposure_notifications_confirm_activity.xml
@@ -90,6 +90,54 @@
                     android:textColor="?android:attr/textColorPrimaryInverse" />
             </RelativeLayout>
 
+            <RelativeLayout
+                android:id="@+id/grant_background_location_view"
+                android:layout_width="match_parent"
+                android:layout_height="wrap_content"
+                android:layout_marginBottom="8dp"
+                android:background="?attr/colorAccent"
+                android:clipToPadding="false"
+                android:paddingLeft="16dp"
+                android:paddingTop="16dp"
+                android:paddingRight="16dp"
+                android:paddingBottom="8dp"
+                android:visibility="gone"
+                tools:visibility="visible">
+
+                <ImageView
+                    android:id="@+id/grant_background_location_icon"
+                    android:layout_width="24dp"
+                    android:layout_height="24dp"
+                    android:layout_alignTop="@id/grant_background_location_summary"
+                    android:layout_alignBottom="@id/grant_background_location_summary"
+                    android:layout_alignParentLeft="true"
+                    android:layout_centerVertical="true"
+                    android:src="@drawable/ic_outline_location_on"
+                    app:tint="?attr/colorPrimary" />
+
+                <TextView
+                    android:id="@+id/grant_background_location_summary"
+                    style="@style/TextAppearance.AppCompat.Small.Inverse"
+                    android:layout_width="0dip"
+                    android:layout_height="wrap_content"
+                    android:layout_alignParentRight="true"
+                    android:layout_marginLeft="16dp"
+                    android:layout_toRightOf="@id/grant_background_location_icon"
+                    android:layout_weight="1"
+                    android:text="@string/exposure_grant_background_location_description"/>
+
+                <Button
+                    android:id="@+id/grant_background_location_button"
+                    style="@style/Widget.AppCompat.Button.Borderless"
+                    android:layout_width="wrap_content"
+                    android:layout_height="wrap_content"
+                    android:layout_below="@id/grant_background_location_summary"
+                    android:layout_alignLeft="@id/grant_background_location_summary"
+                    android:layout_marginLeft="-16dp"
+                    android:text="@string/exposure_grant_background_location_button"
+                    android:textColor="?android:attr/textColorPrimaryInverse" />
+            </RelativeLayout>
+
             <RelativeLayout
                 android:id="@+id/enable_bluetooth_view"
                 android:layout_width="match_parent"
diff --git a/play-services-nearby-core-ui/src/main/res/values-de/strings.xml b/play-services-nearby-core-ui/src/main/res/values-de/strings.xml
index 6be1d2f9..838956ea 100644
--- a/play-services-nearby-core-ui/src/main/res/values-de/strings.xml
+++ b/play-services-nearby-core-ui/src/main/res/values-de/strings.xml
@@ -66,4 +66,8 @@ Deine Identität oder das Testergebnis werden nicht geteilt."</string>
     <string name="exposure_confirm_bluetooth_description">Bluetooth muss eingeschaltet sein.</string>
     <string name="exposure_confirm_location_description">Standortzugriff muss eingeschaltet sein.</string>
     <string name="exposure_confirm_button">Aktivieren</string>
+    <string name="pref_exposure_error_nearby_not_granted_title">Neue Berechtigung benötigt</string>
+    <string name="pref_exposure_error_nearby_not_granted_description">Tippe hier um die benötigten Berechtigungen zu erteilen</string>
+    <string name="exposure_grant_background_location_description">Fast geschafft! Du musst den Zugriff auf den Standort im Hintergrund erlauben indem du auf dem nächsten Bildschirm \'Immer zulassen\' auswählst und dann hierher zurück kommst.</string>
+    <string name="exposure_grant_background_location_button">Einstellungen Öffnen</string>
 </resources>
diff --git a/play-services-nearby-core-ui/src/main/res/values/strings.xml b/play-services-nearby-core-ui/src/main/res/values/strings.xml
index 7aaa43e9..8d66adcf 100644
--- a/play-services-nearby-core-ui/src/main/res/values/strings.xml
+++ b/play-services-nearby-core-ui/src/main/res/values/strings.xml
@@ -78,4 +78,6 @@ Your identity or test result won&apos;t be shared with other people."</string>
     <string name="exposure_confirm_button">Enable</string>
     <string name="pref_exposure_error_nearby_not_granted_title">New Permissions required</string>
     <string name="pref_exposure_error_nearby_not_granted_description">Tap to grant required permissions to Exposure Notifications</string>
+    <string name="exposure_grant_background_location_description">Almost there! You will need to enable background location access by selecting the \'Allow all the time\' option on the next screen. Then press back.</string>
+    <string name="exposure_grant_background_location_button">Update Settings</string>
 </resources>