Implemented daily notification with custom time #33

javiercbk · 2018-05-11T03:17:13Z

I've implemented it for android, I'll try to make it work for IOS as well

Caveat: The notification time is hardcoded in the example app

MaikuB

Looks good! I've left some comments on what I'd say should be changed. Also need to update the rescheduleNotifications() method in FlutterLocalNotificationsPlugin.java. That method is called when a device has been rebooted. Anything scheduled via the alarm manager is wiped out when the device has been turned off.

Once you've done the iOS side. I'll take a look and see if I can test out your fork before I'll merge it in. Also I've no problem with hardcoding the time in the example app as it's just for demonstration purposes so you'd need to do that anyway :)

MaikuB · 2018-05-11T03:43:10Z

lib/flutter_local_notifications.dart

+    {String payload}) async {
+    Map<String, dynamic> serializedPlatformSpecifics =
+        _retrievePlatformSpecificNotificationDetails(notificationDetails);
+    await _channel.invokeMethod('periodicallyShow', <String, dynamic>{


invokeMethod should pass in 'showDailyAtTime' for better separation. This means that the onMethodCall override on the native side will need a different case to handle what needs to be done. If you could also split up repeatNotification to two separate methods (one for interval-based, the other for time-based) then that would be good as well

I have chosen this approach because I considered this feature as a Daily periodicallyShow use case, that way changes in both Java and Objective-C remain minimal. I can add that change though.

Valid point, however from the Dart side they're separate methods so from a maintainability perspective, I think it's better to separate it out as it makes mapping between the Java/Objective C and Dart more obvious

MaikuB · 2018-05-11T03:46:43Z

lib/flutter_local_notifications.dart

+
+  Time([this.hour = 0, this.minute = 0, this.second = 0]);
+
+  Map<String, int> get serialized => {


if you could make this a toMap() method to be consistent with the rest of the code then that would be great as reading something like time.serialized feels awkward

MaikuB · 2018-05-11T03:48:21Z

lib/flutter_local_notifications.dart

+  final int minute;
+  final int second;
+
+  Time([this.hour = 0, this.minute = 0, this.second = 0]);


there should be some validation here to restrict the accepted values, possibly through assert statements. Should add documentation as well through comments but you can leave that for me to do

fixed bad time constructor Fixed param name, suppresed warning Better notification body

javiercbk · 2018-05-11T07:15:18Z

Added support to IOS as well

MaikuB · 2018-05-11T07:19:57Z

Yep I took a look and it looks about right but will need some of the changes I commented about earlier regarding the string that's passed from Dart side via the invokeMethod call. I think once all the changes are made, we're good to merge it. I'll then do some clean up and testing before publishing a new version.

Thanks for your contribution btw. I've just realised that I've been using your JSON to Dart tool. Super handy and I've asked on the Flutter repo about putting a link to your site but go no response :S

Added support for IOS

javiercbk · 2018-05-11T07:35:03Z

I've made the requested changes to the Dart's Time class. I'll make the other changes tomorrow.

Glad that my Dart to JSON tool is helping others

javiercbk · 2018-05-14T15:45:17Z

I've added a weekday bitwise flag option. I haven't test it though but I will tonight

MaikuB · 2018-05-14T22:30:42Z

By that are you referring to the ability to just have notifications displayed on weekdays only? If so then I don't think that works for the user notifications framework on iOS with the date components as you'd have to create a separate notification for each day, which means each one has their own ID. On that note, i think it'll be possible to schedule a notification to fire on a specified a day of the week and time but that's a separate discussion :)

javiercbk · 2018-05-14T22:37:05Z

In android it was easy, you just schedule a notification every day and when the intent is fired you just choose not to fire the notification. In IOS things are a bit mor complicated, you have to schedule one notification for each day.

My first solution is to provide the first notification ID and then increment the number internally. But that solution is really ugly because the plugin user would have to calculate the new notifications id. This is just a proof of concept. If it works I would generate the notification id with some uuid or the unix timestamp and return an array of ids. That of course would break the default API usage because there would be a single function that does generate the ids for you but the others won't.

It would be up to you to accept that change or not and I would understand if you don't want that change in your project. In that case I would create a plugin of my own and publish it because I need weekdays notifications for my project :)

MaikuB · 2018-05-14T22:43:57Z

Yeah I had a feeling Android would be easier and that it would be more harder on iOS. I would say leave that for the moment and if you could make the other changes then we can merge this PR in. Looking at how to do it for weekdays can be done in a separate PR or something I can look into even

javiercbk · 2018-05-14T22:45:00Z

Ok, let me wrap up what I got and finish this PR tonight (It's 19:45 Argentina so I guess that I would finish this in a couple of hours)

MaikuB · 2018-05-14T23:00:05Z

Cheers mate. I should have some bandwidth to look into the weekday one and touched on the rough idea i've got for it :)

javiercbk · 2018-05-15T00:18:02Z

Done, I've created a custom name for the new channel method and reverted the changes on the weekdays. I've created a new branch on my fork with my work on weekdays notification. I'll leave that branch for you to check the code.

Since I need to send notifications on weekdays for my app, I'have to create my own plugin to do that. I prefer swift over Objective C but probably I will reusing some parts of this project and I'll mention you and your project as well in the library that I'm creating.

Let me know if you need anything. Cheers

MaikuB · 2018-05-15T08:27:24Z

Np, I should be able to merge your changes later. I do prefer Swift as well except I originally ran into issues with debugging in Swift. I heard there use to be issues with plugins written in Swift that were consumed by apps where the iOS head project is in Objective C. Not sure if that issue still exists as it's not something I've been monitoring

MaikuB · 2018-05-16T11:23:06Z

fyi, i've uploaded a new version that includes your changes with fixes I made. Also added ability to show a weekly notification on a specific day and time that you could also use to solve your problem for showing notifications on weekdays

javiercbk mentioned this pull request May 11, 2018

Support setting a repeating notification in a custom hour and minute #31

Closed

javiercbk force-pushed the showDailyAtTime branch from 523c1bb to 7f57d12 Compare May 11, 2018 03:21

MaikuB requested changes May 11, 2018

View reviewed changes

javiercbk added 2 commits May 11, 2018 04:13

Implemented daily notifications with time

59e5301

fixed bad time constructor Fixed param name, suppresed warning Better notification body

Added support for IOS

a62bd8f

Asserts and toMap changes to Time class

1b784e5

Added support for IOS

javiercbk force-pushed the showDailyAtTime branch from 46b551b to 1b784e5 Compare May 11, 2018 07:31

Added showDailyAtTime channel method name

2d691fd

MaikuB approved these changes May 15, 2018

View reviewed changes

MaikuB merged commit e353dfd into MaikuB:master May 15, 2018

Charlie9830 mentioned this pull request Jul 18, 2019

assembleDebug failed with exit code 1 #268

Closed

adarshbalu mentioned this pull request Jun 21, 2020

Problem while using schedule notification #673

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented daily notification with custom time #33

Implemented daily notification with custom time #33

javiercbk commented May 11, 2018 •

edited

Loading

MaikuB left a comment •

edited

Loading

MaikuB May 11, 2018

javiercbk May 11, 2018

MaikuB May 11, 2018

MaikuB May 11, 2018

MaikuB May 11, 2018

javiercbk commented May 11, 2018

MaikuB commented May 11, 2018

javiercbk commented May 11, 2018

javiercbk commented May 14, 2018

MaikuB commented May 14, 2018

javiercbk commented May 14, 2018 •

edited

Loading

MaikuB commented May 14, 2018 •

edited

Loading

javiercbk commented May 14, 2018 •

edited

Loading

MaikuB commented May 14, 2018

javiercbk commented May 15, 2018 •

edited

Loading

MaikuB commented May 15, 2018

MaikuB commented May 16, 2018


		Time([this.hour = 0, this.minute = 0, this.second = 0]);

		Map<String, int> get serialized => {

Implemented daily notification with custom time #33

Implemented daily notification with custom time #33

Conversation

javiercbk commented May 11, 2018 • edited Loading

MaikuB left a comment • edited Loading

Choose a reason for hiding this comment

MaikuB May 11, 2018

Choose a reason for hiding this comment

javiercbk May 11, 2018

Choose a reason for hiding this comment

MaikuB May 11, 2018

Choose a reason for hiding this comment

MaikuB May 11, 2018

Choose a reason for hiding this comment

MaikuB May 11, 2018

Choose a reason for hiding this comment

javiercbk commented May 11, 2018

MaikuB commented May 11, 2018

javiercbk commented May 11, 2018

javiercbk commented May 14, 2018

MaikuB commented May 14, 2018

javiercbk commented May 14, 2018 • edited Loading

MaikuB commented May 14, 2018 • edited Loading

javiercbk commented May 14, 2018 • edited Loading

MaikuB commented May 14, 2018

javiercbk commented May 15, 2018 • edited Loading

MaikuB commented May 15, 2018

MaikuB commented May 16, 2018

javiercbk commented May 11, 2018 •

edited

Loading

MaikuB left a comment •

edited

Loading

javiercbk commented May 14, 2018 •

edited

Loading

MaikuB commented May 14, 2018 •

edited

Loading

javiercbk commented May 14, 2018 •

edited

Loading

javiercbk commented May 15, 2018 •

edited

Loading