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.

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

Remove a workaround for targetSdkVersion < 23 #305

Closed
hotchemi opened this issue Apr 29, 2017 · 7 comments
Closed

Remove a workaround for targetSdkVersion < 23 #305

hotchemi opened this issue Apr 29, 2017 · 7 comments
Labels
Milestone

Comments

@hotchemi
Copy link
Member

hotchemi commented Apr 29, 2017

Overview

  • Currently we have a workaround that checks PermissionUtils.getTargetSdkVersion(target) < 23 to make sure that the library works under Marshmallow device.
  • This feature is requested by a few user initially, but it's over one and a half year since the library came out so it's okay to remove this workaround to avoid unnecessary check for most of user.
    static void onRequestPermissionsResult(MainActivity target, int requestCode, int[] grantResults) {
        switch (requestCode) {
            case REQUEST_SHOWCAMERA:
                if (PermissionUtils.getTargetSdkVersion(target) < 23 && !PermissionUtils.hasSelfPermissions(target, PERMISSION_SHOWCAMERA)) {
                    target.onCameraDenied();
                    return;
                }
                if (PermissionUtils.verifyPermissions(grantResults)) {
                    target.showCamera();
                } else {
                    if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_SHOWCAMERA)) {
                        target.onCameraNeverAskAgain();
                    } else {
                        target.onCameraDenied();
                    }
                }
                break;
            default:
                break;
        }
    }
@hotchemi hotchemi added this to the 2.4.0 milestone Apr 29, 2017
@mannodermaus
Copy link
Contributor

I feel like, at this point, we can force our users to use a targetSdkVersion of at least 23. I'm interested whether we can check this in the annotation processor, actually. I'd like to raise an Exception on lower Target SDK levels, just because we can avoid users opening issues regarding a regression in support.

@hotchemi
Copy link
Member Author

hotchemi commented Apr 29, 2017

Umm, I'd say it's a bit strong restriction.
The reason of checking getTargetSdkVersion is to deal with the case that user turn off a permission from setting page and back to app on under Marshmallow device, but I think it's too rare case to take care for now.
In other words about 99% case PermissionsDispatcher assures the correct action even if taretSdkVersion is lower than 23. Thus showing a caution message or something is fine for sure, but raising Exception seems like too strong 😺

@mannodermaus
Copy link
Contributor

mannodermaus commented Apr 29, 2017

Ah, I see now. However, if we keep this conditional logic anyway (to show the "caution message" you're referring to, for instance), why not just keep the current functionality? I agree that we can could clean up the generated code here if we removed the targetSdkVersion check altogether, but if we just remove the behavior and keep the conditional, I'd prefer the current way.

@hotchemi
Copy link
Member Author

hotchemi commented Apr 29, 2017

Well, of course we can keep that but getTargetSdkVersion method accesses to package manager and parse the manifest file so it's not a light method I guess. And another reason is that it's been a long time since Marshmallow being out so it's not early to encourage user to update targetSdkVersion to 23:D
If user stick to the older version they can use old ver of PermissionsDispatcher, the library is already matured so it's not a cruel attitude to conservative user.

@mannodermaus
Copy link
Contributor

mannodermaus commented Apr 29, 2017

I completely agree with you, especially since we've arrived at Target SDK 26 by now! That was my motivation regarding #305 (comment), which we can use to clean up the generated code. If a user really wants to stay at 22 or lower (for reasons unknown), they can use the current stable version. Going forward, we would then present the "caution message" during compile-time. It doesn't have to be a build-failing error - a warning would be just fine. Again, we would have to find out if we can get the current Target SDK from inside the annotation processor, because I don't know.

@SolveSoul
Copy link

SolveSoul commented Jul 4, 2017

Is there any action needed for this? My project can't build anymore throwing exactly this error:

Error:(50, 26) error: cannot find symbol method getTargetSdkVersion(PermissionsActivity)

My targetSdkVersion is set to 25 and my Gradle has the following included:

    apt 'com.github.hotchemi:permissionsdispatcher-processor:2.4.0'
    compile 'com.github.hotchemi:permissionsdispatcher-processor:2.4.0'

EDIT: NVM, I forgot to update my apt

@hotchemi
Copy link
Member Author

hotchemi commented Jul 4, 2017

you should add runtime module as compile dependency.

 compile 'com.github.hotchemi:permissionsdispatcher:2.4.0'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants