search

builttoroam / device_calendar

A cross platform plugin for modifying calendars on the user's device
https://pub.dev/packages/device_calendar
BSD 3-Clause "New" or "Revised" License
260 stars 263 forks source link

Implementing Rrule package #403

Closed GoldenSoju closed 1 year ago

GoldenSoju commented 2 years ago

New Pull Request for #400, related to #298

Currently working in example:

Hurdles (all should be solved before merge):

thomassth commented 2 years ago

Hmm, it doesn't fix git tracing as good as I hoped

I'm trying the old-fashioned way, cherrypicking each commits and resolving stuff.

GoldenSoju commented 2 years ago

Sorry all this merging and conflict solving had me a bit confused... I added two more commits as it seems that it wasn't pushed correctly yesterday.

thomassth commented 2 years ago

@GoldenSoju I can only get some of the features working right now. Is this intended?

GoldenSoju commented 2 years ago

@thomassth no, not at all intended. My working version branch works fine... Let me try to compare my working branch and this branch to see what went missing during the merge.

thomassth commented 2 years ago

I've added a checklist to the top.

Feel free to add lines and check items that are done.

(Do you think we need a separate checklist for android and iOS?)

thomassth commented 2 years ago

I think you have pulled all of the changes bar some dependency updates.

Maybe it's the example 's problem?

https://github.com/GoldenSoju/device_calendar/compare/GoldenSoju:develop_rrule...fast_merge_develop_rrule

GoldenSoju commented 2 years ago

Hm just some minor corrections were needed but now it should work fine. I also had to correct something in my version of the Rrule package, I guess that was the main bug for the monthly and yearly recurrences to fail. Deletion also seems to work fine.

(Do you think we need a separate checklist for android and iOS?)

No I don't think that will be necessary. After all, the iOS EKRecurrenceRule is actually just the iCalendar Rrule with a different name. You can also get a valid Rrule String if you convert the EKRecurrenceRule to AnyObject. (See SwiftDeviceCalendarPlugin.swift beginning with line 528). So I think iOS uses the iCalendar mechanics under the hood and no iOS specific 'magic'.

thomassth commented 2 years ago

Some bugs in current build:

Weekdays/weekends seems fixed

“first Monday of Jan, yearly” also includes all Mondays of current January, similar things on monthly (every month becomes every week)

“Every 2 years” not working

I think I’m going to write some tests.

GoldenSoju commented 2 years ago

Hm it works on my side (testing mainly with google calendar). Can you please try again after flutter clean and flutter pub get ? Maybe you don't have the newest version of the Rrule dependency.

Also in the example app, file calendar_events.dart you might want to change line 135 to something with a bigger end date. Sth like final endDate = DateTime.now().add(Duration(days: 365 * 4));. Atm it only catches events from -/+ 30 days.

thomassth commented 2 years ago

I'll try later today.

Either way your rrule package need to be merged to the main branch first.

If you think it's ready go submit a PR.

GoldenSoju commented 2 years ago

I just submitted the pull request for the rrule package.

GoldenSoju commented 2 years ago

@thomassth The Rrule package supports now JSON(iCAL format) and I applied the changes to the Flutter and Android part. I will start working on the iOS part today/tomorrow. If you want to check, I commited the newest changes to this branch

GoldenSoju commented 2 years ago

@thomassth iOS implementation is also finished. I tested daily/weekly/monthly and yearly recurrences and it seems all to work and the resulting recurrence rule strings also are all correct as far as I've seen. Therefore I think Android/iOS parts are fine, while minor errors might occur due to the example app (add event page).

GoldenSoju commented 2 years ago

@thomassth Ok I also merged all outstanding commits from the main branch into this one. So it is up to date. I got everything working so far, except All Day events in the example app. Haven't figured out yet how to solve the problem though.

GoldenSoju commented 2 years ago

@thomassth Hi! I implemented the device_calendar package with rrule package in one of my projects and found some shortcomings in the previous pull requests.

The newest version fixes those problems and the example is also now working correctly (there are no specified tests yet, but so far manual testing works fine on Android and iOS).

The only functionality still missing at the moment would be multi-all-day-events (with or without recurrence) on iOS, but I saw there was another pull request/discussion for that problem.

Please check the newest version if you have time.

thomassth commented 1 year ago

After rebase, I can confirm all these are working with rrule on android. But they need to be tested on iOS: (use the delete button within the details page)

Also needs to check if it overwrites other PRs, since this is quite beefy Especially #410

GoldenSoju commented 1 year ago

@thomassth

I checked on iOS, below all points work without problems.

Also all tests were passed on iOS.

Also needs to check if it overwrites other PRs, since this is quite beefy Especially https://github.com/builttoroam/device_calendar/pull/410

Seems like your latest commits reflect all changes made in develop branch of builttoroam/device_calendar. All seems to be up-to-date according to comparison.

thomassth commented 1 year ago

Great!

One last thing, @GoldenSoju are you participating in Hacktoberfest? I can add the tag in

GoldenSoju commented 1 year ago

@thomassth

Great!

One last thing, @GoldenSoju are you participating in Hacktoberfest? I can add the tag in

Thanks for asking, that would be nice! :)