Update the post schedule label to correctly reflect the date and time display settings

[brentswisher]

Description

Fixes #15654.

Updates the post schedule label to correctly reflect a user's date and time display preferences. This was previously using the "datetimeAbbreviated" format, which is not updated when a user changes their date/time settings.

Additionally, now if a user specifies a custom date or time format, it will be correctly reflected in the publish date.

An alternative to this approach would be to update the functionality that is setting the settings.formats.date and settings.formats.time to correctly set the time in settings.formats.datetime, and settings.formats.datetimeAbbreviated.

I opted for my approach over this because it will also pass through any totally custom date or time setting. Also, because I couldn't for the life of me find where settings.formats.time was getting updated when the site settings were updated.

If the other approach is preferred, I would be happy to revisit. Someone would just need to point me in the right direction for where those get set. It looks like it used to be in lib/client-assets.php but got removed here

How has this been tested?

For 24 hour time
Change time settings to use 24h time. (Settings > General > Time Format > H:i)
Create a post.
Schedule for any time past 12:00. (ie, 13:25).
Close popover by clicking outside.
Publish date now correctly lists 12:25

For custom date/time settings
Change date/time settings to use something custom:
(Settings > General > Time Format > G:i)
(Settings > General > Date Format > d-m-Y)
Create a post.
Schedule for any time before 12:00. (ie, 1:25).
Close popover by clicking outside.
Publish date now correctly display custom format of 21-05-2019 01:25

Screenshots

24 hour time

Custom date/time settings

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[jorgefilipecosta]

Hi @brentswisher, thank you for your contribution 👍
Would it make sense to make this change in core:

https://github.com/WordPress/WordPress/blob/28a8f31ffa9dc238b56eb2413a728e861207b5af/wp-includes/script-loader.php#L615

Making datetimeAbbreviated format be the concatenation of time and date?

[brentswisher]

Potentially, I wasn't sure where it was defined in core - so thanks for pointing that out! That leads to another question - what is the purpose of the "datetimeAbbreviated" format? If I set my date and time format to custom things that are quite long, does it make sense to set something called dateTimeAbbreviated to that? Will updating that in core cause design issues in places where they are expecting a set formatting mask? There seemed to be a lot of questions I didn't know the answer to so I went with the smallest change to fix the issue, but if you think it warrants a larger discussion I am happy to pursue that as well.

[brentswisher]

@jorgefilipecosta - My last comment had more questions and no answers so I tried to track this down a little better.

Investigating it looks like:

• 'datetimeAbbreviated' as a format was added in this commit by @aldavigdis specifically for the use here in the post-schedule component
• It is not used anywhere else in Gutenberg I can find
• It is not used anywhere else in WordPress core I can find
• The "I can find" part here is important, I am not confident enough in my understanding of WordPress as a whole to make the call is really isn't used anywhere and can be removed

I would propose we:

1. Implement the change I have in this PR, so that 24hr time is supported and this issue can be closed.
2. Open a separate issue here and in trac asking if the 'datetimeAbbreviated' format needs to remain, as it looks like it is not used. That way it will hopefully get some more experienced eyes on it that may know the history before removing it.

[aldavigdis]

A lot of bike shedding went on when I worked on this a year ago, but datetimeAbbreviated was added to satisfy some design requirements and review if I remember it correctly.

I'm not sure if decisions had been made on how to handle localisation of dates by then, but at the time, there were some issues with parsing dates from PHP in the React components.

[jorgefilipecosta]

Hi @swissspidy, would it be possible to provide some feedback regarding the i18n of this string?

[tellthemachines]

Both settings.formats.date and settings.formats.time are put through the translation function in the script loader. I've tested these changes with several different language settings and all looks to be working fine:

I'm not sure what context datetime or datetimeAbbreviated could be useful in as they set a very specific formatting, as opposed to date and time which reflect the user settings. AFAICT neither of them are being used anywhere but, for the sake of fixing this annoying bug quickly, I vote we get this in and perhaps create a ticket to clean up all those formats later.

[mapk]

I just can't get this running on my local to test. Sorry.

[tellthemachines]

These changes look good!

An alternative to this approach would be to update the functionality that is setting the settings.formats.date and settings.formats.time to correctly set the time in settings.formats.datetime, and settings.formats.datetimeAbbreviated.

I'm not sure that would make much difference, as it would just be a concatenation of the date and time settings that come from wp_admin. It's more a question of if we need a static datetime that is independent of what the user sets, or not. If it's not being used, probably best to scrap it altogether.

[mapk]

Just tested this and was able to get it running locally. It looks and works great from a design perspective. 👏

I switched around a few different settings and they worked.

[brentswisher]

@jorgefilipecosta is this okay to merge or did you still have some reservations?