Convert event times to user timezone by default

Created on 22 October 2024, 6 months ago

Problem/Motivation

Currently the calendar display all events in the time in which they occur in their own timezone, if set. This could be confusing where calendars contain events in different timezones. For example, two events could start at the same actual time in different timezones, and the calendar will show them at different times.

Proposed resolution

Add a new setting to convert timezones in the display, and enable it by default. When enabled, skip passing in the event timezone when preparing event data for output, and then pass the setting to the Javascript, so it can be included in the JSON payload return on drag-and-drop.

Drag-and-drop processing should change the logic of when events are remapped based on this setting, particularly for events which are not all day.

Feature request
Status

Active

Version

3.0

Component

Code

Created by

🇨🇦Canada mandclu

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mandclu
  • Merge request !43Setting to convert timezones → (Merged) created by mandclu
  • 🇩🇪Germany jurgenhaas Gottmadingen

    I've reviewed the code, and it looks good to me. The only suggestions for the UpdateController is about the if ($new_data['allDay'] || ($empty_end && $timezone && !$this->convertTzs)) { statement that we have twice. Maybe that could be delegated into a needsTzRemap method? That would help for maintenance so that we never forget to update all instances, if we ever need to update or enhance that block.

    Do you think I should also test this on a Drupal site?

  • 🇨🇦Canada mandclu

    TBH I struggled with this question of the not-so-DRY repetition of checks myself. It seemed like a helper method would need to passed all those same values only to return a boolean based on a pretty simple combination, so I ultimately decided it wasn't worth the effort, and instead kept the relevant four lines of code identical, so updating both at once is trivial, at least.

    Theoretically, earlier in the code I could also combine $empty_end and !$this->convertTzs into a new boolean value, so at least the repeated code is a little cleaner.

    If you have the ability test this on a Drupal site, a fresh set of eyes would definitely be likely to catch edge cases I missed.

    The drag-and-drop changes I tested were:

    • Drag an in-day event to a different start time
    • Resize an in-day event to a different duration
    • Drag an in-day event to the all-day header to make it all day
    • Drag an all-day event into the day grid to make it in-day
    • Drag an all-day event between days
    • Resize an all-day event to make it span more (or fewer) days

    I also tried using events that were the same TZ as the user, events that were the site default (with the user TZ different), event in different timezones, etc.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Sure, that helper function is not a blocker. I only had a hard time to verify in my brain if those conditions with ANDs and ORs are really doing the same thing. We can certainly keep that as is.

    Now, I started testing, and the calendar shows a false result. Here is the setup:

    • The site's TZ is Berlin, UTC+2
    • The first event is at 4pm in Berlin's TZ
    • The second event is at 4pm in London's TZ, UTC+1

    The calendar shows the first event at 6pm and the second at 5pm. So, the second is correct but the first is wrong, it should be at 4pm.

    I need to go to a meeting now but thought I leave that simple test here. Maybe you have a chance to look into it.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Sorry for the noise, I fooled myself completely. I had created the first event with the default timezone, and only later realized that the default was UTC. I then changed the default TZ to Berlin and that shifted the first event back to the new time, which was entirely correct.

    So, I've tested this all again with proper settings and I can confirm that all the use cases are working correctly. Very nice work indeed. What an impressive solution that is.

  • Pipeline finished with Skipped
    6 months ago
    #319211
    • mandclu committed 352a7687 on 3.0.x
      Issue #3482465 by mandclu, jurgenhaas: Convert event times to user...
  • 🇨🇦Canada mandclu

    Awesome! Merged in.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024