moko-permissions icon indicating copy to clipboard operation
moko-permissions copied to clipboard

Add `NotGranted` to `PermissionState` for Android

Open jacobras opened this issue 2 years ago • 1 comments

Here's one idea to fix the issue with [deny and don't ask again] returning an incorrect state (#104), by adding a new state for Android: NotGranted. It's the most accurate description of the situation: it could be NotDetermined or DeniedAlways but there's no way to determine the difference between [deny] and [always deny] when requesting the state. One has to keep track of whether a permission has been requested in the app already, so that's up to consumers.

This change does not affect iOS' behaviour, nor providePermission()/isPermissionGranted() behaviour. Only on for requesting the current state on Android.

Alternative solution would be to add no different state, but make Android return Denied by default. Benefit is that the state enum doesn't change and existing apps would be likely to keep working. Downside is that the default state is Denied and that could look like a bug. But again, a documentation update helps.

Before

See screen capture in #104

After

MokoPermissionStateBugAndroidFixed

jacobras avatar Feb 29 '24 12:02 jacobras

Looking for opinions here on the 3 possible solutions I see :) One issue is that now NotDetermined is never used on Android, making it iOS-only. So the whole thing would look like this:

1️⃣ This PR:

  • NotDetermined iOS-only
  • NotGranted Android-only
  • Granted Both
  • Denied Android-only
  • DeniedAlways Both, but on Android only for push

Can't say that looks ideal.

2️⃣ The alternative approach I mentioned in the description above (returning "denied" by default on Android) would look like this:

  • NotDetermined iOS-only, starting state
  • Denied Both, starting state on Android
  • Granted Both
  • DeniedAlways Both, but on Android only for push

Of course, that changes behaviour for existing apps using this library.

3️⃣ Simplest change would be no behaviour change, but only a documentation update. Then technically it remains as it's currently:

  • NotDetermined Both, but on Android also meaning "denied always"
  • Denied Both
  • Granted Both
  • DeniedAlways Both, but on Android only for push

jacobras avatar Mar 08 '24 10:03 jacobras

looks good i check with changes on both platforms

ZiXOps avatar Apr 12 '24 10:04 ZiXOps