Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Permissions] PermissionRequired switches to permissionNotAvailableContent when clicking outside of the permission request #549

Closed
fschuetz04 opened this issue Jul 9, 2021 · 13 comments · Fixed by #990
Assignees

Comments

@fschuetz04
Copy link

fschuetz04 commented Jul 9, 2021

Describe the bug

When using the PermissionRequired composable, it first shows permissionNotGrantedContent. When calling permissionState.launchPermissionRequest(), the permission request is launched. If the user clicks on Deny, permissionNotGrantedContent is still shown. If the permission is requested again and the user clicks on Deny again, permissionNotAvailableContent is shown. If the user clicks outside of the permission request the first time, permissionNotAvailableContent is shown immediately. In my opinion, this is inconsistent.

To Reproduce

Example code:

package com.example.app

import android.Manifest
import androidx.compose.material.Button
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import com.google.accompanist.permissions.ExperimentalPermissionsApi
import com.google.accompanist.permissions.PermissionRequired
import com.google.accompanist.permissions.rememberPermissionState

@ExperimentalPermissionsApi
@Composable
fun PermissionTest() {
    val permissionState = rememberPermissionState(Manifest.permission.READ_EXTERNAL_STORAGE)

    PermissionRequired(
        permissionState = permissionState,
        permissionNotGrantedContent = {
            Button(onClick = {
                permissionState.launchPermissionRequest()
            }) {
                Text("Grant permission")
            }
        },
        permissionNotAvailableContent = {
            Text("Permission not available")
        }
    ) {
        Text("Permission granted")
    }
}

Expected behavior

I would expect a different behavior. Either, clicking outside the request shows permissionNotGrantedContent, or denying the request immediately shows permissionNotAvailableContent.

Environment:

  • Android OS version: Android 11
  • Device: Emulator (Google Pixel 4a)
  • Accompanist version: v0.13.0
@manuelvicnt
Copy link
Collaborator

This bug is a tricky one for multiple reasons:

  • Android 11 allows clicking outside the permissions dialog. Other APIs do not allow this.
  • When clicking outside the dialog, the activity result callback is called saying the permission is not granted, but the system also says the rationale shouldn't be shown.

I agree with you that the UX in this case is far from ideal. However, the system doesn't give us enough information to know the user hasn't interacted with the dialog.

I'll ask members of the permissions team to see if there's a better way to handle this case. Thank you!

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Aug 12, 2021
@manuelvicnt manuelvicnt removed the stale Stale issues which are marked for closure label Aug 12, 2021
@fschuetz04
Copy link
Author

fschuetz04 commented Aug 16, 2021

If keeping state across app restarts is possible, this would be solvable. Let's say we start in state A and ActivityResultContracts.RequestPermission() returns false (permission denied):

  • State A and should show rationale: First request, user denied it → Go to state B
  • State A and should not show rationale: First request, user clicked outside the permission dialog → Stay in state A
  • State B and should show rationale: Second request, user clicked outside the permission dialog → Stay in state B
  • State B and should not show rationale: Second request, user denied it → Go to state C

State A, B, and C could each show a different user-provided screen (or A and B the same one). In State C, no permission should be requested.

If the permission is granted (either via a request or via the app settings), the state must be reset to state B, since as far as I'm concerned, the permission request dialog is shown only once after the user denies the permission through settings again.

This is probably not in scope to implement, right?

Maybe the user could be made responsible to manage the state. If so, not managing the state and getting the current behavior should be possible of course. Maybe via another set of composables.

@fschuetz04
Copy link
Author

fschuetz04 commented Aug 31, 2021

BTW, if you're interested in code: https://gist.github.com/fschuetz04/bde1bc46cfd914bdb25689ea5be40c7f. The state value must be kept across restarts by the user, otherwise this does not work.

@github-actions
Copy link

github-actions bot commented Oct 1, 2021

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Oct 1, 2021
@manuelvicnt manuelvicnt removed the stale Stale issues which are marked for closure label Oct 3, 2021
@alexvanyo
Copy link
Collaborator

The issue with keeping some sort of persistent state across app restarts is that there's no guarantee that it stays in sync with the underlying state of the system, which could happen when denying or giving permission in the system settings, or if the user clears out app data locally.

@fschuetz04
Copy link
Author

fschuetz04 commented Oct 14, 2021

As far as my tests went, after denying in settings, the app is allowed to ask for permission once. So by setting the state to one request left whenever the permission is granted, this works out fine.

When app data is cleared, the app is allowed to ask for permission twice again, so this works fine too.

I tested this on Android 11, don't know if it is different on other versions.

@alexvanyo
Copy link
Collaborator

Ah, I see the importance of this block now:

            if (permissionGranted && state != PermissionRequiredState.OneRequestLeft) {
                onStateChange(PermissionRequiredState.OneRequestLeft)
            }

This is admittedly unlikely, but does this handle the case where the state is NoRequestsLeft, the app isn't open, and the user toggles the permission of the app in settings to "Allow" or "Ask every time," and then toggles it back to "Don't allow"?

I think in that situation the app would continue to show denied, since it never got a chance to see a state where the permission was granted, so the persisted state would still be NoRequestsLeft, even though there's the opportunity to ask for permission again directly.

@fschuetz04
Copy link
Author

Great question, I did not think about this scenario. I mean it's not as critical, but still. I'll test it and get back.

@fschuetz04
Copy link
Author

fschuetz04 commented Oct 15, 2021

I got to check it now. As expected, this does not handle the case you described. The app is allowed to ask for permission once, but thinks it is not allowed to ask.

Again, I don't think it's critical since it's a rare case and opening app settings works fine. I don't think there is any other way to handle this case, but please correct me if I'm wrong.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Nov 14, 2021
@manuelvicnt manuelvicnt removed the stale Stale issues which are marked for closure label Nov 15, 2021
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Dec 17, 2021
@alexvanyo alexvanyo removed the stale Stale issues which are marked for closure label Dec 20, 2021
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Jan 20, 2022
@manuelvicnt manuelvicnt removed the stale Stale issues which are marked for closure label Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants