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

change ios part - migration to Objective-c #213

Draft
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

andreychernikovich
Copy link

No description provided.

Copy link
Contributor

@bhl09 bhl09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found numerous bugs on this, please see comments

}
[ekEvent setTitle:title];
[ekEvent setNotes:description];
[ekEvent setAllDay:isAllDay];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When adding an event, isAllDay is always true even when it is sent as false in dart

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

}
[ekEvent setCalendar:ekCalendar];
[ekEvent setLocation:location];
[ekEvent setRecurrenceRules: [self createEKRecurrenceRules: arguments]];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either I don't set a recurrence or set a recurrence, it always return as daily frequency, 0 interval, ending in 01/01/1970

Screen Shot 2020-04-01 at 2 51 40 pm

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

[ekEvent setCalendar:ekCalendar];
[ekEvent setLocation:location];
[ekEvent setRecurrenceRules: [self createEKRecurrenceRules: arguments]];
[self setAttendees:arguments event:ekEvent];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When adding an event with attendees, they are always lost, returning no attendees

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will correct the addition of Attendees later

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

[ekEvent setLocation:location];
[ekEvent setRecurrenceRules: [self createEKRecurrenceRules: arguments]];
[self setAttendees:arguments event:ekEvent];
[ekEvent setAlarms: [self createReminders: arguments]];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When adding an event with a reminder of 30 minutes, it breaks the example app to return no events, however, I can still see the event in the iOS calendar app with a bizarre reminder.
Screen Shot 2020-04-01 at 2 57 25 pm

I have to delete the event with the bizarre reminder to get the example app working again

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

return reminders;
}

-(void)createOrUpdateEvent: (FlutterMethodCall *)call result:(FlutterResult)result{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Url field missing for adding/editing an event. It was available in Swift:

let url = arguments[self.eventURLArgument] as? String

if let urlCheck = url, !urlCheck.isEmpty {
    let iosUrl = URL(string: url ?? "")
    ekEvent!.url = iosUrl
}
else {
    ekEvent!.url = nil
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

[localSources addObject:ekSource];
}
}
if ([localSources count] == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating a calendar, the code doesn't return any local calendars when I can in Swift - always returning the "Local calendar was not found" error

@nickrandolph
Copy link
Contributor

@andreychernikovich apologies for the delay in getting back to this PR - are you able to rebase off latest develop branch and fix any conflicts please.

@nickrandolph nickrandolph changed the title change ios part - migration to Objective-c [WIP] change ios part - migration to Objective-c Jun 5, 2020
@nickrandolph nickrandolph marked this pull request as draft June 5, 2020 23:10
@nickrandolph
Copy link
Contributor

Link to #202

@andreychernikovich andreychernikovich marked this pull request as ready for review June 8, 2020 12:31
@andreychernikovich
Copy link
Author

Hi @nickrandolph! I fixed all conflicts many days ago have you watched this PR?

@andreychernikovich andreychernikovich changed the title [WIP] change ios part - migration to Objective-c change ios part - migration to Objective-c Aug 5, 2020
@nickrandolph
Copy link
Contributor

@andreychernikovich apologies for the delay - I'll be trying to review this PR this week

@laurisvan
Copy link

Hi @nickrandolph!

How does the review look? Any chances to do it this week?

@andzejsw
Copy link
Contributor

andzejsw commented Apr 23, 2021

@nickrandolph have you looked deeper into this PR? I myself remade new fork with this PR changes. Fixed issue that in iOS example app permissions wasn't requested, but now I am stopped on EXC_BAD_ACCESS, which occurs on hasEventPermissions method. But my knowledge in objective-c is very limited.

@nickrandolph
Copy link
Contributor

@andzejsw I haven't had a chance to get back to this PR. My concern here it to make sure that we don't affect support for Swift. To test this, we need to make sure there are both Swift and Objective-C versions of the example app.

@nickrandolph
Copy link
Contributor

Also check out https://pub.dev/packages/device_calendar_extended and pull in any relevant pieces to get the Objective-C piece to work. It'd be good to be able to consolidate the packages so there's only one option that devs need to worry about going forward.

@thomassth
Copy link
Contributor

What is the significance of implementing obj-C? I thought both swift and obj-C are first class languages on iOS.

I read the issue referenced in Readme and maybe there's some workaround mentioned.

flutter/flutter#16049 (comment)

@nickrandolph nickrandolph marked this pull request as draft April 24, 2021 23:16
@nickrandolph
Copy link
Contributor

@thomassth tbh I'm not sure, as I'm not too familiar with the issue with using the library from Objective-C. I've marked this PR as draft to make sure it doesn't accidentally get merged. It'd be good to see what the device_calendar_extended code does and understand why their implementation works better for Objective-C devs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants