accompanist icon indicating copy to clipboard operation
accompanist copied to clipboard

[Permissions] Support an API to handle the "need settings" for permission

Open danielesegato opened this issue 2 years ago • 10 comments

Description The only way for an app to detect if the user denied the permission permanently during a permission request is to compare the "shouldShowRationale" before and after the permission request itself (I always hated this of the permission API, but it is what it is).

These are the cases for denied requests (boolean before / after are the showRationale value before and after lauching the request, shown indicates if the request has actually been presented to the user, assuming they are all denies):

+--------+-------+----+--------+--------------------------------------------+
| Before | After | -> | Shown? | Interpretation                             |
+--------+-------+----+--------+--------------------------------------------+
|  false |  true |    |   yes  | 1st request, denied                        |
+--------+-------+----+--------+--------------------------------------------+
|  true  |  true |    |   yes  | Nth request, denied standard               |
+--------+-------+----+--------+--------------------------------------------+
|  true  | false |    |   yes  | Nth request, denied with do-not-ask-again  |
+--------+-------+----+--------+--------------------------------------------+
|  false | false |    |   no   | Nth request, denied should send to setting |
+--------+-------+----+--------+--------------------------------------------+

As developers we need to identify the last case so that we know we have to open the settings for the permission. The 3rd case we do not need to do anything, the user just answered with a deny, so that's a deny, it's next time (4th) that we will have to open settings.

The current API doesn't provide any way to handle this case.

Rationale Flag A complication: in some occasion you might want to show the user a different Rationale when you know you need to go into settings.

Proposal Change the current onPermissionResult callback from providing a granted boolean to a new PermissionResult entity that gives the full picture:

  • Granted
  • Denied(doNotAskAgain = false)
  • Denied(doNotAskAgain = true)
  • NotAsked

And provide a new launchPermissionSettingRequest() that send the user to settings.

For multiple permission the map would become Map<String, Boolean> --> Map<String, PermissionResult>

This is still not ideal because we, developers, need to manually handle a state to know when we should show the setting rationale, but at present I wouldn't know how to simplify this.

danielesegato avatar Oct 03 '22 14:10 danielesegato

See https://github.com/google/accompanist/pull/990 for the rationale for the current API design.

In your table, the false -> false case also can occur when the user hasn't been asked for the permission before, and then cancels the first permission request (presses the back button, or taps outside the dialog)

alexvanyo avatar Oct 03 '22 17:10 alexvanyo

Thanks. I've read the rationale but i disagree with it.

As others users pointed out you can't make that decision if you don't know if the user was asked.

I understand there are corner cases but i think a library managing permissions should let me have all the options.

I'm not sure why the last comment say that the library can't handle that. If the library can't how can we?

Inside the result callback we don't even have access to the code that allow us to check the "shouldShowRationale" of that permission.

Overall i think this library is incomplete without something that tell the developer if the permission was shown to the user or not.

I believe the case false -> false, with cancel is detectable by the library but not by the library user.

danielesegato avatar Oct 04 '22 05:10 danielesegato

@alexvanyo I agree wee need to extend permission callback. We also have some case on Android 11

The same case -> Ask USer to go the settings if permission was denied permanently

  1. Ask user about permission couple of time (user deny it) or user choose never ask again
  2. Ask user to change permission in settings (Show custom dialog)
  3. User choose Ask every time in permissions settings
  4. The permission status is Denied with shouldAskRationaly false and permission is asked. We are not able to handle is it asked or not ? Also from point 2 remember we need to ask user to go to settings.

Our propose: permissionState.launchPermissionRequest() returns Boolean (isPermissonAsked)

Or event better. extend PermissionStatus to

PermissionStatus.Granted
PermissionStatus.Denied(shouldShowRationaly)
PermissionStatus.AskEveryTime / Denied(askEveryTime)

lebedynskyi avatar Oct 11 '22 13:10 lebedynskyi

