io-app
io-app copied to clipboard
fix: [IABT-1386] Fix the Android camera permissions request
Short description
This PR is going to add the OS permissions request in the QR Code Scanner screen after the rationale showed up on Android.
https://user-images.githubusercontent.com/5963683/183673717-95943319-1c9b-4698-878b-0334dd8dedbc.mov
List of changes proposed in this pull request
- Moved the rationale logic inside the
BarcodeCamera
component
How to test
On Android, remove all the permissions from the emulator using adb shell pm reset-permissions
and navigate to the QR Code Scanner screen. An alert should show up and after pressing the button another alert with the relative permissions request should appear. Giving the correct permissions should make the camera available in the screen.
Affected stories
- 🐞 IABT-1386: Manca finestra modale per autorizzazione accesso alla fotocamera su Android
Generated by :no_entry_sign: dangerJS against 97d44d9b37013795dc6868380dd9df5196f26e36
Codecov Report
Merging #4095 (97d44d9) into master (0ec6012) will decrease coverage by
0.01%
. The diff coverage is0.00%
.
@@ Coverage Diff @@
## master #4095 +/- ##
==========================================
- Coverage 47.12% 47.11% -0.02%
==========================================
Files 1292 1292
Lines 25723 25729 +6
Branches 6876 6881 +5
==========================================
Hits 12122 12122
- Misses 13551 13557 +6
Partials 50 50
Impacted Files | Coverage Δ | |
---|---|---|
ts/components/BarcodeCamera.tsx | 29.57% <0.00%> (-6.63%) |
:arrow_down: |
ts/screens/wallet/payment/ScanQrCodeScreen.tsx | 9.09% <ø> (+0.75%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 0ec6012...97d44d9. Read the comment docs.
If I definitely refuse the permission, every time I access the feature this prompt is presented, but nothing happens when tapping on "Scegli". It could be better to prevent the alert to be shown if the user has chosen so. As a fallback there's the "Settings" button in the screen that can be used to revert the setting. What do you think?
![]()
Ok, so, this is actually a tricky bug. Fact is: we don't really know whether a user denied manually the permission or if it's already denied automatically by the OS. The solution I came up by is using PermissionsAndroid.request
on Android. This method automatically manages the rationale behaviour following the OS requests. On Android 12 for example, the rationale won't be displayed the first time, but only when the permission has been denied a first time.
This solution changes the current rationale logic, delegating it to the OS. Could it be a good fix?
This solution changes the current rationale logic, delegating it to the OS. Could it be a good fix?
For me it's ok, if it's follow the Android HIG.
This solution changes the current rationale logic, delegating it to the OS. Could it be a good fix?
For me it's ok, if it's follow the Android HIG.
Ok so, the PermissionsAndroid.request
definitely uses the shouldShowRequestPermissionRationale
method under the hood.
The Android guidelines are quite vague and there are various StackOverflow posts about how weird it is the shouldShowRequestPermissionRationale
behaviour, which won't show the rationale the very first time a user is prompted with the permission request.
I think that following the shouldShowRequestPermissionRationale
direction should be enough, as stated here:
After the user views an educational UI, or the return value of
shouldShowRequestPermissionRationale()
indicates that you don't need to show an educational UI this time, request the permission. Users see a system permission dialog, where they can choose whether to grant a particular permission to your app.
and here:
If the
ContextCompat.checkSelfPermission()
method returnsPERMISSION_DENIED
, callshouldShowRequestPermissionRationale()
. If this method returns true, show an educational UI to the user. In this UI, describe why the feature, which the user wants to enable, needs a particular permission.
This is the only way if we don't want to leverage our persistent storage in order to show the rationale only the very first time the user needs to set the permission.