Feature/permission settings #199
Conversation
| @@ -78,7 +79,11 @@ public void onCreatePreferences(Bundle savedInstanceState, String rootKey) { | |||
| camera.setOnPreferenceChangeListener( | |||
| (preference, newValue) -> { | |||
| if (camera.isChecked()) startInstalledAppDetailsActivity(requireActivity()); | |||
| else PermissionUtils.requestCameraPermission(requireActivity()); | |||
| else { | |||
| if (PermissionUtils.shouldShowRational(requireActivity(), Manifest.permission.CAMERA)) | |||
There was a problem hiding this comment.
There is a constant for that.
android/app/build.gradle
Outdated
| @@ -1,6 +1,7 @@ | |||
| apply plugin: 'com.android.application' | |||
| apply plugin: 'de.undercouch.download' | |||
| apply plugin: 'com.google.firebase.crashlytics' | |||
| apply plugin: 'kotlin-android' | |||
There was a problem hiding this comment.
integrated some kotlin code, but then decided against it, so not really needed. i'll remove
| == PackageManager.PERMISSION_GRANTED) { | ||
| setupCamera(); | ||
| } else if (shouldShowRequestPermissionRationale(Constants.PERMISSION_CAMERA)) { | ||
| PermissionUtils.showCameraPermissionsControllerToast(requireActivity()); |
There was a problem hiding this comment.
This should show a toast for the camera permission, not for all the controller permissions.
There was a problem hiding this comment.
public static void showCameraPermissionsControllerToast(Activity activity) {
Toast.makeText(
activity.getApplicationContext(),
R.string.camera_permission_denied + " " + R.string.permission_reason_stream_video,
Toast.LENGTH_LONG)
.show();
}
this is the definition of the function. just one toast.
There was a problem hiding this comment.
Yes but since it comes from camera fragment, we don't know if the permission was needed for the controller or something else. As a matter of fact it is probably used for previewing the camera feed isn't it?
There was a problem hiding this comment.
right, so you want to change the message? if so, can you share some content?
| if (isGranted) { | ||
| setupCamera(); | ||
| } else { | ||
| PermissionUtils.showCameraPermissionsControllerToast(requireActivity()); |
There was a problem hiding this comment.
Why is it CameraPermissionsController?
There was a problem hiding this comment.
because toast message is same
| PermissionUtils.showCameraPermissionsControllerToast(requireActivity()); | ||
| } | ||
| break; | ||
| case PERMISSION_STORAGE: |
There was a problem hiding this comment.
Why is this needed in controller fragment?
There was a problem hiding this comment.
this is a master permission handler, to be accessed anywhere in app extending this class
There was a problem hiding this comment.
But I'm talking about the storage permission. The controller does not need storage permission afaik.
There was a problem hiding this comment.
i was using it as a global thing, so that if some other screen needs it, it could be used. but it's not used right now, so i've removed it.
There was a problem hiding this comment.
I understand if it's a general function, but here it is just for the ControllerFragment. It would be nice, if it was not required to have this function in each fragment.
There was a problem hiding this comment.
yeah so i've removed it
android/app/src/main/java/org/openbot/common/ControlsFragment.java
Outdated
Show resolved
Hide resolved
…2295/OpenBot into feature/permissionSettings # Conflicts: # android/app/src/main/java/org/openbot/common/CameraFragment.java
No description provided.