permissions-dispatcher / PermissionsDispatcher

A declarative API to handle Android runtime permissions.
https://github.com/permissions-dispatcher/PermissionsDispatcher
Apache License 2.0
11.22k stars 1.44k forks source link

[ktx] Issue: on denied permission callback called immediately before showing rationale #684

Closed allanconda-mercari closed 4 years ago

allanconda-mercari commented 4 years ago

Overview

ViewModel unexpectedly re-emits the previous decision (deny) when running the fragment for the second time immediately after denying for the first time.

Setup:

Expected

Deny callback is not triggered immediately on re-enter of fragment with permission. Should be same behavior as reentering the Fragment after relaunching the app.

Actual

Deny callback is triggered immediately. And then Rationale callback is called as well. Since deny callback has been called, the fragment is already removed in the backstack. If I proceed with the rationale and press the Deny again, it will call the on deny callback again. Deny callback will be called twice in total.

If I re-launch the app fresh the deny callback will only be called on press of deny button.

Environment

I am trying to migrate my kapt permissions code into ktx, and encountered this issue (don't have this issue with kapt).

As for the fix, I think for fragments we should be passing the fragment as the lifecycle owner, and then observe with the Fragment.viewLifecycleOwner for the permissionRequestResult

Reproducible steps

  1. Open the fragment requiring permission
    • permission dialog shows up*
  2. Press deny
    • Deny callback is triggered, in my case I exit the fragment (popBackStack) *
  3. Attempt to open the fragment again
hotchemi commented 4 years ago

let us check, thx!

hotchemi commented 4 years ago

https://github.com/permissions-dispatcher/PermissionsDispatcher/pull/689

hotchemi commented 4 years ago

The issue has been resolved in 1.0.1 I guess 🙏 Let us know if it has not been addressed 🙇

hotchemi commented 4 years ago

Let me close the issue.

allanconda-mercari commented 4 years ago

Hello! I hadn't checked this in a while. Glad it's resolved! I'll update the version and confirm.