From 7faa9ac7bd9a36c0ae916a8d11c1f2deedca27ae Mon Sep 17 00:00:00 2001 From: jathpala Date: Fri, 29 Jul 2016 10:42:14 +1000 Subject: [PATCH] Better support 24-hour time (#2622) * Added support for 24-hour time to the thread list view (Issue #682) * Add 24-hour time support to the thread list scroll tooltip (Issue #682) * Fix for 24-hour time in the thread list scroll tooltip (#682) Correctly imports the DateUtils module * Add support for 24-hour time to the draft threads list (Issue #682) * Add 24-hour time to the message sidebar * Fix for 24-hour time in the message view so the rollover tooltip is 24hour also (#682) * Removed unused date functions from utils fullTimeString and shortTimeString from src/flux/models/utils.coffee were not compatible with 24-hour time. These functions were modified and moved to DateUtils in src/date-utils. * Fix for display of 24-hour time in the message view (Issue #682) * Removed unused import of Utils in a couple of files Prompted by Travis build errors. * Updates to handling of date/time display Incorporates changes suggested by @bengotow. Re-enables support for the isDetailed property in message-timestamp (if this is set to true, a medium length date/time string will be used for display). Re-enables additional display varieties based on when the email was received. Note that this is implemented slightly different to the orinal version - time is now given as an absolute time rather than "... days ago" format. TZ guessing moved to the global scope of date-utils for performance reasons. * Minor de-linting * Re-enable all tests by unfocusing the test suite A previous commit (ad04775) added an fdescribe() to one of the tests in draft-helpers-spec. This changes that to a regular describe() so that all tests will be run when running ./N1 --test. * Added tests for the new DateUtils functions Added tests for getTimeFormat, mediumTimeString and fullTimeString. Removed no longer relevant tests from message-timestamp-spec as _formattedDate has been removed in favour of the functions in date-utils. To test shortTimeString, we need to be able to set a fake current time which is possible in jasmine 2.0+ but not in 1.3 which is currently in use. As a possible bug, when running more than 10 tests the following warning is raised: "(node:25025) Warning: Possible EventEmitter memory leak detected. 11 on-config-reloaded listeners added. Use emitter.setMaxListeners() to increase limit", source: internal/process/warning.js (24) * Minor de-linting --- .../draft-list/lib/draft-list-send-status.jsx | 4 +- .../message-list/lib/message-timestamp.cjsx | 32 +---- .../spec/message-timestamp-spec.cjsx | 22 --- .../lib/sidebar-related-threads.jsx | 4 +- .../thread-list/lib/thread-list-columns.cjsx | 10 +- .../lib/thread-list-scroll-tooltip.cjsx | 4 +- spec/date-utils-spec.es6 | 128 ++++++++++++++++++ src/date-utils.es6 | 90 +++++++++++- src/flux/models/utils.coffee | 23 ---- 9 files changed, 234 insertions(+), 83 deletions(-) diff --git a/internal_packages/draft-list/lib/draft-list-send-status.jsx b/internal_packages/draft-list/lib/draft-list-send-status.jsx index 4742ca8e1..02723babc 100644 --- a/internal_packages/draft-list/lib/draft-list-send-status.jsx +++ b/internal_packages/draft-list/lib/draft-list-send-status.jsx @@ -1,5 +1,5 @@ import React, {Component, PropTypes} from 'react' -import {Utils} from 'nylas-exports' +import {DateUtils} from 'nylas-exports' import {Flexbox} from 'nylas-component-kit' import SendingProgressBar from './sending-progress-bar' @@ -24,6 +24,6 @@ export default class DraftListSendStatus extends Component { ) } - return {Utils.shortTimeString(draft.date)} + return {DateUtils.shortTimeString(draft.date)} } } diff --git a/internal_packages/message-list/lib/message-timestamp.cjsx b/internal_packages/message-list/lib/message-timestamp.cjsx index 10d4f6672..c314f1313 100644 --- a/internal_packages/message-list/lib/message-timestamp.cjsx +++ b/internal_packages/message-list/lib/message-timestamp.cjsx @@ -15,35 +15,15 @@ class MessageTimestamp extends React.Component +nextProps.date isnt +@props.date or nextProps.isDetailed isnt @props.isDetailed render: => - msgDate = moment.tz(@props.date, Utils.timeZone) - nowDate = @_today() - formattedDate = @_formattedDate(msgDate, nowDate, @props.isDetailed) -
{formattedDate}
- - _formattedDate: (msgDate, now, isDetailed) => - timeFormat = DateUtils.getTimeFormat upperCase: true - if isDetailed - return msgDate.format "MMMM D, YYYY [at] #{timeFormat}" + if @props.isDetailed + formattedDate = DateUtils.mediumTimeString(@props.date) else - diff = now.diff(msgDate, 'days', true) - isSameDay = now.isSame(msgDate, 'days') - if diff < 1 and isSameDay - return msgDate.format timeFormat - if diff < 1.5 and not isSameDay - timeAgo = msgDate.from now - monthAndDay = msgDate.format timeFormat - return monthAndDay + " (" + timeAgo + ")" - if diff >= 1.5 and diff < 365 - return msgDate.format "MMM D" - if diff >= 365 - return msgDate.format "MMM D, YYYY" + fromattedDate = DateUtils.shortTimeString(@props.date) +
{formattedDate}
# Stubbable for testing. Returns a `moment` _today: -> moment.tz(Utils.timeZone) - - - module.exports = MessageTimestamp diff --git a/internal_packages/message-list/spec/message-timestamp-spec.cjsx b/internal_packages/message-list/spec/message-timestamp-spec.cjsx index 868a02ac7..8a900c84a 100644 --- a/internal_packages/message-list/spec/message-timestamp-spec.cjsx +++ b/internal_packages/message-list/spec/message-timestamp-spec.cjsx @@ -18,25 +18,3 @@ describe "MessageTimestamp", -> feb28 = moment([2015, 1, 28]) mar01 = moment([2015, 2, 1]) expect(mar01.diff(feb28, 'days')).toBe 1 - - it "displays the full time when in detailed timestamp mode", -> - expect(@item._formattedDate(msgTime(), null, true)).toBe "February 14, 2010 at 3:25 PM" - - it "displays the time from messages shown today", -> - now = msgTime().add(2, 'hours') - expect(@item._formattedDate(msgTime(), now)).toBe "3:25 PM" - - it "displays the time from messages yesterday with the relative time if it's less than 36 hours ago", -> - now = msgTime().add(21, 'hours') - expect(@item._formattedDate(msgTime(), now)).toBe "3:25 PM (21 hours ago)" - - now = msgTime().add(30, 'hours') - expect(@item._formattedDate(msgTime(), now)).toBe "3:25 PM (a day ago)" - - it "displays month, day for messages less than a year ago, but more than 24 hours ago", -> - now = msgTime().add(2, 'months') - expect(@item._formattedDate(msgTime(), now)).toBe "Feb 14" - - it "displays month, day, and year for messages over a year ago", -> - now = msgTime().add(2, 'years') - expect(@item._formattedDate(msgTime(), now)).toBe "Feb 14, 2010" diff --git a/internal_packages/participant-profile/lib/sidebar-related-threads.jsx b/internal_packages/participant-profile/lib/sidebar-related-threads.jsx index bf4f8dbc6..9be4ecd31 100644 --- a/internal_packages/participant-profile/lib/sidebar-related-threads.jsx +++ b/internal_packages/participant-profile/lib/sidebar-related-threads.jsx @@ -1,5 +1,5 @@ import React from 'react' -import {Utils, Actions} from 'nylas-exports' +import {Actions, DateUtils} from 'nylas-exports' export default class RelatedThreads extends React.Component { static displayName = "RelatedThreads"; @@ -60,7 +60,7 @@ export default class RelatedThreads extends React.Component { {subject} {snippet} - {Utils.shortTimeString(lastMessageReceivedTimestamp)} + {DateUtils.shortTimeString(lastMessageReceivedTimestamp)} ) }) diff --git a/internal_packages/thread-list/lib/thread-list-columns.cjsx b/internal_packages/thread-list/lib/thread-list-columns.cjsx index 6fe156fd5..11c3b2bc9 100644 --- a/internal_packages/thread-list/lib/thread-list-columns.cjsx +++ b/internal_packages/thread-list/lib/thread-list-columns.cjsx @@ -1,6 +1,7 @@ _ = require 'underscore' React = require 'react' classNames = require 'classnames' +moment = require 'moment' {ListTabular, RetinaImg, @@ -8,7 +9,7 @@ classNames = require 'classnames' MailImportantIcon, InjectedComponentSet} = require 'nylas-component-kit' -{Thread, FocusedPerspectiveStore, Utils} = require 'nylas-exports' +{Thread, FocusedPerspectiveStore, Utils, DateUtils} = require 'nylas-exports' {ThreadArchiveQuickAction, ThreadTrashQuickAction} = require './thread-list-quick-actions' @@ -17,11 +18,14 @@ ThreadListParticipants = require './thread-list-participants' ThreadListStore = require './thread-list-store' ThreadListIcon = require './thread-list-icon' +# Get and format either last sent or last received timestamp depending on thread-list being viewed TimestampComponentForPerspective = (thread) -> if FocusedPerspectiveStore.current().isSent() - {Utils.shortTimeString(thread.lastMessageSentTimestamp)} + rawTimestamp = thread.lastMessageSentTimestamp else - {Utils.shortTimeString(thread.lastMessageReceivedTimestamp)} + rawTimestamp = thread.lastMessageReceivedTimestamp + timestamp = DateUtils.shortTimeString(rawTimestamp) + {timestamp} subject = (subj) -> if (subj ? "").trim().length is 0 diff --git a/internal_packages/thread-list/lib/thread-list-scroll-tooltip.cjsx b/internal_packages/thread-list/lib/thread-list-scroll-tooltip.cjsx index 405d50940..2e625b823 100644 --- a/internal_packages/thread-list/lib/thread-list-scroll-tooltip.cjsx +++ b/internal_packages/thread-list/lib/thread-list-scroll-tooltip.cjsx @@ -1,5 +1,5 @@ React = require 'react' -{Utils} = require 'nylas-exports' +{Utils, DateUtils} = require 'nylas-exports' ThreadListStore = require './thread-list-store' class ThreadListScrollTooltip extends React.Component @@ -25,7 +25,7 @@ class ThreadListScrollTooltip extends React.Component render: -> if @state.item - content = Utils.shortTimeString(@state.item.lastMessageReceivedTimestamp) + content = DateUtils.shortTimeString(@state.item.lastMessageReceivedTimestamp) else content = "Loading..."
diff --git a/spec/date-utils-spec.es6 b/spec/date-utils-spec.es6 index c6eded991..bf79b14d9 100644 --- a/spec/date-utils-spec.es6 +++ b/spec/date-utils-spec.es6 @@ -42,4 +42,132 @@ describe('DateUtils', function dateUtils() { expect(thisWeekend.format('MM-DD-YYYY')).toEqual('03-12-2016') }); }); + + describe('getTimeFormat: 12-hour clock', () => { + beforeEach(() => { + spyOn(NylasEnv.config, 'get').andReturn(false) + }); + + it('displays the time format for a 12-hour clock', () => { + const time = DateUtils.getTimeFormat(null) + expect(time).toBe('h:mm a') + }); + + it('displays the time format for a 12-hour clock with timezone', () => { + const opts = { timeZone: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm a z') + }); + + it('displays the time format for a 12-hour clock with seconds', () => { + const opts = { seconds: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm:ss a') + }); + + it('displays the time format for a 12-hour clock with seconds and timezone', () => { + const opts = { seconds: true, timeZone: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm:ss a z') + }); + + it('displays the time format for a 12-hour clock in uppercase', () => { + const opts = { upperCase: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm A') + }); + + it('displays the time format for a 12-hour clock in uppercase with seconds', () => { + const opts = { upperCase: true, seconds: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm:ss A') + }); + + it('displays the time format for a 12-hour clock in uppercase with timezone', () => { + const opts = { upperCase: true, timeZone: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm A z') + }); + + it('displays the time format for a 12-hour clock in uppercase with seconds and timezone', () => { + const opts = { upperCase: true, seconds: true, timeZone: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('h:mm:ss A z') + }); + }); + + describe('getTimeFormat: 24-hour clock', () => { + beforeEach(() => { + spyOn(NylasEnv.config, 'get').andReturn(true) + }); + + it('displays the time format for a 24-hour clock', () => { + const time = DateUtils.getTimeFormat(null) + expect(time).toBe('HH:mm') + }); + + it('displays the time format for a 24-hour clock with timezone', () => { + const opts = { timeZone: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('HH:mm z') + }); + + it('displays the time format for a 24-hour clock with seconds', () => { + const opts = { seconds: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('HH:mm:ss') + }); + + it('displays the time format for a 24-hour clock with seconds and timezone', () => { + const opts = { seconds: true, timeZone: true } + const time = DateUtils.getTimeFormat(opts) + expect(time).toBe('HH:mm:ss z') + }); + }); + + describe('mediumTimeString: 12-hour time', () => { + beforeEach(() => { + spyOn(NylasEnv.config, 'get').andReturn(false) + }); + + it('displays a date and time', () => { + const datestring = DateUtils.mediumTimeString('1982-10-24 22:45') + expect(datestring).toBe('October 24, 1982, 10:45 PM') + }); + }); + + describe('mediumTimeString: 24-hour time', () => { + beforeEach(() => { + spyOn(NylasEnv.config, 'get').andReturn(true) + }); + + it('displays a date and time', () => { + const datestring = DateUtils.mediumTimeString('1982-10-24 22:45') + expect(datestring).toBe('October 24, 1982, 22:45') + }); + }); + + describe('fullTimeString: 12-hour time', () => { + beforeEach(() => { + spyOn(NylasEnv.config, 'get').andReturn(false) + self.tz = moment.tz(moment.tz.guess()).zoneAbbr() + }); + + it('displays a date and time', () => { + const datestring = DateUtils.fullTimeString('1982-10-24 22:45') + expect(datestring).toBe(`Sunday, October 24th 1982, 10:45:00 PM ${self.tz}`) + }); + }); + + describe('fullTimeString: 24-hour time', () => { + beforeEach(() => { + spyOn(NylasEnv.config, 'get').andReturn(true) + self.tz = moment.tz(moment.tz.guess()).zoneAbbr() + }); + + it('displays a date and time', () => { + const datestring = DateUtils.fullTimeString('1982-10-24 22:45') + expect(datestring).toBe(`Sunday, October 24th 1982, 22:45:00 ${self.tz}`) + }); + }); }); diff --git a/src/date-utils.es6 b/src/date-utils.es6 index a76f2e37e..ce1a53482 100644 --- a/src/date-utils.es6 +++ b/src/date-utils.es6 @@ -1,10 +1,15 @@ -import moment from 'moment' +import moment from 'moment-timezone' import chrono from 'chrono-node' import _ from 'underscore' // Init locale for moment moment.locale(navigator.language) +// Initialise moment timezone +const tz = moment.tz.guess() +if (!tz) { + console.error("DateUtils: TimeZone could not be determined. This should not happen!") +} const yearRegex = / ?YY(YY)?/ @@ -156,21 +161,100 @@ const DateUtils = { return moment(date) }, + + /** + * Return a formatting string for displaying time + * + * @param {Date} opts - Object with different properties for customising output + * @return {String} The format string based on syntax used by Moment.js + * + * seconds, upperCase and timeZone are the supported extra options to the format string. + * Checks whether or not to use 24 hour time format. + */ getTimeFormat(opts) { const use24HourClock = NylasEnv.config.get('core.workspace.use24HourClock') let timeFormat = use24HourClock ? "HH:mm" : "h:mm" + if (opts && opts.seconds) { timeFormat += ":ss" } - if (!use24HourClock && opts && opts.upperCase) { - timeFormat += " A" + + // Append meridian if not using 24 hour clock + if (!use24HourClock) { + if (opts && opts.upperCase) { + timeFormat += " A" + } else { + timeFormat += " a" + } } + if (opts && opts.timeZone) { timeFormat += " z" } return timeFormat }, + + + /** + * Return a short format date/time + * + * @param {Date} datetime - Timestamp + * @return {String} Formated date/time + * + * The returned date/time format depends on how long ago the timestamp is. + */ + shortTimeString(datetime) { + const now = moment() + const diff = now.diff(datetime, 'days', true) + const isSameDay = now.isSame(datetime, 'days') + let format = null + + if (diff <= 1 && isSameDay) { + // Time if less than 1 day old + format = DateUtils.getTimeFormat(null) + } else if (diff < 2 && !isSameDay) { + // Month and day with time if up to 2 days ago + format = `MMM D, ${DateUtils.getTimeFormat(null)}` + } else if (diff >= 2 && diff < 365) { + // Month and day up to 1 year old + format = "MMM D" + } else { + // Month, day and year if over a year old + format = "MMM D YYYY" + } + + return moment(datetime).format(format) + }, + + + /** + * Return a medium format date/time + * + * @param {Date} datetime - Timestamp + * @return {String} Formated date/time + */ + mediumTimeString(datetime) { + let format = "MMMM D, YYYY, " + format += DateUtils.getTimeFormat({seconds: false, upperCase: true, timeZone: false}) + + return moment(datetime).format(format) + }, + + + /** + * Return a long format date/time + * + * @param {Date} datetime - Timestamp + * @return {String} Formated date/time + */ + fullTimeString(datetime) { + let format = "dddd, MMMM Do YYYY, " + format += DateUtils.getTimeFormat({seconds: true, upperCase: true, timeZone: true}) + + return moment(datetime).tz(tz).format(format) + }, + }; export default DateUtils diff --git a/src/flux/models/utils.coffee b/src/flux/models/utils.coffee index 1fed99bf2..33e51bb6e 100644 --- a/src/flux/models/utils.coffee +++ b/src/flux/models/utils.coffee @@ -3,12 +3,6 @@ fs = require('fs-plus') path = require('path') moment = require('moment-timezone') -# Attempts to use Intl.DateTimeFormat().resolvedOptions().timeZone, falls back -# to intelligently guessing based on how key dates over one year are formatted. -tz = moment.tz.guess() -if not tz - console.error("Utils:TimeZone could not be determined. This should not happen!") - DefaultResourcePath = null TaskRegistry = require('../../task-registry').default DatabaseObjectRegistry = require('../../database-object-registry').default @@ -58,23 +52,6 @@ Utils = v.__constructorName = type return v - timeZone: tz - - shortTimeString: (time) -> - return "" unless time - diff = moment().diff(time, 'days', true) - if diff <= 1 - format = "h:mm a" - else if diff > 1 and diff <= 365 - format = "MMM D" - else - format = "MMM D YYYY" - moment(time).format(format) - - fullTimeString: (time) -> - return "" unless time - moment(time).tz(Utils.timeZone).format("dddd, MMMM Do YYYY, h:mm:ss a z") - fastOmit: (props, without) -> otherProps = Object.assign({}, props) delete otherProps[w] for w in without