io-app icon indicating copy to clipboard operation
io-app copied to clipboard

fix: [IABT-1386] Fix the Android camera permissions request

Open dgopsq opened this issue 2 years ago • 2 comments

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.

dgopsq avatar Aug 03 '22 10:08 dgopsq

Affected stories

  • 🐞 IABT-1386: Manca finestra modale per autorizzazione accesso alla fotocamera su Android

Generated by :no_entry_sign: dangerJS against 97d44d9b37013795dc6868380dd9df5196f26e36

pagopa-github-bot avatar Aug 03 '22 11:08 pagopa-github-bot

Codecov Report

Merging #4095 (97d44d9) into master (0ec6012) will decrease coverage by 0.01%. The diff coverage is 0.00%.

Impacted file tree graph

@@            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.

codecov[bot] avatar Aug 03 '22 11:08 codecov[bot]

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?

dgopsq avatar Aug 26 '22 14:08 dgopsq

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.

emiliopavia avatar Aug 26 '22 15:08 emiliopavia

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 returns PERMISSION_DENIED, call shouldShowRequestPermissionRationale(). 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.

dgopsq avatar Aug 29 '22 08:08 dgopsq