image I had same issue with this. I wanted to navigate the user to app settings if permission dialog is permanently denied but this case does not exist neither in library or android documentation. image

Basically from the code and the diagram above we only have 2 states: GRANTED and NOT_GRANTED. DENIED is not valid here since DENIED is not equals to NOT_GRANTED, because first time permission state is NOT_GRANTED but is not DENIED as well.

In the end what I did was combination from first comment and state.

`var permissionRationaleShown by remember { mutableStateOf(false) } var snackBarSettingsShown by remember { mutableStateOf(false) }

var shouldShowRationaleBeforeCalling by remember {
    mutableStateOf(false)
}

val permissionsState = rememberMultiplePermissionsState(
    requiredPermissions.toList(),
    onPermissionsResult = { permissions ->
        permissions.entries.filter { permissionResult ->
            !permissionResult.value
        }.firstOrNull { permissionResult ->
            !ActivityCompat.shouldShowRequestPermissionRationale(
                context.getActivity(),
                permissionResult.key
            ) && shouldShowRationaleBeforeCalling
        }?.let {
            snackBarSettingsShown = true
        }
    }
)

Scaffold(snackbarHost = { SnackbarHost(snackBarHostState) }) {
    if (snackBarSettingsShown) {
        LaunchedEffect(key1 = snackBarHostState) {
            val result = snackBarHostState
                .showSnackbar(
                    permanentlyDeniedMessage,
                    context.getString(R.string.open_settings),
                    duration = SnackbarDuration.Long
                )
            snackBarSettingsShown = when (result) {
                SnackbarResult.Dismissed -> {
                    false
                }
                SnackbarResult.ActionPerformed -> {
                    context.openApplicationSettings()
                    false
                }
            }
        }
    }
    if (permissionRationaleShown) {
        PermissionRationaleDialog(
            title = rationaleTitle,
            desc = rationaleDescription,
            onClick = {
                shouldShowRationaleBeforeCalling = permissionsState.shouldShowRationale
                permissionsState.launchMultiplePermissionRequest()
                permissionRationaleShown = false
            },
            onDismiss = { permissionRationaleShown = false }
        )
    }

    if (permissionsState.allPermissionsGranted) {
        content(
            PermissionState.Granted
        )
    } else {
        if (permissionsState.shouldShowRationale) {
            content(
                PermissionState.NotGranted {
                    permissionRationaleShown = true
                }
            )
        } else {
            content(
                PermissionState.NotGranted {
                    shouldShowRationaleBeforeCalling = permissionsState.shouldShowRationale
                    permissionsState.launchMultiplePermissionRequest()
                }
            )
        }
    }
}`

So we have 2 UI states: GRANTED and NOT_GRANTED. In NOT_GRANTED state we can lauchPermission and only inside result we can check should we show the settings.

mzukic3 avatar Oct 18 '22 18:10 mzukic3

The "do not ask again" use case cannot be added to this Accompanist wrapper unless the platform APIs support it. There's no way to guarantee this behavior in past and future versions of Android. Any potential workaround we try to add now might not work in the future.

@danielesegato a library managing permissions should let me have all the options

It seems like there's an expectation mismatch here. This library is a wrapper on top of the current platform APIs that makes them easier to consume in Compose apps. This library doesn't aim to be the silver bullet for permissions on Android apps.

I've read the rationale but i disagree with it

It's definitely fine to disagree :D but unfortunately, it is what it is. If you want to support that use case, I'm afraid you'll need to be responsible for it maintain it yourself.

manuelvicnt avatar Nov 02 '22 09:11 manuelvicnt

Thanks, I did create my own solution.

What I meant when I said "I disagree" was that I don't think handling those "happy path" is enough for any app; with few exceptions. Speaking from an UX perspective having a button that when pressed does nothing is never an acceptable user experience, even if the user caused it with the double deny. I hope we can at least partially agree on this.

