FEATURE: Display calendar events adjusted for timezones #432
Conversation
|
@jancernik One issue I noticed with public holidays is that they can span two days. Ca you repro this? 
Her the grey colored event is Labor Day. User "testuser5" is in Los Angeles, user "pmusaraj" in Montreal (Canada). The holiday is only on the 4th, not sure why this is showing it as starting a day before. Curious if you can repro. Also, looks like PR needs rebasing. |
|
Hmm, thanks for letting me know. I'm working on this kind of issues right now. I had a similar thing happen when testing in Chrome and Firefox. Due to differences in the way they handle the default Date object, with some timezones, all the events get rendered one day earlier in Firefox. This is not related to my change. I found out it's always been this way. EDIT: Good news. This problem seems to have been related to my specific Firefox install. I tried using the Dev version and then reinstalled it, and everything seems to be working fine :) |
jancernik
force-pushed
the
event-calendar-timezone-compensation
branch
from
62e3f80
to
625fa20
Compare
For some time zones, events were being rendered one day earlier when using the default Date object
These options (Etc/GMT-04, Etc/GMT+5, etc.) are being parsed incorrectly. They are essentially being parsed as if the signs were flipped. I tried forcefully feeding the timezone with the opposite sign, which fixed the event rendering but not the calendar itself. I'm opting to remove them from the list to avoid confusion.
There was a problem hiding this comment.
Nice progress, just a few minor comments, but overall I like the recent changes.
I'd really like to merge and test this with real data shortly. Do you think you can make some final fine-tuning and merge this today or Monday @jancernik ?
assets/javascripts/discourse/initializers/discourse-calendar.js
Outdated
Show resolved
Hide resolved
assets/javascripts/discourse/initializers/discourse-calendar.js
Outdated
Show resolved
Hide resolved
assets/javascripts/discourse/initializers/discourse-calendar.js
Outdated
Show resolved
Hide resolved
I also added comments
assets/javascripts/discourse/initializers/discourse-calendar.js
Outdated
Show resolved
Hide resolved
| // Base margin required to shrink down the event by one day | ||
| const basePxOffset = 5.5 - segmentDuration; | ||
| // Default space between two consecutive events | ||
| // 5.5px = ( ( ( 2px margin + 3px padding ) * 2 ) + 1px border ) / 2 |
jancernik
changed the title
There was a problem hiding this comment.
Looks great locally, I tested a bit further and it's pretty solid.
I left one small comment on the acceptance test, feel free to address it in this PR or merge as is and keep an eye on test runs.
Thanks @jancernik!
| const eventElement = getEventByText("Cordoba"); | ||
|
|
||
| assert.ok(eventElement.style.marginLeft.includes("8.33")); // ( ( 1 - (-3) ) / 24 ) * 50% | ||
| assert.ok(eventElement.style.marginRight.includes("41.66")); // ( ( 24 - ( 1 - (-3) ) ) / 24 ) * 50% |
There was a problem hiding this comment.
This is cool but I am a little worried it's going to be fragile. Different browsers may be off by a hair in their rendering (for example, Firefox), and the test will fail.
I'd be ok with making this less precise. Either by checking that the margin matches the rounded value (i.e. == 8) or simply checking the presence of a margin? It's certainly not as accurate, but the test will be a bit more resilient.
There was a problem hiding this comment.
Yes, I agree, it can fail at some point, but checking only for the margin presense feels like we are only testing one conditional. Checking the round value is better. We need to parse the string to get the value, but it's not that bad. I'll update the test before merging.
1 and -3 correspond to the timezones, Lisbon (GMT+1), Cordoba (GMT-3)
This PR adds the option to enable a timezone adjustment for calendar events. This will make it so events render offset from the grid to reflect the appropriate start and end moments according to the viewer's timezone.
fullDay="true")enable_timezone_offset_for_calendar_eventsDenver (GMT-6) / Tokyo (GMT+9)
By default, grouped events (holidays) get split by timezone. This can be configured. If the timezone difference between events is less than a set threshold, events get grouped anyway and will be displayed with the timezone closest to the average.
split_grouped_events_by_timezone_thresholdThreshold
3 / 1 / 0