From ea5280713f69a1e09798b958a95a2581c4c6aae7 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 20 Apr 2021 12:21:25 -0300 Subject: [PATCH] Address review feedback for Connect via Bluetooth UI --- .../briarproject/bramble/api/FeatureFlags.java | 2 +- .../test/BrambleCoreIntegrationTestModule.java | 2 +- .../org/briarproject/briar/android/AppModule.java | 2 +- .../android/conversation/BluetoothConnecter.java | 10 ++++++---- .../BluetoothConnecterDialogFragment.java | 15 ++++++++++++--- .../conversation/ConversationActivity.java | 2 +- briar-android/src/main/res/values/strings.xml | 5 +++-- .../briarproject/briar/headless/HeadlessModule.kt | 2 +- .../briar/headless/HeadlessTestModule.kt | 2 +- 9 files changed, 27 insertions(+), 15 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/FeatureFlags.java b/bramble-api/src/main/java/org/briarproject/bramble/api/FeatureFlags.java index e15074dd6..407e9717f 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/FeatureFlags.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/FeatureFlags.java @@ -11,5 +11,5 @@ public interface FeatureFlags { boolean shouldEnableDisappearingMessages(); - boolean shouldEnableConnectViewBluetooth(); + boolean shouldEnableConnectViaBluetooth(); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java index bb946a51a..22dea21f6 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java @@ -40,7 +40,7 @@ public class BrambleCoreIntegrationTestModule { } @Override - public boolean shouldEnableConnectViewBluetooth() { + public boolean shouldEnableConnectViaBluetooth() { return true; } }; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java index 045579ec9..5770b1021 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java @@ -300,7 +300,7 @@ public class AppModule { } @Override - public boolean shouldEnableConnectViewBluetooth() { + public boolean shouldEnableConnectViaBluetooth() { return IS_DEBUG_BUILD; } }; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecter.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecter.java index 452e7fd78..050a91e50 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecter.java @@ -110,14 +110,15 @@ class BluetoothConnecter { } boolean areRequirementsFulfilled(Context ctx, - ActivityResultLauncher permissionRequest) { + ActivityResultLauncher permissionRequest, + Runnable onLocationDenied) { boolean permissionGranted = SDK_INT < 23 || locationPermission == Permission.GRANTED; boolean locationEnabled = isLocationEnabled(ctx); if (permissionGranted && locationEnabled) return true; if (locationPermission == Permission.PERMANENTLY_DENIED) { - showDenialDialog(ctx); + showDenialDialog(ctx, onLocationDenied); } else if (locationPermission == Permission.SHOW_RATIONALE) { showRationale(ctx, permissionRequest); } else if (!locationEnabled) { @@ -126,12 +127,13 @@ class BluetoothConnecter { return false; } - private void showDenialDialog(Context ctx) { + private void showDenialDialog(Context ctx, Runnable onLocationDenied) { new AlertDialog.Builder(ctx, R.style.BriarDialogTheme) .setTitle(R.string.permission_location_title) .setMessage(R.string.permission_location_denied_body) .setPositiveButton(R.string.ok, getGoToSettingsListener(ctx)) - .setNegativeButton(R.string.cancel, null) + .setNegativeButton(R.string.cancel, (v, d) -> + onLocationDenied.run()) .show(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecterDialogFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecterDialogFragment.java index bb2d5ff82..30d3b781e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecterDialogFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/BluetoothConnecterDialogFragment.java @@ -103,13 +103,14 @@ public class BluetoothConnecterDialogFragment extends DialogFragment { // to prevent it from automatically dismissing the dialog. // The dialog is shown in onStart(), so we set the listener here later. AlertDialog dialog = (AlertDialog) getDialog(); - Button positiveButton = (Button) dialog.getButton(BUTTON_POSITIVE); + Button positiveButton = dialog.getButton(BUTTON_POSITIVE); positiveButton.setOnClickListener(this::onStartClicked); } private void onStartClicked(View v) { // The dialog starts a permission request which comes back as true - // if the permission is already granted. So it is a generic entry point. + // if the permission is already granted. + // So we can use the request as a generic entry point to the whole flow. permissionRequest.launch(ACCESS_FINE_LOCATION); } @@ -117,8 +118,16 @@ public class BluetoothConnecterDialogFragment extends DialogFragment { Activity a = requireActivity(); // update permission result in BluetoothConnecter bluetoothConnecter.onLocationPermissionResult(a, result); + // what to do when the user denies granting the location permission + Runnable onLocationPermissionDenied = () -> { + Toast.makeText(requireContext(), + R.string.toast_connect_via_bluetooth_no_location_permission, + LENGTH_LONG).show(); + dismiss(); + }; // if requirements are fulfilled, request Bluetooth discoverability - if (bluetoothConnecter.areRequirementsFulfilled(a, permissionRequest)) { + if (bluetoothConnecter.areRequirementsFulfilled(a, permissionRequest, + onLocationPermissionDenied)) { bluetoothDiscoverableRequest.launch(120); // for 2min } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java index a2ef7ffc8..32b43d9db 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java @@ -371,7 +371,7 @@ public class ConversationActivity extends BriarActivity this::showIntroductionOnboarding); } }); - if (!featureFlags.shouldEnableConnectViewBluetooth()) { + if (!featureFlags.shouldEnableConnectViaBluetooth()) { menu.findItem(R.id.action_connect_via_bluetooth).setVisible(false); } // enable alias and bluetooth action once available diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 27e3ec46c..b1ed421b9 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -171,8 +171,9 @@ Disappearing messages Connect via Bluetooth Connect via Bluetooth - Your contact needs to be in close proximity for this to work. They need to press start at the same time as you and confirm the following system dialog. - Can not continue without Bluetooth + Your contact needs to be nearby for this to work.\n\nYou and your contact should both press \"Start\" at the same time. + Cannot continue without Bluetooth + Cannot continue without location permission Connecting via Bluetooth… Successfully connected via Bluetooth Could not connect via Bluetooth diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt index 48ed124f8..58c217d4a 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt @@ -94,6 +94,6 @@ internal class HeadlessModule(private val appDir: File) { override fun shouldEnableImageAttachments() = false override fun shouldEnableProfilePictures() = false override fun shouldEnableDisappearingMessages() = false - override fun shouldEnableConnectViewBluetooth() = false + override fun shouldEnableConnectViaBluetooth() = false } } diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt index c16d6a2cf..c76212827 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt @@ -82,7 +82,7 @@ internal class HeadlessTestModule(private val appDir: File) { override fun shouldEnableImageAttachments() = false override fun shouldEnableProfilePictures() = false override fun shouldEnableDisappearingMessages() = false - override fun shouldEnableConnectViewBluetooth() = false + override fun shouldEnableConnectViaBluetooth() = false } @Provides