Prevent rationale dialog from showing twice #195
Conversation
Change-Id: Idf2806ea3dc84fa34a6cbe2eae7b723a4c8b4f72
|
@SUPERCILEX any reason not to do this? |
There was a problem hiding this comment.
@samtstern Minor comments plus a warning about a possible edge case.
I've been feeling a bit under the weather this past week which is why I haven't responded as quickly as usual. 🌧 Hopefully that 3-day weekend will help me! 😄
|
|
||
| // Check if fragment is already showing | ||
| Fragment fragment = fm.findFragmentByTag(RationaleDialogFragment.TAG); | ||
| if (fragment != null && fragment instanceof RationaleDialogFragment) { |
There was a problem hiding this comment.
The null check is redundant since instanceof does it for you.
|
|
||
| // Check if fragment is already showing | ||
| Fragment fragment = fm.findFragmentByTag(RationaleDialogFragmentCompat.TAG); | ||
| if (fragment != null && fragment instanceof RationaleDialogFragmentCompat) { |
| @@ -24,8 +28,17 @@ public void showRequestPermissionRationale(@NonNull String rationale, | |||
| @StyleRes int theme, | |||
| int requestCode, | |||
| @NonNull String... perms) { | |||
| FragmentManager fm = getFragmentManager(); | |||
There was a problem hiding this comment.
There's an edge case when dealing with fragments: since the activity and child fragment managers are different, findFragmentByTag could still return null even if the dialog is showing in another manager. AFAIK, it doesn't matter which fragment manager is used to display a dialog so the SupportFragmentPermissionHelper could look like this to fix the bug:
@Override
public FragmentManager getSupportFragmentManager() {
Fragment f = getHost();
FragmentActivity activity = f.getActivity();
if (activity != null) {
return activity.getSupportFragmentManager();
} else {
// Not sure what else we can do here
return f.getChildFragmentManager();
}
}TBH, this edge case should be so rare that it doesn't really matter... your call.
There was a problem hiding this comment.
Thanks for the tip! I'll do what you suggested.
Man child fragment managers are a special kind of no fun.
There was a problem hiding this comment.
Edit: Nevermind. I just saw that there is a new version of the library (1.1.3) which has this fixed.
When a permission is requested from a support library Fragment this implementation of SupportFragmentPermissionHelper.getSupportFragmentManager() will prevent the Fragment from receiving the onRequestPermissionsResult() callback.
I was debugging a case when a rationale dialog was shown, followed by the system's Request permission dialog. As the Activity's fragment manager is used, it's the Activity which has its onRequestPermissionsResult() called, and not the Fragment's one
There was a problem hiding this comment.
@Zhelyazko thanks for the comment anyway!
Change-Id: I02907904fdb9e1b5b2db2684c8c606ad50865089
|
@SUPERCILEX thanks for the review and feel better! Enjoy the long weekend. |
|
Thanks! I plan to review the FirebaseUI PR after lunch so you'll be seeing more of me before the weekend. 😂 |
This addresses issue #193