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

Fixed All day events start time may be incorrect (iOS only) #337 #450

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

OleksandraFedotova
Copy link

Fixed fullDay with DateRange events creation (not only one fullday events possible to create) Timezone should be set only if event is not full day

Fixed fullDay with DateRange events creation (not only one fullday events possible to create)
Timezone should be set only if event is not full day
@IVLIVS-III
Copy link
Contributor

Could you please describe what exactly this PR fixes.
What went wrong / didn't work as expected?

It looks like this PR also introduces all-day-multi-day events. Is that correct?

@IVLIVS-III
Copy link
Contributor

IVLIVS-III commented Oct 17, 2022

I just tested your changes on iOS.
Works great, all-day-multi-day events can be read from the calendar without issues.

Could you please also change the example app, so while adding a new event, when toggling all-day to on, the input field of event end no longer disappears.
With your PR hiding this field is no longer necessary on iOS.
It would be good, if we could also test creating multi-day all-day events from within the example app.

@IVLIVS-III IVLIVS-III self-requested a review October 17, 2022 17:56
@OleksandraFedotova
Copy link
Author

My PR fixed the problem with creating all-day-multi-day events in IOS. Yep I can fix test app as well)

@IVLIVS-III
Copy link
Contributor

Great, that's what I thought. Is a very welcome change.

Perfect if you can change the example app as well, thank you.

IVLIVS-III added a commit to IVLIVS-III/device_calendar that referenced this pull request Nov 2, 2022
Now that we have multi-day all-day events on both platforms (see builttoroam#450), handling the platforms differently is no longer necessary.
@IVLIVS-III
Copy link
Contributor

Since I don't want this PR to be blocked any longer, waiting only for changes in the example app, I split it up into a separate PR (see #454).

Thanks @OleksandraFedotova for your contribution.

Copy link
Contributor

@IVLIVS-III IVLIVS-III left a comment

Choose a reason for hiding this comment

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

LGTM

@IVLIVS-III IVLIVS-III merged commit 039f047 into builttoroam:develop Nov 2, 2022
@OleksandraFedotova
Copy link
Author

@IVLIVS-III Sorry for not providing changes to the example app, will do my best next time while contributing ;)

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.

2 participants