Skip to content

Conversation

@wildseansy
Copy link

@wildseansy wildseansy commented Dec 5, 2016

See #115 for more info. @morenoh149

Closes #115

@wildseansy wildseansy force-pushed the android-permissions branch 3 times, most recently from 7f5ac46 to be94eba Compare December 5, 2016 18:46
Copy link
Owner

@morenoh149 morenoh149 left a comment

Choose a reason for hiding this comment

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

Please restore the newlines here, by restoring the carriage return or adding bullet points. And squash commits.



## API
`getAll` (callback) - returns *all* contacts as an array of objects
Copy link
Owner

Choose a reason for hiding this comment

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

looks like the whitespace removed here removed the newlines? These should each be rendered on their own line.

```

## Permissions Methods (optional)
`checkPermission` (callback) - checks permission to access Contacts.
Copy link
Owner

Choose a reason for hiding this comment

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

newlines were removed here also

README.md Outdated

#### Permissions on Android M and greater

Because permissions need need to be granted at time of use in Android M, use [`react-native-android-permissions`](https://github.com/lucasferreira/react-native-android-permissions) to request permissions if you support Android M or above.
Copy link
Owner

Choose a reason for hiding this comment

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

Because permissions need need to be granted -> Because permissions need to be granted

@morenoh149
Copy link
Owner

could you briefly explain to me now, when would an Android M user ever use the old checkPermissions api? if ever.

@wildseansy
Copy link
Author

@morenoh149 - The old checkPermissions API already doesn't really do anything on Android because older versions of android just use the manifest on install. In the README it says under Permissions Methods (optional)

On android permission request is done upon install so this function will only show if the permission has been granted.

An alternative approach would be to wrap usage of react-native-android-permissions under the checkPermissions call for android versions greater than M.

@wildseansy
Copy link
Author

@morenoh149 - removed spacing. Ready for review.

@SRandazzo
Copy link

@theseansy If i'm reading correctly it looks like that module (https://github.com/lucasferreira/react-native-android-permissions) has been merged into RN proper (facebook/react-native#9292) and the repository for react-native-android-permissions now lists it as a deprecated module!

We can now user AndroidPermissions. directly in RN

which means we could update the internal API of this module to use that as well (as you suggested)

@wildseansy
Copy link
Author

@SRandazzo - awesome. Will close.

@wildseansy wildseansy closed this Dec 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants