search

NathanaelA / nativescript-permissions

Wraps up the entire Android 6 permissions system in a easy to use plugin.
MIT License
46 stars 22 forks source link

Update permissions.android.js #38

Closed aaron-burke closed 5 years ago

aaron-burke commented 5 years ago

Fix if() logic in hasAndroidX() and hasSupportVersion4()... || evaluates each part of the if, so if androidx.** doesn't exist it will die.

NathanaelA commented 5 years ago

@aaron-burke -

Thank you for your PR; I really appreciate them...

if (!android.support || !android.support.v4 || !android.support.v4.content || !android.support.v4.content.ContextCompat || !android.support.v4.content.ContextCompat.checkSelfPermission) {
        return false;

JavaScript evaluates from Left to Right in an if statements; so at the first OR statement that is true (i.e. no !android.support) - it will automatically return false without evaluating the rest of the if statement.

What exactly caused you to think this was needed; as I tested this in both the androidx and the android.support versions of the runtimes to verify it detected the correct path...

aaron-burke commented 5 years ago

Yes, you are correct, I took a quick look and assumed the ||'s were allowing later evaluation, but the !(not) should do as you say, return false...

The problem I get in trying someone else's demo project that uses your plugin is as follows:

System.err: ReferenceError: androidx is not defined System.err: File: "file:///data/data/org.nativescript.demong/files/app/tns_modules/nativescript-permissions/permissions.js, line: 178, column: 1

Just seeing if I can fix the code now, I'll close this pull request in a moment and try to find what is going on.

NathanaelA commented 5 years ago

You know I'm pretty sure I know what the issue is with "androidx". I'm not sure why it would have originally worked if it is what I think it is... Give me a few minutes to test...