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
270 stars 271 forks source link

Change of actual RecurrenceRule Class #298

Open GoldenSoju opened 3 years ago

GoldenSoju commented 3 years ago

First of all thanks to everyone for your efforts in 'reviving' the package and the official null-safety version.

I was wondering if someone else would see a benefit in changing the actual RecurrenceRule class to the one from RRule package (Link) . The benefit I see, is a better compatibility with external data as the Recurrence Rule can easily converted to an iCalendar/RFC 5545 compliant String (and vice versa). Also some other utility functions like getting recurrences of a recurring event or human-readable text conversion are included.

I am hoping to read your opinions. :-)

andzejsw commented 3 years ago

In my opinion this is great idea. I also see that @thomassth agrees. Would like to see what @nickrandolph thinks. I just think that these changes should be made after PR https://github.com/builttoroam/device_calendar/pull/297 is merged.

nickrandolph commented 3 years ago

Yeh I don't see any issues with adopting that package.

GoldenSoju commented 3 years ago

Sorry for my late reply, I didn't find earlier time to work on this.

So I tried to implement the RRule package and it worked very well for Android (and Flutter). (I commited to my fork).

I wanted to start implementing on iOS today, just to realize that iOS uses the EKRecurrenceRule (EventKit) class and it has no easy way to receive an RFC 5545 String. So while on Android I was able to receive the RFC 5545 String and everything worked just fine, on iOS the EKRecurrenceRule would have to be converted from/to RFC.

Not sure how to proceed from here. Looking at some libraries the conversion EKRecurrenceRule <-> RFC 5545 isn't done in some lines, so that could take me some time implement.

Maybe someone has a better idea?

GoldenSoju commented 3 years ago

So I applied the change also to the iOS part. (I commited to my fork).

For iOS I made use of an exisiting library. Please note that I am not used to Swift/iOS and didn't manage to import the files correctly. So I copied all relevant code to the top of the ios/Classes/SwiftDeviceCalendarPlugin.swift file. Which is super ugly, so it would be great if someone with the ability and time could import the files of the ios/Classes/EkRruleConverter/* folder into SwiftDeviceCalendarPlugin.swift in a cleaner/more appropriate way, that would be great. Besides this I had to do changes on the example project to make it run.

So far I got the project in my fork running and it seems to work just fine. I hope to find some time next week to add some tests.

Any feedback will be much appreciated.

thomassth commented 2 years ago

@GoldenSoju any luck progressing?

Or you can submit your PR as a draft, so we can take a closer look before merging.

GoldenSoju commented 2 years ago

@thomassth yes I made good progress the last few weeks. I think I can share the result next weekend (or in 2 weeks).

GoldenSoju commented 2 years ago

@thomassth I think I have now a version that I can share. My fork and tree for device_calendar

I tested with the sample app and it works fine on Android and iOS. I cannot promise that I found all 'bugs' though.

I also had to do changes directly on the RRule package: My fork for rrule (I added my modified version to the device_calendar's pubspec.yaml. So this link is just for reference.)

thomassth commented 2 years ago

That’s good to hear!

if you feel ready, submit the fork as a PR here so we can have a better look.

Although I’m not a fan of keeping modified packages within another package, so we will probably wait after a PR over at rrules is merged before we merge this. (I’ve also submitted a PR over there for other issues.)

GoldenSoju commented 2 years ago

@thomassth Done :) please refer this pull request.

GoldenSoju commented 2 years ago

@thomassth The develop branch of builttoroam/device_calendar seems almost complete. Are there any more things you want to change or does only the version and readme update remain? Anything I could help with?

thomassth commented 2 years ago

Sorry misread the labels

Right now not much can be done actually, unless someone other than me can initiate a PR from develop to release branch.

If I open a PR for that, I'll need other maintainers to approve it, which is very unlikely so far.

thomassth commented 2 years ago

Feel free to try if you want to:

https://github.com/builttoroam/device_calendar/compare/release...develop?expand=1

GoldenSoju commented 2 years ago

@thomassth

Done. I will also start testing this branch internally for my production app.