I also understand your point about the API not supporting it (always found that very annoying even before compose, maybe you can explain what lead to the decision of not exposing that information?).

Would it help if I create a feature request on the android issue tracker asking to support that case in the API? Basically adding a needSetting boolean or the opposite would be enough. I'm not sure how things works inside Google, hope you can provide feedback to the other departments :-)

I'd rather use a solution provided by you than my own! Thanks for your answer!

danielesegato avatar Nov 03 '22 22:11 danielesegato

Speaking from an UX perspective having a button that when pressed does nothing is never an acceptable user experience, even if the user caused it with the double deny. I hope we can at least partially agree on this.

I totally agree on this :D IMO, it's a terrible UX and I'd probably do something different in my own app

Would it help if I create a feature request on the android issue tracker asking to support that case in the API?

Yeah, that would be great! I'd be surprised if there aren't hundreds of issues requesting this functionality. I'm not sure if those have been closed or not. But it's worth trying again.

hope you can provide feedback to the other departments

We've been providing feedback but it's a sensitive topic and we haven't been successful yet :( we feel this pain too.

Thanks for your feedback!

manuelvicnt avatar Nov 04 '22 07:11 manuelvicnt

@danielesegato Mind sharing your implementation?

ForceGT avatar Nov 09 '22 14:11 ForceGT

@ForceGT Overall I'm not happy with it and I'm pushing to change the UX so that I do not need that at all. And I would have to go through some legality before I could do it anyway.

After working on it I can second what Manuel said: until the platform expose you the information any kind of heuristic you adopt to detect the situation is just going to be hacky and error prone / not robust.

I still think the platform SHOULD expose that information, but until it does I think it's better to just design an UX that doesn't need to know.

I can give you an idea of how my solution works, for the solo purpose of convincing you that you are better of designing a different UX; as I said I consider it an unreliable hack

I've created custom ActivityResultContract for permission requests (both single and multiple). These contracts receive a data class of the request: which permission is being asked and what was the state of shouldShowRationale at the time of asking for the permission.

In createIntent() the request data class is saved in a private static variable store in the contract companion object (ich!) and when getSynchronousResult() or parseResult() are invoked they return the last request along with the result of the permission request.

        companion object {
            private lateinit var lastInput: Map<String, InternalPermissionRequest>
            private var lastInputTag: Any? = null
       }
       
       // ..
       
        override fun createIntent(...) {
            lastInput = input.requests.associateBy { it.permission }
            lastInputTag = input.tag
            /// ...
        }
        
        // ...

This isn't great (the static save part) but it was the quickies way to make it work... I could have done better with some singleton to persist the information in case of process death or to avoid conflicts between concurrent permission requests... But they are both very unlikely situation that can be handled with some fallback...

The "compose" state receive this result and compare compare the initial value of shouldShowRationale with the new one to detect if the permission request was shown to the user...

A problem this have is that if you go from should rationale = false to permission denied + should rationale = false my code currently detect this as a "the request wasn't shown to the user" but it could also mean the user ignored it (back)...

Manuel was right again here.

The user receive this data class instead of the boolean in the result callback:

data class PermissionResult(
    val state: PermissionState,
    val requestShownToUser: Boolean = true, // <--- this is the detection result
    val tag: Any? = null,
)

As you can see I did some more modifications to the library like allowing to tag a request to know where the permission request come from (was it requested when pressing button A or B?). And I provide the full permission state.

My code also keep and expose the information of "needSetting" in the permission State once it is detected, but clear it on resume if the rationale changes.

    data class Denied(
        val shouldShowRationale: Boolean,
        val needSetting: Boolean = false,
    ) : PermissionStatus

There are more things I did with the multi-request. I think overall when the UX will be updated I'll get rid of the needSetting / requestShownToUser and keep the rest of the modifications.

danielesegato avatar Nov 11 '22 08:11 danielesegato

