Skip to content
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

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

Closed
javiercbk opened this issue May 10, 2018 · 6 comments
Closed
Labels
duplicate This issue or pull request already exists

Comments

@javiercbk
Copy link
Contributor

I want to schedule a notification everyday at 10 AM. But currently I depend on the time that the notification is being setted up.

What I'm thinking is the following

final now = new DateTime();
// the first notification should be on 10:00 AM, so we set that date and set a daily interval
final firstNotification = new DateTime(now.year, now.month, now.day, 10, 0);
await flutterLocalNotificationsPlugin.periodicallyShow(
        ALARM_NOTIFICATION_ID,
        ALARM_TITLE,
        ALARM_BODY,
        RepeatInterval.Daily,
        platformChannelSpecifics,
        firstNotification); // the date starting date of the notification

What do you think?

Thanks

@MaikuB MaikuB added the duplicate This issue or pull request already exists label May 10, 2018
@MaikuB
Copy link
Owner

MaikuB commented May 10, 2018

I believe what you're asking for is duplicate of what was discussed in #25. I wasn't able to find a nice abstraction and way to implement it on all platforms (note: i have to consider the legacy iOS notification APIs as well).

However, with regards to what you've suggested, I don't think it will work. I believe it's rather misleading as what you're after is for the notification to fire at a specific time but you've actually passed through a DateTime instance. IMO, it'd be more accurate to create a custom time class but then the issue would be how to implement on all platforms. The implementation for iOS side is bit more tricky, particularly for the newer user notifications framework. Currently the periodicallyShow method uses this trigger where you can see it's not possible to specify a time

Doing it based on date time requires specifying to the framework the date/time components it should match on via this trigger. Doing a time based one on iOS means it'll always fire at the specified time and as far as i know, there seems to be no way to combine the time and interval with the newer framework. The older UILocalNotification framework allowed for doing this.

I think the best that can done in terms of providing an abstraction for all platforms is have another method (i guess called something like showDailyAtTime(...)) that fires a notification daily at a specified time. Meaning there's no interval can be specified. Unfortunately, it's not something I can explore at the moment so I can focus on building my app(s), but you're welcome to submit a PR for me to review :)

@javiercbk
Copy link
Contributor Author

First of all, thanks for your response.

In regards to your comment

I think the best that can done in terms of providing an abstraction for all platforms is have another method (i guess called something like showDailyAtTime(...)) that fires a notification daily at a specified time. Meaning there's no interval can be specified. Unfortunately, it's not something I can explore at the moment so I can focus on building my app(s), but you're welcome to submit a PR for me to review :)

What signature do you imagine for such method? I imagine something like this.

class RepeatTime {
  final int hours;
  final int minutes;
  RepeatTime(this.hours, this.minutes);
}

await flutterLocalNotificationsPlugin. showDailyAtTime(
        ALARM_NOTIFICATION_ID,
        ALARM_TITLE,
        ALARM_BODY,
        new RepeatTime(10, 0), // I could use two integers here
        platformChannelSpecifics);

If we can agree on this method interface I will contribute on the Java implementation. Objective C creeps me out a little bit, I'll try to do my best but I can't promise a nice and expressive Objective C implementation.

@MaikuB
Copy link
Owner

MaikuB commented May 10, 2018

That looks about right though I'd just rename the RepeatTime class to Time. Whilst I doubt anyone would really use the specify the seconds, it may be worth adding in that component for the sake of completion. Re: objective C, I get where you're coming as this project was first time actually writing in objective C :)

@javiercbk
Copy link
Contributor Author

Ok, I'll create the java version tonight, and see how much can I do in the Objective-C world.

Probably I'll create a PR to discuss how I'm advancing.

@javiercbk
Copy link
Contributor Author

I've just created the PR for android. I'll try to make it work for IOS

@MaikuB
Copy link
Owner

MaikuB commented May 15, 2018

Closing as this is addressed in PR #33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants