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.

Sign up for GitHub

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

Jump to bottom

Prevent rationale dialog from showing twice #195

Merged
merged 2 commits into from
Jan 12, 2018
Merged

Prevent rationale dialog from showing twice #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6212d0b
Prevent rationale dialog from showing twice
samtstern Jan 11, 2018
a74b918
Address review feedback
samtstern Jan 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ ext {

mavenGroup = 'pub.devrel'
mavenArtifactId = 'easypermissions'
mavenVersion = '1.1.1'
mavenVersion = '1.1.2-SNAPSHOT'

bintrayOrg = 'easygoogle'

Expand Down
15 changes: 14 additions & 1 deletion ...sions/src/main/java/pub/devrel/easypermissions/helper/BaseFrameworkPermissionsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import android.app.FragmentManager;
import android.support.annotation.NonNull;
import android.support.annotation.StyleRes;
import android.app.Fragment;
import android.util.Log;

import pub.devrel.easypermissions.RationaleDialogFragment;

Expand All @@ -11,6 +13,8 @@
*/
public abstract class BaseFrameworkPermissionsHelper<T> extends PermissionHelper<T> {

private static final String TAG = "BFPermissionsHelper";

public BaseFrameworkPermissionsHelper(@NonNull T host) {
super(host);
}
Expand All @@ -24,8 +28,17 @@ public void showRequestPermissionRationale(@NonNull String rationale,
@StyleRes int theme,
int requestCode,
@NonNull String... perms) {
FragmentManager fm = getFragmentManager();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip! I'll do what you suggested.

Man child fragment managers are a special kind of no fun.

Copy link

@Zhelyazko Zhelyazko Jan 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zhelyazko thanks for the comment anyway!


// Check if fragment is already showing
Fragment fragment = fm.findFragmentByTag(RationaleDialogFragment.TAG);
if (fragment instanceof RationaleDialogFragment) {
Log.d(TAG, "Found existing fragment, not showing rationale.");
return;
}

RationaleDialogFragment
.newInstance(positiveButton, negativeButton, rationale, theme, requestCode, perms)
.showAllowingStateLoss(getFragmentManager(), RationaleDialogFragment.TAG);
.showAllowingStateLoss(fm, RationaleDialogFragment.TAG);
}
}
16 changes: 15 additions & 1 deletion ...issions/src/main/java/pub/devrel/easypermissions/helper/BaseSupportPermissionsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import android.support.annotation.NonNull;
import android.support.annotation.StyleRes;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.util.Log;

import pub.devrel.easypermissions.RationaleDialogFragmentCompat;

Expand All @@ -11,6 +13,8 @@
*/
public abstract class BaseSupportPermissionsHelper<T> extends PermissionHelper<T> {

private static final String TAG = "BSPermissionsHelper";

public BaseSupportPermissionsHelper(@NonNull T host) {
super(host);
}
Expand All @@ -24,8 +28,18 @@ public void showRequestPermissionRationale(@NonNull String rationale,
@StyleRes int theme,
int requestCode,
@NonNull String... perms) {

FragmentManager fm = getSupportFragmentManager();

// Check if fragment is already showing
Fragment fragment = fm.findFragmentByTag(RationaleDialogFragmentCompat.TAG);
if (fragment instanceof RationaleDialogFragmentCompat) {
Log.d(TAG, "Found existing fragment, not showing rationale.");
return;
}

RationaleDialogFragmentCompat
.newInstance(rationale, positiveButton, negativeButton, theme, requestCode, perms)
.showAllowingStateLoss(getSupportFragmentManager(), RationaleDialogFragmentCompat.TAG);
.showAllowingStateLoss(fm, RationaleDialogFragmentCompat.TAG);
}
}
8 changes: 7 additions & 1 deletion ...ions/src/main/java/pub/devrel/easypermissions/helper/SupportFragmentPermissionHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.content.Context;
import android.support.annotation.NonNull;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentActivity;
import android.support.v4.app.FragmentManager;

/**
Expand All @@ -16,7 +17,12 @@ public SupportFragmentPermissionHelper(@NonNull Fragment host) {

@Override
public FragmentManager getSupportFragmentManager() {
return getHost().getChildFragmentManager();
FragmentActivity hostActivity = getHost().getActivity();
if (hostActivity != null) {
return hostActivity.getSupportFragmentManager();
} else {
return getHost().getChildFragmentManager();
}
}

@Override
Expand Down