Wizcorp / phonegap-plugin-localNotifications

PhoneGap plugin for providing native local notification system to JavaScript.
MIT License
38 stars 29 forks source link

Updated efficiency & implemented Notification.Builder #12

Closed fean closed 10 years ago

fean commented 10 years ago

[Warning: The Notification icon is still set to one absolute to my project environment]

aogilvie commented 10 years ago

@fean Thanks! It's a national holiday here in Japan, so i'll take a look after the long weekend! Thanks again :+1:

fean commented 10 years ago

@aogilvie Not a problem, go have some fun!

aogilvie commented 10 years ago

@fean Had a quick look through. I have suggested some minor alterations above.

fean commented 10 years ago

Indeed you're right. I'll add the defaults and set-up the readme.md to comply with the changes.

When a message is put on the statusbar it shows the ticker text and the notification icon for a second. In the case of my project it said: New eMerge notification.

fean commented 10 years ago

Ok, the title, ticker and icon property on the JavaScript add() function have now become optionals. I've also re-written the readme.md to comply with the changes I made to the API.

aogilvie commented 10 years ago

Awesome! Thanks for this.

Two slight indentation issues that would be really quick to fix: https://github.com/fean/phonegap-plugin-localNotifications/blob/v3.0/android/jp/wizcorp/phonegap/plugin/localNotification/AlarmHelper.java#L47 Line 72 in the readme in the add notification options Object example: https://github.com/fean/phonegap-plugin-localNotifications/blob/v3.0/README.md

Then this is ready to go in! Thanks!

fean commented 10 years ago

Allright, I just fixed the indentation issues. I think we're good to go.

fean commented 10 years ago

I totally forgot to implement icon in the AlarmRestoreOnBoot.onReceive() function. I'll go implement it right away.

To use the Notification.Builder.build() function android-support-v4.jar is not necessary. If you want to ensure code compatibility below Api level 5 you can use the classes in android-support-v4.jar. The notification class you are talking about which resides within the android-support-v4.jar is NotificationCompat.Builder. If you'd want to use it's build() function then you'd indeed need android-support-v4.jar.

The android documentation on Notification.Builder:

http://developer.android.com/reference/android/app/Notification.Builder.html

aogilvie commented 10 years ago

OK I ran this with the example project and it works nicely now. Thanks.

The build() compile error was in fact caused by the project.properties target=android-14, which I changed to 16 (my AndroidManifest was already min 16).

Because this is a breaking change for some users supporting < 16, i'm tagging the current v3.0 as 1.0.0 release. I'll merge your code and update the plugin.xml as 2.0.0 and tag.

Thanks for the update!