Events displayed in venue timezone instead of user's

[iandunn]

​#meta4480 describes how the Events API returns data as a "WP timestamp" (Unix timestamp + UTC offset), rather than a true Unix timestamp, which is always UTC.

Because of that, events are displayed in the venue's timezone, rather than the user's. That's usually accurate for in-person events, but usually inaccurate for online events, where attendees can be located in any timezone.

Once the API starts returning a proper Unix timestamp, Core should update the widget to display events in the current user's timezone.

cc @rarst, @dd32

[Rarst]

Per that ticket current API return is local event date and time without time zone, not WP timestamp.

[iandunn]

Ah, ok, I couldn't find much info on them beyond just "Unix timestamp plus UTC offset". That how we ​convert Meetup.com API timestamps are converted to local time. What's the distinction?

[Rarst]

No, I mean your understanding of WP timestamp is correct, but at the time I opened the meta issue timestamps weren't returned, the API return was "date": "2019-06-18 19:00:00". I don't know if that's still the state of it, assumed so.

[iandunn]

Ah, that's right. The patch on ​#meta4480 will add new fields, but hasn't been committed yet.

[...] events are displayed in the venue's timezone, rather than the user's. That's usually accurate for in-person events, but usually inaccurate for online events, where attendees can be located in any timezone.

See https://core.trac.wordpress.org/ticket/51130
See ​https://meta.trac.wordpress.org/ticket/4480

Please use your own fork for PRs because custom branches will get deleted due to the mirror syncing.

Ah, I misread the handbook, thanks.

[...] events are displayed in the venue's timezone, rather than the user's. That's usually accurate for in-person events, but usually inaccurate for online events, where attendees can be located in any timezone.

See https://core.trac.wordpress.org/ticket/51130
See ​https://meta.trac.wordpress.org/ticket/4480

[hlashbrooke]

This is an excellent idea - with all meetups being online, it makes a lot more sense for the time to show in the user's timezone. Could we indicate that at the top of the widget and state which timezone it is? So for me it would say "Times shown in local timezone (UTC+2)". Your patch may already do that, but I can't test locally at the moment.

[iandunn]

Shows UTC offset next to the city name

[iandunn]

Shows UTC offset and timezone ID next to the event time

[iandunn]

That's an interesting idea 🤔

I'd thought about adding it next to the event time, but it felt too cluttered. Putting it next to the city time would mitigate that. I uploaded some screenshots above that show examples of those.

It also shows the difference between using the UTC offset, and the timezone abbreviation. I think I'm leaning towards the timezone abbreviation, since that's shorter, and would be more familiar for most people (I'm assuming).

If we go with the timezone abbreviation, then maybe it would be better to place it next to the event time? That'd be more directly connected to the time, and more likely to be seen.

There may be some technical challenges getting either of them, though, though. Date().getTimezoneOffset() and new Date().toLocaleTimeString('en-us',{timeZoneName:'short'}).split(' ')[2] have various issues with browser support. moment().tz("America/Los_Angeles").zoneAbbr() might not be worth the performance hit for the enqueue. I guess we could try the two native methods, and fallback to an empty string if neither are available?

[iandunn]

xref ticket:47798#comment:31 for discussion about internationalizing the start/end month in format_event_data_time().

[audrasjb]

As soon as they are not displayed and not passed to any hook, those strings don't need to be prepared for i18n.

[sippis]

This sounds like a great improvement! For example user in Stockholm does not have any events coming up in Sweden, but we are having a WordCamp Finland Online later this year. Sweden is UTC+2 and we are living UTC+3 in Finland, so user currently would see wrong time if it would be meetup instead of WordCamp. And our Helsinki group might have online meetup which would interest them. Stockholm and Helsinki are so close to each other, that events in one of those cities will most likely be visible in both cities at the moment at least.

Replying to iandunn:

I'd thought about adding it next to the event time, but it felt too cluttered. Putting it next to the city time would mitigate that. I uploaded some screenshots above that show examples of those.

I think it's enough to show the timezone next to the city. Putting it next to event time makes the view too cluttered indeed. Though in that case, the user might think the event times are still displayed in the venue/local time rather than in their own timezone and add more confusion.

Some visual indication that tells how times are shown, as @hlashbrooke suggested, would be really good addition.

It also shows the difference between using the UTC offset, and the timezone abbreviation. I think I'm leaning towards the timezone abbreviation, since that's shorter, and would be more familiar for most people (I'm assuming).

Personally I'm leaning towards showing UTC offset rather than timezone abbreviation. It really depends on the location of user to which they are used to. In my mind timezone abbreviations are something that is used in US and UK, but not in central Europe for example and I've grown to use UTC offset since the elementary school :D

[iandunn]

In my mind timezone abbreviations are something that is used in US and UK, but not in central Europe for example

Ah, great point. Maybe we should use an I18n'd string to let translators pick which one, then? e.g.,

/* translators: Do not translate. Set either `abbreviation` or `offset`, in English. This determines if the Events Widget shows timezone abbreviations or offsets. Pick whichever is most common in your locale. */
if ( 'abbreviation' === __( 'events-widget-timezone-display' ) ) {
    // show the timezone abbreviation
} else {
    // show the UTC offset
}

[sippis]

Sounds a bit hacky, but it works! :D If translators do not translate or translate it incorrectly, using UTC offset as a fallback +1

Actually it could be usable if core would handle the same in bit more elegant way in future - but I guess that's not going to happen in timespan which this events widget change is wanted to go out.

[iandunn]

Yeah, I've seen Core do it in other places, but I agree it'd be nice if there were a more elegant way.

I don't think there's any way to get it via the locale/tz info available via JavaScript. There may be some other canonical source, though. I'll see what I can find.

Or we could just use the UTC offset for simplicity, if that's what's most common around the world.

[iandunn]

Er, nevermind, it does look like ​toLocaleTimeString() will do that. ​#pr539 is already using it, but w/ the locale hardcoded. I wanted to remove that anyway, though. ​Leaving that param empty should default to the user's (browser/BCP47) locale. As long as we can still parse it out reliably, that should work.

[iandunn]

I've updated the PR to show the localized abbreviation 👍🏻

---

Note to self:

To manually test abbreviations and timezones on OS X, open the browser with overridden environment variables:

TZ="Europe/Helsinki" open /Applications/FirefoxDeveloperEdition.app

To verify: console.log( Intl.DateTimeFormat().resolvedOptions().timeZone );

You don't need to change the OS/browser locale, because (I'm guessing) the timezone shortname is pulled from ​tzdata, rather than locale data.

@rarst, @ryelle, @dd32 - This is still a WIP, but it's roughly working for me. If you see any big problems with the general approach, please let me know.

The test failure is unrelated, see #core51446.

I think this is ready for more eyes.

I requested review from Kelly and Dion. @Rarst, GitHub won't let me request it from you formally, but I'd love to hear any feedback you have as well.

Nothing jumps out at me in PHP, not really familiar with date stuff in JS so no opinion there. :)

👍🏻 , thanks.

Oh, it looks like the notStrictEqual( daylightSavingAbbreviation, standardAbbreviation ); assertion _is_ failing in Travis, unrelated to the CORS error.

That's odd, since it passes locally, and in a Win7/IE11 VM. Looking into it.

Ah, I think it's because Travis is running with a UTC timezone, so there is no difference. Stubbing it will probably fix.

I rewrote that test to stub the Date functions, but at that point it's not testing anything that the other tests aren't. So, I just removed it.

I think this is ready for review, despite the Travis errors. I'm still digging into those, but they appear to be unrelated. Possibly related to #core51466.

Made a few changes, and the Travis issues are resolved now.

Do you think any more changes are needed, @ocean90 ?

@iandunn From an i18n perspective this is looking good. Created #51498 to use wp.i18n for the remaining strings which I will commit once this has landed.

I'd probably update the @since tags to use 5.6.0 since there doesn't seem to be a 5.5.2 in the near future.

I'd probably update the @since tags to use 5.6.0 since there doesn't seem to be a 5.5.2 in the near future.

👍🏻 updated in f48e101d9258bfc6e3fb86b637ef372122e92101

Awesome, thanks!

[iandunn]

In 49145:

Community Events: Trim events by Unix timestamp for accuracy.

The date and end_date fields are WP timestamps representing the venue's local time. As of ​meta:changeset:10270 (​#meta4480), new start_unix_timestamp and end_unix_timestamp values are available, providing a proper Unix timestamp in the UTC timezone. Using those is more precise, and removes the time window where the event has expired but still appears in the Events Widget.

