Set timezone for DayView after initialization

RareScrap commented 3 years ago

New Issue Checklist

[x] Use the latest version of CalendarKit
[x] I've read the Contribution Guidelines
[x] I've searched for existing GitHub issues

Issue Description

I want to set a specific timezone for DayView so as not to depend on the device timezone.

Code I'm using with CalendarKit

I tried to set the timezone as follows, but it didn't work - the DayView's timezone still depends on the device's timezone

dayView.calendar.timeZone = TimeZone(secondsFromGMT: 0)!

Result I am trying to achieve

I want the DayViews to always be displayed at the same position on the timeline, regardless of the device's timezone

richardtop commented 3 years ago

Hi, you'll need to instantiate DayView with the modified Calendar in order to make it run in a different TimeZone:

  override func loadView() {
    calendar.timeZone = TimeZone(identifier: "Europe/Paris")!

    dayView = DayView(calendar: calendar)
    view = dayView
  }

Please, refer to the example project: https://github.com/richardtop/CalendarKit/blob/7746388c9ac27730c5d0f8bdd83b0f2d94a73cdd/CalendarKitDemo/CalendarApp/CustomCalendarExampleController.swift#L56

RareScrap commented 3 years ago

Thx for quick answer

instantiate DayView with the modified Calendar

I use storyboards with custom viewcontroller (not DayViewController) and I don't want to create a DayView manually. Can I change the calendar of an existing DayView?

richardtop commented 3 years ago

Currently it's not possible, as the proper Calendar has to be configured on view initialization and propagated to other modules of the CK.

I highly suggest you subclassing DayViewController and then incorporating that subclassed controller into your application, e.g. by using View Controller Containment.

Another approach would be to configure DayView programmatically. Storyboards are not 100% supported with the current implementation and might not work properly.

To sum up, in order to use CK with a custom timezone, you'll have to pass the calendar to it on initialization.

RareScrap commented 3 years ago

How difficult is it to implement a function that will simply reset the state of the DayView and configure it again after setting the timezone?

richardtop commented 3 years ago

That shouldn't be too difficult, however, it might lead to bugs and inconsistencies, as CK hasn't been designed with Storyboaard/XIB support in mind.

In short, this could be achieved by:

Adding a property observer here: https://github.com/richardtop/CalendarKit/blob/7746388c9ac27730c5d0f8bdd83b0f2d94a73cdd/Source/DayView.swift#L63
Invoking a code similar to this: https://github.com/richardtop/CalendarKit/blob/7746388c9ac27730c5d0f8bdd83b0f2d94a73cdd/Source/DayView.swift#L94 but keeping in mind the current date. The idea is to "silently" change the DayViewState in all of the CalendarKit modules.

It might also require modifying other CalendarKit modules (e.g. Header) to support this state/calendar swap after initialization. The whole library has been developed with assumption of Calendar instance not changing after initialization.

Feel free to submit a pull request with this change.

RareScrap commented 3 years ago

I started working on the ability to change the calendar after DayView initializing. I will be glad if you can help me and review my code. You can see my solution here #294.

richardtop commented 3 years ago

Thanks for the initial pull request. The direction looks good to me, I've left some comments. I'm looking forward to a finalized version.

I see two approaches to continue:

Always read the calendar from the DayViewState, so that it would be the only source of truth. The whole state could be "swapped".
Store the Calendar independently (the way it's done now) in every view and trigger an update for that view when it's changed. Both have pros & cons, which approach are you planning to go with?

RareScrap commented 3 years ago

I plan to use the second approach as this will not result in modules being dependent on the state object. I think this will lead to better independence of the modules from each other.

But could you please describe in detail the pros and cons of each of these approaches. I do not know all the details of the CalendarKit source code and I am wondering how my decision can affect the overall architecture

richardtop commented 3 years ago

Sounds good. The DayViewState object has been added to coordinate date change events between multiple modules (shown on the screenshot): Without the DayViewState each module had to figure out where the date change event came from and not notify that particular event source. Moreover, each module had to differentiate between programmatic events (e.g. when other module changes their date) and user-driven events, where the module changes date because of the user interaction. Keeping track of that turned out to be error-prone and often caused feedback loops so that the calendar could skip a date or animate incorrectly.

To sum up, the purpose of the DayViewState is to simplify date change management. DayViewState collects all of the user and programmatic inputs, notifies all of the relevant components and gives the context (e.g. current & previous dates), so that they could figure out all of the required animations. For reference, it's an Observer pattern from the "Design Patterns" book.

Rule: any event sent via DayViewState shouldn't trigger another day view state update event.

As for the pros & cons to the two approaches:

Storing Calendar in the DayViewState:

➕One source of truth as to which calendar is used ➕ Ease of debugging: it would be possible to track at which point Calendar is set and whether it's propagated correctly ➖ Adding more complexity into a single-purpose class ➖ Not all modules which need to use Calendar are connected to the DayViewState, this would lead to a tighter coupling

Propagating Calendar changes down via view hierarchy:

➕ Parent views already have access to the child views - adding this functionality is simple ➕ Simple to use just a single module (e.g. header) with a custom calendar, when working with CalendarKit on a component-basis ➖ A bit more code to write, as all of the needed views should receive the changed Calendar

To me the 2nd approach also seems natural, as there is no feedback-loop problem which was the reason to move to the DayViewState approach.

RareScrap commented 3 years ago

Thanks for the answer. However, I ran into an additional problem. Why is the project using the dateOnly() method? I mean, why would we want to cut off hours, minutes and seconds from a date object? Due to the fact that this is happening, when setting a new time zone, you have to restore the original date - add to the date that passed through the dateOnly() method, the number of seconds from the gmt of the previous timezone, and then pass it through the dateOnly() method again. For example:

let originDate = vc.timeline.date.addingTimeInterval(TimeInterval(vc.timeline.calendar.timeZone.secondsFromGMT()))
vc.timeline.calendar = newCalendar
vc.timeline.date = originDate.dateOnly(calendar: newCalendar)

Why bother cutting off hours, minutes and seconds from a date object at all? Now I have to use a lot of boilerplate to fix this.

richardtop commented 3 years ago

The dateOnly method was to strip the date of all the time information, so what's passed is just date and time set sharp to 00:00 of a timezone the CalendarKit is running in.

The reason for this addition is mainly to simplify debugging, as the time information has no meaning in the context of controlling the navigation in the CalendarKit.

To put it simply, if one module tells another module to move to a date which is not fixed at 0:00, there is an issue.

Of course, there is at least one opportunity to use the time information, for example, to keep the scroll position of the Calendar: https://github.com/richardtop/CalendarKit/issues/211

What is the problem and why does the use of dateOnly method prevents you from implementing this feature?

RareScrap commented 3 years ago

It took me 24 days, a bunch of burned out nerve cells, to realize that cutting off time really makes debugging much easier. I was constantly having problems updating dates to match the new time zones. Now I understand that it would be much more difficult for me to debug without cutting off the time. All this time, I have been fixing various bugs, which just appeared due to the incorrect "zeroing" of dates to a new timezone.

Please take a look at my improved solution and share your thoughts.

richardtop commented 3 years ago

@RareScrap I've reviewed the changes, please take a look. Let's continue the discussion only in your pull request, as it's pretty close to the completion.

richardtop / CalendarKit