Skip to content

Update to create a notification channel for SDK 26 and up #140

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

Closed

Conversation

pauly815
Copy link

@pauly815 pauly815 commented May 8, 2018

PR Checklist

What is the current behavior?

#136
#124

What is the new behavior?

Updated background-http.android.ts to check the current SDK version. If SDK 26 or higher, a default notification channel is created and set in the notification config.

This update has been tested on a Samsung S8 emulator running Android 8 and a Samsung S6 running Android 7.

One thing to note, I tried to use the Build.VERSION_CODES.O constant but it didn't seem to work when running on a Samsung S6 with Android 7.

Fixes/Implements/Closes #[136].

@ghost ghost added the new PR label May 8, 2018
@pauly815
Copy link
Author

pauly815 commented May 8, 2018

Just realized that this is like Pull Request #126. I am using nativescript angular. Is there any update on a resolution cause my app won't work in Android 8 without these changes.

Copy link
Contributor

@lini lini left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request. I reviewed the code and have two suggestions to improve it:

  • The tns-platform-declarations reference uses API level 17, so trying to run a TypeScript compiler on the background-http plugin source files will fail because of the new methods (from API 26) in the code. This can probably be fixed with a simple declare var android; somewhere at the top of the file.

  • I see that there are two places in the code where you need to create a channel and this is done in the same way. Perhaps it will be easier to maintain if you extract the code to a private method and call it where needed.

@pauly815
Copy link
Author

@lini
I made updates per your suggestions. I re-tested on the same emulator and device as before to confirm the fix still works with the new changes.

@zbranzov
Copy link
Contributor

Hi @pauly815 ,
Can you send an app so we can reproduce the issue?
Currently, using the demo-angular sample from the repository, we are unable to reproduce your case. We are testing on Nexus 5x and Pixel 2 (both Android 8.1) and latest plugin version. Additionally, the fix seems to duplicate a native code from the native library that plugin is using https://github.com/gotev/android-upload-service/blob/e1528d3b5049a10817fdfb8745d35e7ba580a37f/uploadservice/src/main/java/net/gotev/uploadservice/UploadTask.java#L126

@pauly815
Copy link
Author

@zbranzov
I can't send the app that I'm having the issue with but I'll see if I can replicate the problem with the demo-angular app. Maybe it's a plugin conflict issue.

@pauly815
Copy link
Author

@zbranzov
Looks like the latest 3.2.6 update resolved the issue. I was able to upload in my app without the fix in this pull request.

@pauly815 pauly815 closed this May 21, 2018
@ghost ghost removed the new PR label May 21, 2018
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