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

Implementing Rrule package #403

Merged
merged 23 commits into from
Oct 6, 2022

Conversation

GoldenSoju
Copy link
Contributor

@GoldenSoju GoldenSoju commented Jan 5, 2022

New Pull Request for #400, related to #298

Currently working in example:

  • can build daily or weekly recurring events.
  • can build monthly or yearly events.
  • can build complex (eg. multi days a week) events.
  • can delete all events in the chain.
  • can delete single events.
  • can delete this and following instances

Hurdles (all should be solved before merge):

  • rrule changes are published.
  • most features are working / non-working features hidden and labeled.
  • re-enable lower API support (full or partial), or document why they should be dropped.

@thomassth

This comment has been minimized.

@GoldenSoju
Copy link
Contributor Author

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 thomassth changed the title Fast merge develop rrule Implementiing Rrule package Jan 6, 2022
@thomassth thomassth changed the title Implementiing Rrule package Implementing Rrule package Jan 6, 2022
@thomassth
Copy link
Contributor

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

@thomassth thomassth marked this pull request as draft January 6, 2022 08:57
@GoldenSoju
Copy link
Contributor Author

GoldenSoju commented Jan 6, 2022

@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
Copy link
Contributor

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
Copy link
Contributor

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

Maybe it's the example 's problem?

GoldenSoju/device_calendar@GoldenSoju:develop_rrule...fast_merge_develop_rrule

@GoldenSoju
Copy link
Contributor Author

GoldenSoju commented Jan 6, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

I just submitted the pull request for the rrule package.

@GoldenSoju
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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).

… fast_merge_develop_rrule

� Conflicts:
�	android/src/main/kotlin/com/builttoroam/devicecalendar/CalendarDelegate.kt
�	android/src/main/kotlin/com/builttoroam/devicecalendar/DeviceCalendarPlugin.kt
�	example/lib/presentation/pages/calendar_event.dart
�	ios/Classes/SwiftDeviceCalendarPlugin.swift
�	lib/src/device_calendar.dart
GoldenSoju added 2 commits January 26, 2022 18:47
…ast_merge_develop_rrule

# Conflicts:
#	ios/Classes/SwiftDeviceCalendarPlugin.swift
@GoldenSoju
Copy link
Contributor Author

@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.

@thomassth thomassth added help wanted This issue is open for someone to pick up and action good first issue Good for newcomers enhancement New feature or request labels Mar 27, 2022
@thomassth thomassth marked this pull request as ready for review March 27, 2022 14:38
GoldenSoju added 3 commits April 14, 2022 15:10
Reworking example to work correctly with new recurrence rule;
Several bug fixes;
Fixing example to work correctly with recurring multi-(all-)day events;
adding addPostFrameCallback to even_item.dart to avoid state error;
@GoldenSoju
Copy link
Contributor Author

@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
Copy link
Contributor

thomassth commented Oct 1, 2022

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)

  • can build daily or weekly recurring events.
  • can build monthly or yearly events.
  • can build complex (eg. multi days a week) events.
  • can delete all events in the chain.
  • can delete single events.
  • can delete this and following instances

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

@GoldenSoju
Copy link
Contributor Author

GoldenSoju commented Oct 5, 2022

@thomassth

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

  • can build daily or weekly recurring events.
  • can build monthly or yearly events.
  • can build complex (eg. multi days a week) events.
  • can delete all events in the chain.
  • can delete single events.
  • can delete this and following instances

Also all tests were passed on iOS.

Also needs to check if it overwrites other PRs, since this is quite beefy
Especially #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
Copy link
Contributor

Great!

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

@GoldenSoju
Copy link
Contributor Author

@thomassth

Great!

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

Thanks for asking, that would be nice! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest-accepted reward for PRs help wanted This issue is open for someone to pick up and action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants