-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Full Support BigPicture(BannerMode) and Fixed issue of largeIcon using url [Android] #1444
Conversation
Import Big Image Url and largeIconUrl feature for LocalNotification in android
I'm really waiting for this one. Hope it will be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, unfortunately there is too many issues in the code and not only those listed below.
I will take a look to this feature since it might be important for some application.
I will come back to you ASAP.
channel.setDescription(this.config.getChannelDescription()); | ||
channel.enableLights(true); | ||
channel.enableVibration(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is duplicate with L829-831.
@@ -410,6 +449,7 @@ public void sendToNotificationCentre(Bundle bundle) { | |||
notification.setVibrate(vibratePattern); | |||
} | |||
|
|||
soundUri = RingtoneManager.getDefaultUri(RingtoneManager.TYPE_NOTIFICATION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line introduce a regression by forcing sounds.
Log.d(TAG, "sendNotificationWithImage: bitmao log: " + image); | ||
Log.d(TAG, "sendNotificationWithImage: bitmao log: " + largeIconImage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A LOG_TAG
constant already exist.
@@ -57,6 +70,8 @@ | |||
private static final long ONE_HOUR = 60 * ONE_MINUTE; | |||
private static final long ONE_DAY = 24 * ONE_HOUR; | |||
|
|||
private String TAG = "RNPushNotificationHelper"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A LOG_TAG
constant already exist.
@@ -145,12 +160,15 @@ public void sendNotificationScheduledCore(Bundle bundle) { | |||
} else { | |||
getAlarmManager().setExact(AlarmManager.RTC_WAKEUP, fireDate, pendingIntent); | |||
} | |||
getAlarmManager().setExact(AlarmManager.RTC_WAKEUP, fireDate, pendingIntent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is duplicate, result is:
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
if(allowWhileIdle && Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
getAlarmManager().setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, fireDate, pendingIntent);
} else {
getAlarmManager().setExact(AlarmManager.RTC_WAKEUP, fireDate, pendingIntent);
}
getAlarmManager().setExact(AlarmManager.RTC_WAKEUP, fireDate, pendingIntent);
} else {
getAlarmManager().set(AlarmManager.RTC_WAKEUP, fireDate, pendingIntent);
}
} else { | ||
if (largeIconResId != 0 && (largeIcon != null || Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP)) { | ||
notification.setLargeIcon(largeIconBitmap); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is also duplicate with L340-342
Thanks for your contribution, this helped to implement a solution to this. You can test the version on |
Now this library (react-native-push-notification) can support these features:
Note: These changes has not effect on other features (Without remove other features)
Thanks to @tidianeb5
@Dallas62 Please merge this pull request