pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
297 stars 442 forks source link

Support per-user time zones #6082

Open asmecher opened 4 years ago

asmecher commented 4 years ago

See https://forum.pkp.sfu.ca/t/per-user-timezone/59311.

Allow users to configure their time zone so that dates can be formatted more usably.

maifeld commented 4 years ago

The desired situation, in short: The server running OJS is set to UTC as is the setting in config.inc.php: "20:00 GMT" Alice is in New York (UTC -4) and every current time she sees in OJS displays: "16:00 EDT" Bob is in Los Angeles (UTC -7) and at the same instant every time he sees is displayed by OJS: "13:00 PDT" Charlie is in Honolulu (UTC -10) and calls them to talk about a 10 am meeting while he sees: "10:00 HST" The workaround for this optimal solution is to at least display the timezone. Else, Alice, Bob, and Charlie all see "20:00" and presume it is in their current time.

NateWr commented 4 years ago

Instead of user-configurable time zones, I'd like to see us draw on the system clock. This is where working at the JavaScript level in the browser has a significant benefit and it should be possible for any UI components going forward.

asmecher commented 4 years ago

@NateWr, what about due dates sent by email? I suppose there's an implicit assumption that a due date is midnight per the assigning editor's local time zone, though this starts to get pretty fuzzy pretty fast. We could simply choose not to make assumptions about emailed dates and send them as is...

NateWr commented 4 years ago

@asmecher we don't allow times when setting a review due date, though we store it in a dateTime column in the database. That probably deserves to be reconciled. I'd be tempted to do away with times for those, since in practice these are rarely hard deadlines and usually just trigger a follow-up automated or manual (our reminders and things work on a next day basis, I think).

asmecher commented 4 years ago

Agreed, @NateWr. I've been expecting a case to pop up from the back of my mind where we needed to supply time information to a user through a means other than their browser, but I can't think of a good one. :+1: to handling time zones via the browser. What about date formats (localization in particular) to boot?

NateWr commented 4 years ago

For localization of date formats I think we depend on existing translations. That was discussed in this comment.

I suppose the two are going to be related. We'll need to get an overall datetime as YYYY-MM-DD HH:MM:SS, get the timezone offset of the server, and in the browser adjust the displayed datetime, while still using the formats in the config file... I don't see how we can get around requiring our translators to provide what's needed for the translation (see linked comment above).

asmecher commented 4 years ago

If I remember correctly, what we're missing is a good strftime implementation in JS that would allow us to format dates consistently on both server and client side. If we move time zone handling to the browser, I think it's worth moving wholesale to client-based date formatting, relying on toLocaleDateString.

If we only have dates in emails to consider for server-side formatting, a single strftime format string in the config file can handle that, and we don't need parallel implementations.

NateWr commented 4 years ago

If we move time zone handling to the browser,

I don't know if we'll ever be able to go fully browser or server rendering. Although toLocaleDateString provides translated month and day names, I don't think that it offers any control over the formatting. You specify the locale and it prints out the order it thinks is best for that locale. I suspect that's not going to satisfy a lot of users.

For that reason, I'm leaning towards a browser-based strftime implementation in JS. I think all we need to do is ask for day and month names from translators, and we can provide a localized translation file at run-time for each active locale.

asmecher commented 4 years ago

Although toLocaleDateString provides translated month and day names, I don't think that it offers any control over the formatting.

I was looking at the options 2nd parameter to toLocaleDateString, which supports a bunch of formatting, in particular dateStyle of full, long, medium, and short. I was thinking we could add JSON to config.inc.php to allow full customization, i.e.:

js_date_format_long = "{\"dateStyle\": \"long\"}"

If we only really have one use case for PHP-based date formatting (emails), then we'd only need one strftime format in the config file and everything else could move over to the toLocaleDateString approach.

(This isn't too different from using a strftime implementation in JS, but all the implementations I saw had very poor support for i18n.)

NateWr commented 4 years ago

I think the options parameter lets us say whether we want a long or short representation of a day/week/month, but it doesn't allow complete formatting like strftime. For example, it doesn't allow us to change the order of information or decide what delimiters we want.

Themes are another place where we won't be rendering dates in JavaScript. We're also introducing, right now, a setting for date formats to be configured per journal and locale using strftime: https://github.com/pkp/pkp-lib/issues/5540. That will create more confusion if dates don't match (for the moment, most editors/managers don't know about the config option).

I just don't think we're going to get around needing a common date formatter that we can use in the server and the browser. If all we need to support our languages is to ask our translators to provide day, week and month names, I think that they can provide that.

asmecher commented 4 years ago

I've already been too persistent for my depth of knowledge in Javascript compared to yours :) but I do hope we can avoid manual translation of day and month names: image ...or alternately we could perhaps pass this in from PHP where we'll already have it? I'm just hoping we can hold back the contributions to climate change made by unnecessary translations. But I'll cede the point if you've got specific plans.

NateWr commented 4 years ago

We could use that to generate the translations. I'm hesitant to do that at run-time because it feels like code that shouldn't run on every request and we'd need to make sure it was all calculated before any other JavaScript executes.

Another option would be to have a build script. A simple Node script could generate the necessary JS object whenever npm run dev or npm run build is run, and the appropriate one could be loaded into the pkp global based on the active locale. (We could also have a build script that simply adds the entries to a .po script automatically, so that translators don't need to do the work, but the values would be available for editing in places like the Custom Locales plugin.)

Either way I don't think this route would give us the necessary entries for the formats object that the strftime module requires. Perhaps we can source some of them from the config file / journal settings, or maybe just not support those characters.

NateWr commented 4 years ago

@asmecher would @Vitaliy-1's approach to mapping between strftime and moment.js solve our needs here? https://github.com/pkp/ui-library/pull/113/files#diff-4fbdab4bbbf5fd9f4b13dfa4d50ec6b5R16-R40 We could roll that into a globally available function and use it wherever we need to display datetimes in the browser.

asmecher commented 4 years ago

:+1: :+1: :+1: Yes, this is great.

NateWr commented 4 years ago

I notice that the map does not include some of the composite datetime formats that strftime supports. Is it ok for us to define a limited set of characters that we support?

asmecher commented 4 years ago

Yes, and perhaps to document those in the config.TEMPLATE.inc.php if that can be done briefly?