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

fix(connectivity): getActiveNetworkInfo and NetworkInfo modern compliance #8580 #8652

Merged
merged 9 commits into from
Jul 7, 2020

Conversation

triniwiz
Copy link
Member

No description provided.

triniwiz added 3 commits June 17, 2020 17:54
feat(): expose vpn connection type
feat(): expose bluetooth connection type
feat(): expose vpn connection type
feat(): expose bluetooth connection type
@cla-bot
Copy link

cla-bot bot commented Jun 17, 2020

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @triniwiz.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@cla-bot cla-bot bot added the cla: yes label Jun 17, 2020
@NathanWalker NathanWalker changed the title fix(): #8580 fix(connectivity): #8580 Jun 17, 2020
@NathanWalker NathanWalker changed the title fix(connectivity): #8580 fix(connectivity): getActiveNetworkInfo and NetworkInfo modern compliance #8580 Jun 17, 2020
@NathanWalker NathanWalker requested a review from NathanaelA June 17, 2020 22:47
@NathanWalker
Copy link
Contributor

iOS changes lgtm - @NathanaelA can approve android changes.

@triniwiz
Copy link
Member Author

Existing apps using the connectivity module will need to add the following to the AndroidManifest.xml which should be located in app/App_Resources/Android/src/main/AndroidManifest.xml

<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>

@farfromrefug
Copy link
Collaborator

If i may jump in. I think it would be best not to use the extend syntax (android NetworkCallback). But to use constructor implementation instead. Same behavior behind but in a sense it is more readable (typings) as not seen as class extension. Also it allows to pass object for which instance methods can be used as implementation. Means it prevents instantiating functions everytime.
Finally you should use method shorthands for the methods definitions for NetworkCallback. Like 'onUnvailable:notifyCallback'

@farfromrefug
Copy link
Collaborator

@triniwiz as we discussed yesterday, extendis the best way to go as it could be the same syntax on iOS.
Rest the point about not creating funcs every time
And using extend it means you should store the newly created class for later use.

@NathanWalker NathanWalker merged commit 635f31f into master Jul 7, 2020
@NathanWalker NathanWalker deleted the fix/8580 branch July 7, 2020 01:29
@NathanWalker
Copy link
Contributor

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

Successfully merging this pull request may close these issues.

4 participants