fix(local-notifications): Notifications are not generated when using defaults#602
Conversation
| for (const s of scheduleOptions) { | ||
| const entry = LocalNotificationsImpl.createScheduleEntry(s); | ||
| // Some properties should not be inherited by options | ||
| const nativeOptions: ScheduleNativeOptions = { ...entry, color: undefined, notificationLed: undefined }; |
There was a problem hiding this comment.
What is the idea of this extend? Won't this always nullify the properties that are sent by users?
There was a problem hiding this comment.
Not at all. The nativeOptions object is meant for being sent to the native side as a json string.
In line 223, we mark color and notificationLed as undefined to:
- Prevent them from being read from native lib in case they're not set below (because they're set below)
- Correct type warnings
| registerNotification(entry.id, content, trigger, scheduledIds); | ||
|
|
||
| if (interval && entry.displayImmediately) { | ||
| const id = LocalNotificationsImpl.generateNotificationID(); |
There was a problem hiding this comment.
Unlike the old logic, won't this generate a new ID. What will happen if I decide to cancel the ID of the original request? To me the old logic would cancel both the immediately scheduled one and the other one that was scheduled with interval.
There was a problem hiding this comment.
I don't think it worked for you that way. Perhaps, you're confused.
Even now, immediate notifications have their own unique ID.
See:
https://github.com/NativeScript/plugins/blob/main/packages/local-notifications/index.android.ts#L241
https://github.com/NativeScript/plugins/blob/main/packages/local-notifications/index.ios.ts#L151
3 participants
This PR is an attempt to fix #595 along with some cleanup.