To simplify the function, it now only accepts and returns the events themselves, rather than the entire response body.

See #51130
See ​#meta4480
Related: ​https://make.wordpress.org/core/2019/09/23/date-time-improvements-wp-5-3/

[iandunn]

In 49146:

Community Events: Display dates and times in the user's time zone.

Fixes #51130
Props sippis, hlashbrooke, audrasjb, Rarst, iandunn

[audrasjb]

@iandunn Hi! I saw you moved it to milestone 5.6. While we're preparing 5.5.2, I just wanted to add that if it's a high priority, I'm all for backporting this to 5.5.2 which will probably ship in two weeks or so :)

[iandunn]

In 49147:

Community Events: Apply coding standards.

The previous commits intentionally didn't include these, because it would have added an unreasonable amount of diff noise.

See #51130

[iandunn]

Ah, I didn't realize it was going to happen after all, thanks!

I'll update the @since tags and backport 👍🏻

[audrasjb]

Great! 👍

[iandunn]

In 49152:

Community Events: Update timezone-related @since tags to 5.5.2.

r49145 and r49146 were originally planned for 5.6 when they were committed, but are now planned for 5.5.2.

See #51130.

Merged in r49145, r49146, r49147.

[iandunn]

Reopening for backport.

[SergeyBiryukov]

In 49201:

Docs: Add a @deprecated note to WP_Community_Events::format_event_data_time().

Follow-up to [49146].

See #51130.

[tellyworth]

In 49275:

Community Events: Display dates and times in the user's time zone.

Fixes #51130
Merges [49145], [49146], [49147], [49152], and [49201] to the 5.5 branch.
Props sippis, hlashbrooke, audrasjb, Rarst, iandunn

[iandunn]

Huh, ​some of the new QUnit tests are failing in the `5.5` branch, even though they pass in trunk.

>> dashboard > communityEvents.getFormattedDate - multiple day event should use corresponding format
>> Actual: "September 15–17, 2020"
>> Expected: "September 15–17, 2020"
>> at Object.<anonymous> (file:///home/travis/build/WordPress/wordpress-develop/tests/qunit/wp-admin/js/dashboard.js:106:12)

>> dashboard > communityEvents.getFormattedDate - multiple month event should use corresponding format
>> Actual: "September 15 – October 6, 2020"
>> Expected: "September 15 – October 6, 2020"
>> at Object.<anonymous> (file:///home/travis/build/WordPress/wordpress-develop/tests/qunit/wp-admin/js/dashboard.js:117:12)

I saw that happen sometimes when I was writing them, but couldn't reproduce it reliably, and thought copy/pasting might have fixed it. Apparently not :)

I'm gonna take another look.

En dashes (8211) and hyphens (45) are getting mixed up, causing some QUnit tests to fail. They all appear to be en dashes in the code itself, but there could be something in dateI18n / Moment that is converting to a hypen.

See 51130#comment:48

021e8ef demonstrates the problem, but isn't necessarily the best solution.

Trac ticket: https://core.trac.wordpress.org/ticket/51130

[SergeyBiryukov]

Looks like some encoding failure. It would be great to find the source, but if it turns out to be too hard to consistently reproduce, it would probably be easier to replace the – character in the affected strings with the – entity:

• %1$s %2$d–%3$d, %4$d
• %1$s %2$d – %3$s %4$d, %5$d

That would make the strings less readable to translators, but should still work. I think I went with the character in [47124] for readability only.

[SergeyBiryukov]

Actually, it looks like 51130.diff​ resolves this:
​https://travis-ci.com/github/SergeyBiryukov/wordpress-develop/builds/192442752

<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />

This line already exists in trunk since [49101], but not in the 5.5 branch. That explains the difference.

Commented on the ticket: https://core.trac.wordpress.org/ticket/51130#comment:52

Adding this to qunit/index.html appears to resolve the issue:

<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />

This line already exists in trunk since [49101], but not in the 5.5 branch. That explains the difference.

[SergeyBiryukov]

In 49307:

Tests: Declare UTF-8 encoding for QUnit test suite.

This was done for trunk in [49101] as part of other changes, and is now backported to the 5.5 branch.

Follow-up to [49275].

Props iandunn, SergeyBiryukov.
Fixes #51130.

Ah, that makes sense, thanks!