Use moment to render dates everywhere #1114
Conversation
xPaw
added
the
Type: Feature
| lastWeek: "L", // Locale | ||
| sameElse: "L" | ||
| lastWeek: "MMMM DD, YYYY", | ||
| sameElse: "MMMM DD, YYYY" |
There was a problem hiding this comment.
I don't think this is a win. That way of displaying dates is only common in very few countries, so relying on locales is safer (unless I'm missing something).
I think this should be dealt with when internationalizing the client, and left alone until then.
There was a problem hiding this comment.
I think locale formatting is not that good, especially when we can't specify that we want the longer representation. The format I chose here is not american, but rather a sane approach to formatting dates, where full month name is displayed. This removes any confusion that there can be for "is it month or day?"
Also good read on the topic: https://www.troyhunt.com/10-lessons-for-uncultured-web-developers/ (see 2.)
There was a problem hiding this comment.
Ah ah, calling me uncultured much? :D
I would be arguing if the entire app was consistent, but since we are not even consistent with one format (I thought we were, with the American one, but didn't realize we had 16:30, I'm just too vested in this format to see it!), then not so much.
I do think we'll need to bring back this discussion when we localize the whole client, but until then, I'm convinced (especially with the conversion from 02-04 into 2 May. Or is it 4 February? :D)
However, since we're here, let's go for DD MMMM YYYY. It feels more balanced (no need for a comma to visually separate day and year), it represents 10 times more populations than the alternative according to your link, etc. (it's also the standard date format for W3C standards :D)
There was a problem hiding this comment.
(especially with the conversion from
02-04into2 May. Or is it4 February? :D)
It'd be 2 April anyway :D
There was a problem hiding this comment.
Can we keep the comma? DD MMMM, YYYY
There was a problem hiding this comment.
Looking at https://en.wikipedia.org/wiki/Date_format_by_country and others, it looks like 1 May 2017 is the most common, May 1, 2017 is essentially US-only, but 1 May, 2017 exists virtually nowhere.
Do you mind if we go for the first of the 3 versions?
| module.exports = function(time) { | ||
| return new Date(time).toLocaleString(); | ||
| return moment(time).format("MMMM DD, YYYY – HH:mm:ss"); |
There was a problem hiding this comment.
Similar comment here and above. While the previous change (L to MMMM DD, YYYY) forces an American standard, this one will change from 4:30:00 AM to 16:30:00, actually parting from the American standard. So even if we forget about internationalization and go for an opinionated change, there is a loss of consistency in the standard used.
(Though currently tooltip show 4:30:00 AM but message time shows 16:30... Ah, if only we had proper localization :D)
There was a problem hiding this comment.
Exactly, messages already use 24 hour format, and so will this.
There was a problem hiding this comment.
Yeah, my comment in parens betrayed me. I never really noticed that 12/24h format difference before today.
Same comment as before though, I'd advocate for DD MMMM YYYY, HH:mm:ss.
|
|
||
| return h + ":" + m; | ||
| module.exports = function(time) { | ||
| return moment(time).format("HH:mm"); |
There was a problem hiding this comment.
Need this for #98 implementation to avoid conflicts. Hope it will be merged soon.
There was a problem hiding this comment.
As long as it's a "Show seconds" checkbox and not a time format text input, then it should be fine 😄
There was a problem hiding this comment.
It's a "Show seconds" checkbox, don't worry 😆
|
"1 May, 2017" feels odd, I don't recall seeing this anywhere. "1 May 2017"
looks more balanced and is nicer when adding the time as we need only one
(or zero) separator.
Nitpick, granted, but I do think it's more standard-looking :)
|
Use moment to render dates everywhere
Also changes dates to be rendered like
29 April 2017.