Don't erase event timezones

[mcclurgm]

Fixes #650
Or at least part of it. We no longer set the timezones to floating, but this doesn't (yet) preserve the original timezone either, instead changing everything to the system timezone. It also doesn't add any affordance in the UI for time zones.
This now preserves the original timezone and displays timezone information in the dialog UI.

The issue was that we tried to set the timezone before we told libical that the time isn't DATE type. Libical just doesn't change the timezone in that case. So this reorganizes that code to fix it, and also adds tests to make sure that doesn't happen again! While I'm here, I also added support for floating timezones (outside of the implicitly floating DATE types) by changing the timezone argument behavior.

Still to do:

• Preserve the original timezone where possible
• Figure out how to use tests with a TimeManager 😠️

[mcclurgm]

Alright, I think this is ready to go. I messed with the TimeManager a bit to separate it from DBus in a testing mode, which you may or may not want. I'd also appreciate some help testing that the new logic to preserve the original timezone works as intended, since there seem to be some pitfalls there. (You can check and modify timezones in Evolution.)

[mcclurgm]

Ok, there's one thing that I want to discuss here. I just went ahead and implemented converting the timezone back to the original, but thinking back on it I'm not convinced that was a great idea. My hope was to make our lack of timezone support invisible: it displays in the system timezone at all times in Calendar, but if you go to another app that does support timezones, the original timezone info would be intact. Ideally, this would all work together: there could be no issues with this conversion, and it would be truly transparent. However, it's possible (I don't understand time calculations well enough) that this could cause some discrepancies. There's also the question of whether we want to do these implicit conversions at all, or if we should save the time into the event in exactly the way it's presented to the user. Does anyone have thoughts on this? If we wanted to change the saved timezone to the system timezone, that would be a very quick change to make.

[mcclurgm]

Since I guess I can do this now, can I get some input from @elementary/ux please? Before code review I need to know whether we want to implicitly convert timezones back and forth or not. This currently converts the original timezone to local, displays it for editing, and converts the result back to the original timezone. I can think of two options here: we could either convert to local and then not convert back (reset the event's timezone to the system's timezone on save), or we could present everything in the event's timezone in the dialog. Thoughts?

[mcclurgm]

Sorry to spam the comments in my own PR, but I also realized another potential issue with the current method: we'd also have to deal with the recurrence rules, which can have time components. We only display the date, but that's a potential issue I hadn't considered.

Overall, it will be more work (including design) to get everything to work in the original timezone instead of local, but could prevent some odd edge cases. There are examples of other apps that hide time zones though, so it is possible. Mac Calendar displays everything in local time by default and has a setting to turn on explicit time zones, for example.

[mcclurgm]

Ok, I think this is ready to go. Sorry it's taken me so long to get it in this state.

I would ideally like to test the editor behavior to make sure things are working properly, but that's super complicated since it depends on multiple EDS objects that I don't know how I would mock. This could probably use some testing, to make sure it saves data properly.

I would appreciate input on two things in particular:

• As discussed on Slack, I convert events with multiple time zones (different for start and end) to the same time zone. This change is permanent (and destructive of the multiple-time zone data) when you save the event. Does this sound ok? Also, I made the arbitrary decision to convert the end time to the same time zone as the start time, instead of the other way around.
• The timezone display could use some design work. The label is just what I threw together as a proof of concept, but I think it could be a lot nicer in the final version. (@elementary/ux)

[marbetschar]

This is pretty dope - thanks for digging into the Time Zone Limbo!! Also, it's really great to see you backed all of this with tests!

Unfortunately I'm not able to verify this from a functional perspective, since I don't have to deal with multiple timezones. Therefore I just looked at the code and left some nitpicking comments :)

[mcclurgm]

Thanks for the suggestions, especially about the tests. It definitely feels a little strange, but it's the best I could come up with.

As for testing multiple timezones: if you want, you can create events in Evolution with any time zone you choose. They'll transfer over to elementary, so then you can make sure they work. It may not be the same as working with a full calendar, but it is something 🙂

[tintou]

Thanks for dealing with this, it looks good to me 🚀

[marbetschar]

@mcclurgm did the functional testing and it works as expected. One small thing I came across is when adding new events: There the timezone information is not displayed at my end - it only appears after saving the event and editing it. Can we either display the system timezone when creating a new event - or remove the whole timezone information completely there since its not editable anyways?

The following screenshot shows how creating a new event looks like on my end:

[marbetschar]

@mcclurgm another thing which I came across is more from a design point of view: Since the timezone information is not editable anyways, any chance we can move it below the "All day" switch? Something like this:

[mcclurgm]

Thanks all. Marco, I'll have to dig in to creating new events. That's a big oversight on my part, I've only tested with pre-existing events. I'll get to work on that.

As for moving the time zone, that sounds like a nice idea, but I'm not sure how well it would work in practice. Not all time zones are that short, especially in Windows format. (We can't use those now, but I have a local branch that's working on it.) In the future I'd also like to make a selector, which I don't know it would look squeezed in over there.
Perhaps since we now have 2 controls that affect both times, we could move both the all day switch and the time zone to a new row?
I'll keep experimenting.

[mcclurgm]

I just pushed a commit to fix the empty timezone issue. Turns out that to make that work nicely, I also made the code a little cleaner and set the stage for if we want to add a time zone selector, so that's nice.

[marbetschar]

Awesome, thank you! 🚀