WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 9 days ago

#39222 assigned feature request

Add JavaScript date helpers

Reported by: rmccue Owned by: rmccue
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.7
Component: Date/Time Keywords: has-patch needs-unit-tests needs-docs
Focuses: rest-api Cc:

Description

With the REST API, it's now possible to receive raw dates (ISO8601 format) on the frontend, without having the site's date preferences applied.

Formatting dates is a pain in JavaScript, and having it match the site preferences and translations is a pain too. We should introduce a helper library to simplify this, based on our PHP date_i18n() and human_time_diff() functions.

Attachments (2)

39222.diff (74.2 KB) - added by rmccue 7 weeks ago.
Add wp-date.js
39222.2.diff (76.3 KB) - added by rmccue 2 weeks ago.
Updated documentation, and other tweaks

Download all attachments as: .zip

Change History (25)

#1 @rmccue
5 months ago

Note: for proper translations, we'd also need to include a solution for translations, as we need plurals. See #20491 for that. We shouldn't get stuck on that, but would be good to simultaneously solve that.

#3 @afercia
5 months ago

Related: #38342 and #38920 as the attempts to use the REST API there need some JS date formatting utility.

#4 follow-up: @nerrad
5 months ago

Would there be consideration given to using an existing js library for dealing with dates and times (formatting, localization etc). moment.js (https://momentjs.com) seems to be a well known library in use. I've used it in a number of projects and it makes datetimes in js less painful :)

Last edited 5 months ago by nerrad (previous) (diff)

#5 in reply to: ↑ 4 @rmccue
4 months ago

Replying to nerrad:

Would there be consideration given to using an existing js library for dealing with dates and times (formatting, localization etc). moment.js (https://momentjs.com) seems to be a well known library in use. I've used it in a number of projects and it makes datetimes in js less painful :)

Moment is pretty great, but the things we need aren't fully handled by it. Primarily, localisation is something we want to handle ourselves, as is date formatting (from settings).

#6 @nerrad
4 months ago

What is it about the moment localization (https://momentjs.com/docs/#/i18n/) that isn't sufficient? If you're talking about loading our own translations (that would be provided by the wp.i18n project) then you can provide moment custom translations sets.

Setting initial format on a moment object is simple enough as well.

It seems that extending a well known/widely used existing, compatible licensed javascript library that works well with dates and times would be preferable to re-inventing the wheel (again) here.

Last edited 4 months ago by nerrad (previous) (diff)

#7 @rmccue
4 months ago

It's more that we need to have a consistent translation platform across all of WordPress (see #20491). We can integrate the two, but we also need to ensure that the date formatting settings are also respected. In particular, the Date and Time formats from the General Settings page need to be handled, which requires mapping from PHP date codes. There are also a bunch of other pieces which change date formatting dependent on the locale (see date_i18n()).

While it is less work immediately to use Moment's locales, we would then have two translation tools to maintain, and which may not be in sync. This also increases the maintenance burden for the translation teams. It's worth the additional effort to have a consistent set of translations for maintainability.

That's not to say we shouldn't add Moment, but if we do, we need to be considered and purposeful in how we handle adding it, it's not just a drop-in.

This ticket was mentioned in Slack in #core-restapi by swissspidy. View the logs.


4 months ago

This ticket was mentioned in Slack in #core by rmccue. View the logs.


7 weeks ago

#10 @rmccue
7 weeks ago

  • Owner set to rmccue
  • Status changed from new to assigned

I have a lovely little patch for this working. Ended up using moment, since it's very useful, and provides a good building block for other JS.

@rmccue
7 weeks ago

Add wp-date.js

#11 @rmccue
7 weeks ago

  • Keywords has-patch needs-unit-tests needs-docs added; needs-patch removed

39222.diff adds wp-date.js (and wp.date.*), which relies on moment.js (also included). It also removes the Ajax endpoints used for previewing formatted date/times, and replaces it with live JS instead:

http://i.imgur.com/Ht202ww.gif

This removes two admin-ajax.php endpoints (date_format and time_format), and moves them to wp-admin/includes/deprecated.php.

It introduces new functions you can use:

  • wp.date.format() - Format a date.
  • wp.date.date() - Format a date in the site's timezone.
  • wp.date.gmdate() - Format a date in UTC.
  • wp.date.date_i18n() - Format a date, and translate it.

There are some pieces missing from the date translations:

  • Ordinals aren't translated - this matches core, see #22225
  • There's no equivalent of human_time_diff() - This requires full i18n support (for plurals), and hence needs #20491 first.

Otherwise, should be basically complete feature-wise. This should probably have unit tests.

This needs docs once we merge it; I'm not sure if our JSDocs are parsed out anywhere right now, so in lieu of that, regular pages somewhere would be great.

This ticket was mentioned in Slack in #core-restapi by rmccue. View the logs.


7 weeks ago

#13 @rmccue
7 weeks ago

(If desired, we could actually have all the times on the page update live as well, such as the ones for the timezone selector. That requires a little extra work that I didn't feel was necessary, but if anyone thinks it's a good idea, go for it.)

This ticket was mentioned in Slack in #core-i18n by swissspidy. View the logs.


7 weeks ago

#15 follow-up: @flixos90
7 weeks ago

The patch is looking great as far as I can tell (but let's wait for further reviews by more JSy people than me). I had implemented the mapping in a similar way in another project (the implementation here is more complete though), so I guess it shows we're following an approach that makes sense. I agree that using Moment.js is the right choice as we don't need to reinvent the wheel, and mapping the PHP formats is rather straightforward.

Something minor to consider: Instead of hard-coding __( 'F j, Y g:i a' ) for the datetime setting, could we instead concatenate the date_format and time_format values with a space? Also, what is formats.timezone used for (I couldn't see it anywhere in the code, why is that necessary)?

#16 @nerrad
7 weeks ago

Looks great! Glad to see the decision to go with moment - it definitely removes much of the extra work that would have been done.

To echo @flixos90 and confirm the approach, I've done mapping of formats similarly in other projects as well.

Within the mapping, should the date argument be clarified in jsdocs so that its clear it is expecting a moment date object? maybe even rename the variable to dateMoment so its extra clear?

Last edited 7 weeks ago by nerrad (previous) (diff)

#17 follow-up: @swissspidy
7 weeks ago

could we instead concatenate the date_format and time_format values with a space?

Nope. String concatenation is always bad for i18n. Also, these strings already exists like that, see WP_Locale::_strings_for_pot().


Looking at wpDateSettings, it seems like a WP_Locale JS counterpart would be helpful. To get all these formats etc. of the current locale. Perhaps we can handle that in #20491 or a new ticket afterwards.

#18 in reply to: ↑ 17 @flixos90
7 weeks ago

Replying to swissspidy:

could we instead concatenate the date_format and time_format values with a space?

Nope. String concatenation is always bad for i18n. Also, these strings already exists like that, see WP_Locale::_strings_for_pot().

Ah right. What I don't like about it though is that there would be no way for the user to modify the datetime format. It might be worth considering an option for that. On the other hand this might look over-complicated to the user ("I already added date and time formats").
What about concatenating date and time format through another translation string sprintf( _x( '%1$s %2$s', 'concatenation of date and time format' ), $date_format, $time_format )? Then it would be manageable per locale. A locale that does not support concatenation of these values in any way could return an empty string, and in that case it would fall back to what we currently have there. We could even go as far as providing an option for users of a locale that doesn't support concatenation.

Yes, I might be overthinking this, but I think we need a bit more flexibility here.

#19 in reply to: ↑ 15 @rmccue
2 weeks ago

Replying to flixos90:

Something minor to consider: Instead of hard-coding __( 'F j, Y g:i a' ) for the datetime setting, could we instead concatenate the date_format and time_format values with a space?

This is one that's already hardcoded, IIRC, although looking at it now, appears the only place this is used is in the (deprecated) Links manager (links_updated_date_format option).

Would potentially make sense for this to be a wider setting in any case. There's some existing hardcoded date formats that could potentially be replaced with this (such as the date format in the posts list table).

FWIW, the comments list table currently concatenates date formats, and isn't configurable:

$submitted = sprintf( __( '%1$s at %2$s' ),
	/* translators: comment date format. See https://secure.php.net/date */
	get_comment_date( __( 'Y/m/d' ), $comment ),
	get_comment_date( __( 'g:i a' ), $comment )
);

Also, what is formats.timezone used for (I couldn't see it anywhere in the code, why is that necessary)?

It's used for the timezone selector's "Local time is ..." preview; I originally JS-ified that as well, but dropped it from the patch since it became a bit too complex. Can remove the format (the current one is translated at the top of options-general.php).

This ticket was mentioned in Slack in #core by rmccue. View the logs.


2 weeks ago

@rmccue
2 weeks ago

Updated documentation, and other tweaks

#21 @rmccue
2 weeks ago

39222.2.diff removes the unused timezone format, and updates the JSDoc significantly. (I've checked this locally using jsdoc, but needs a review to make sure it matches our standards if I missed something.) In the process, I noticed I accidentally had two definitions of Z, where one should have been z; fixed that up now.

This ticket was mentioned in Slack in #core-restapi by rmccue. View the logs.


10 days ago

This ticket was mentioned in Slack in #core by rmccue. View the logs.


9 days ago

Note: See TracTickets for help on using tickets.