I have implemented an extension of the PermissionState to handle exactly this. Let me know what you think and any improvements are welcome. Manually tested and works well.

Essentially I leverage the onPermissionResult passed to rememberPermissionState and carry out the following logic.

  1. If we can show rationale to the user, don't do anything.
  2. If we could previously show the rationale to the user, but can no longer, then next launchPermissionRequest will invoke the onCannotRequestPermission() callback.
  3. If previously we couldn't show the rationale to the user and we still can't, then we were already at the stage of double denial and invoke the onCannotRequestPermission() callback.
@ExperimentalPermissionsApi
@Composable
fun rememberPermissionState(
    permission: String,
    onCannotRequestPermission: () -> Unit = {},
    onPermissionResult: (Boolean) -> Unit = {},
): ExtendedPermissionState {
    val context = LocalContext.current

    var currentShouldShowRationale by remember {
        mutableStateOf(context.findActivity().shouldShowRationale(permission))
    }

    var atDoubleDenialForPermission by remember {
        mutableStateOf(false)
    }

    val mutablePermissionState = rememberPermissionState(permission) { isGranted ->
        if (!isGranted) {
            val updatedShouldShowRationale = context.findActivity().shouldShowRationale(permission)
            if (!currentShouldShowRationale && !updatedShouldShowRationale)
                onCannotRequestPermission()
            else if (currentShouldShowRationale && !updatedShouldShowRationale)
                atDoubleDenialForPermission = false
        }
        onPermissionResult(isGranted)
    }

    return remember(permission) {
        ExtendedPermissionState(
            permission = permission,
            mutablePermissionState = mutablePermissionState,
            onCannotRequestPermission = onCannotRequestPermission,
            atDoubleDenial = atDoubleDenialForPermission,
            onLaunchedPermissionRequest = {
                currentShouldShowRationale = it
            }
        )
    }
}

@OptIn(ExperimentalPermissionsApi::class)
@Stable
class ExtendedPermissionState(
    override val permission: String,
    private val mutablePermissionState: PermissionState,
    private val atDoubleDenial: Boolean,
    private val onLaunchedPermissionRequest: (shouldShowRationale: Boolean) -> Unit,
    private val onCannotRequestPermission: () -> Unit
) : PermissionState {
    override val status: PermissionStatus
        get() = mutablePermissionState.status

    override fun launchPermissionRequest() {
        onLaunchedPermissionRequest(mutablePermissionState.status.shouldShowRationale)
        if (atDoubleDenial) onCannotRequestPermission()
        else mutablePermissionState.launchPermissionRequest()
    }
}

Here is an example of its use.

      val pushPermissionState = rememberPermissionState(
          permission = Manifest.permission.POST_NOTIFICATIONS,
          onCannotRequestPermission = { launcher.launch(context.buildNotificationSettingsIntent()) }
      )

      if (pushPermissionState.status.isGranted) {
          onPushesEnabled()
      } else {
          GraphicInfo(
              customisation = customisation,
              state = state,
              onEnablePushesPressed = {
                  pushPermissionState.launchPermissionRequest()
              },
              onSkipped = onSkipped
          )
      }

StuStirling avatar Nov 24 '22 14:11 StuStirling

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[bot] avatar Jan 09 '23 03:01 github-actions[bot]

Completed? Isn't this "won't fix"?

danielesegato avatar Jan 14 '23 17:01 danielesegato

As I mentioned in previous comments, the "do not ask again" use case cannot be added to this Accompanist wrapper unless the platform APIs support it, we cannot guarantee the same behaviour in current and future Android API versions. Also, the Android team doesn't consider sending people to the settings screen to grant a permission a good practice / UX experience.

This library is extensible enough for you to provide those use cases if you wanted to. But for now, we aren't going to implement them in the library.

Thank you for the discussions!

manuelvicnt avatar Jan 16 '23 09:01 manuelvicnt