Flutter_local_notifications: [Feature request] Improve API for setting intervals

[erf]

I previously used showDailyAtTime to set a daily reminder - that was a pretty nice API.

I saw it was deprecated, and now to add intervals was pretty rough..

• First you have to get a time zone, by initializing a database and calling either a platform call or getting a third party package.
• Then you have to be sure the TZDateTime, given the timezone location, has a date in the future ( or it will crash ), even though i'm only interested in the hour and seconds of the day.
• Then you have to pick the DateTimeComponents which only have two items.

This was a lot of work to get up and running. I think this could be improved upon.

[MaikuB]

You can still do daily with the new approach. Yes it's more work but the old approach had issues with daylight savings and when users changed locations that they end up in a different timezone. Even though that might not be concern for your app, as this plugin needs to work correctly in the general case this is why the new approach has been put in.

If you'd like to stick with the old approach then you can stick with old versions of the plugin, maintain a fork or look at submitting a PR as I'm not going to keep the old approach given the issues it had

[erf]

I got the new version working. I think it makes sense to build on that, but perhaps make the API a bit nicer.

I would suggest to automate / hide the time zone stuff for the user, as i think most users would prefer to use the local time, and rather make the location optional.

I also think avoiding crashing if the time is older, if you use DateTimeComponents.time as you only want remind at a specific time a day, so if the last year was used it should not matter.

Also DateTimeComponents could perhaps be extended, perhaps motivated by Swifts DateComponents.

I might look at making a PR if i find time and if you are OK with these suggestions.

BTW, thanks for a great library.

[MaikuB]

Whilst trying to follow the date components on iOS is a nice idea, it'd take a lot of work to do because Android doesn't have any support for that and has required custom code to implement what is there right now. As such, if you'd like to see changes in then this will require contribution from the community such as yourself as this is all done in my spare time. I'm happy to provide input but I don't have the time to be looking at implementing this

[erf]

Ok, maybe drop the DateComponents suggestion for now, but my first two points, that is: "hide the time zone" and "allow older dates", could be fixed without too much work i think.

[MaikuB]

Even then you're going to need to look at PR for that. The concept of hiding timezones can lead to issues. For example, what's expected when the user goes to a different timezone. Local time takes on a different meaning when that happens. I'd prefer avoiding making assumptions and give the user control on what should happen. That is another reason why the location needs to be specified right now.

[erf]

I might look into making a PR for this, but i rather to discuss and clear up these details with you first.

You could perhaps make an API that says setIntervalForLocalTimezone or something like that.

Also now to get the timezone is a bit difficult, you mention either a platform call or a third party library, maybe this could be included in the library using a API call getCurrentLocation, as that is what i think most people would want in most cases.

Still, the old date crash, for DateTimeComponents.time should be fixed or?

[MaikuB]

Forgot to add I don't know what you mean by crashing with an older time. Off memory, an assertion occurs as opposed to a crash because trying to schedule a notification that occurred in the past is likely due to a mistake in the code. Even with your proposal to remove it, you haven't mentioned what should happen in the case that it is still allowed to run

[erf]

If i set scheduledDate in zonedSchedule to a older year it crashes with:

ArgumentError (Invalid argument (scheduledDate): Must be a date in the future: Instance of 'TZDateTime')

But the year should not matter, since i only care about daily reminders given an hour, seconds and the DateTimeComponents.time.

[MaikuB]

Ah right, so it's for when the components have been specified then yes that could be fixed/changed but also to keep the error for one-off notifications

[erf]

Yes. You could do a:

if (matchDateComponents == null) {
 validateDateIsInTheFuture()
}

[MaikuB]

If you could submit a PR on this that would be great though not sure if what you suggested there is sufficient. If you plan to look at a PR for other stuff then this could be a good way to familiarise yourself with the code along the way and see if there's other improvements that could be made

[erf]

Ok. Can take a look at it. But now local time is really late ;)

[erf]

I created a PR now for allowing past dates when using DateComponents along with a few other fixes.

I noticed that always_specify_types is enabled in analysis_options.yaml, which makes for long lines in some cases, could we omit this?

[MaikuB]

Closing this as the PR has been merged in

[erf]

Thanks for merging this. Do you plan on releasing a new version soon or should i just use the github version?

[MaikuB]

As the PR had a breaking change that requires a major version bump, I'm looking to release it with other changes (especially #983) but have run into issues with getting the iOS side running on my laptop so probably best if you use GitHub version for now

[erf]

What problems do you have with iOS? Something i can help with?

[MaikuB]

The iOS example app was no longer running on my MacBook. Isn't related to that PR so not sure what's going on. It's ok though, I've been able to get it running again by recreating the iOS example app. This sort of good timing as this allows me to recreate it in Swift, which is the default for Flutter iOS apps now.

[erf]

Sounds good!

[MaikuB]

Published a prerelease that you can use instead of referencing via Git https://pub.dev/packages/flutter_local_notifications/versions/4.0.0-dev.1

Includes some other changes, which is why I wanted to have it as a prerelease. Will give it about a week or so before publish it as a "stable" release

[erf]

Great! Thanks :)