Add relative time formatting #395

mbirtwell · 2015-02-13T17:07:23Z

Another go at formatRelativeCount from #389.

It has the fix-205-format-duration branch merged in
It's in the duration module
I've renamed it to formatTimeOffset, which I think is a bit better (but naming is still hard).
It uses formatMessage and Globalize.plural rather than my hacky alternatives
I've included a skeleton implementation of the formatDuration function as per Support relative time #205 implemented using my formatTimeOffset method

Also I've created this as a pull request against master because otherwise the diff was massive and you couldn't really see what I'd done. I understand that you consider formatDuration to be the real goal here and so may chose to merge this in to fix-205-format-duration (or not at all - but I like to be optimistic).

Ref globalizejs#205

rxaviers · 2015-02-16T14:17:17Z

Thanks for your new PR.

Also I've created this as a pull request against master...

Yeap, better having this PR to compare against master.

It's in the duration module

Ok for now. Let's rename it "relative-time". If this module doesn't add too many bytes, it could live inside the existing date module.

It has the fix-205-format-duration branch merged in

I've renamed it to formatTimeOffset, which I think is a bit better (but naming is still hard).

It uses formatMessage and Globalize.plural rather than my hacky alternatives

I've included a skeleton implementation of the formatDuration function as per Support relative time #205 implemented using my formatTimeOffset method

Updates... I have updated our API according to what we discussed (#389 (comment)). I've closed #205 in favor of the clean #391. I've rebased my old fix-205-format-duration branch on top of up-to-date master, made the changes according to #391 and pushed it into a new branch called fix-391-relative-time.

Can you rebase your changes on top of fix-391-relative-time please? Use .formatRelativeTime() and relativeTimeFormatter() names instead of your custom-named .formatTimeOffset(). Note, you don't need to implement anything based on Date instances for now (if you don't want to). Keep your implementation based on (number, unit). We can add Date instance support in upcoming commits.

Only support numbers as value parameter for the moment

mbirtwell · 2015-02-18T15:14:02Z

I've done this based on fix-391-relative-time now. Let me know what you think?

rxaviers · 2015-02-18T19:39:35Z

Awesome. I've scanned through your code and saw you got the spirit. I will do a better review soon (couple of days).

A quick comment though, make sure you follow jQuery's style guide.

rxaviers · 2015-02-19T12:52:37Z

src/relative-time.js

+ * - minWordOffset [Optional Number] The maximum offset when special offset words like
+ *  yesterday and tomorrow will be looked for. Some languages provide several of these.
+ *  default null -> use all available
+ *  Set to 0 to not use any except today, now etc.


Interesting option...

Does any other programming language have something similar?

Given Ecma-402 names are in the extensive form (e.g., minimumFractionDigits) and we're (kinda) following it where we can, it would be good to change minWordOffset to minimumWordOffset and max... accordingly.

I'm curious why minWordOffset is here, but maxWordOffset is there. I recommend having the options descriptions in one place (there) and having here saying: "see .relativeTimeFormatter() for details"

There was only meant to be maxWordOffset (minWordOffset was a typo).
I don't know of any other libraries which provide a similar option but I thought I had a use case which for suppressing the use of yesterday in favour of always using the x days ago form, but I changed my mind in the end.

Alright about minWordOffset. Thanks.

About maxWordOffset, I can easily think of a use case where "1 day ago" is preferred than yesterday. Although, I don't think of any use case where one wants yesterday, but not before yesterday. Do you have one?

mbirtwell · 2015-03-01T13:50:50Z

Sorry one of my commits (from a week ago) was been created with my work email address. I've signed to CLA for that email address as well now.

rxaviers · 2015-03-05T19:02:17Z

examples/amd-bower/main.js

@@ -91,4 +94,6 @@ require([
 	document.getElementById( "requirements" ).style.display = "none";
 	document.getElementById( "demo" ).style.display = "block";

+	document.getElementById( "relative-time").innerText = en.formatRelativeTime( -35, 'second' );


Need extra space and to replace ' for ":

- document.getElementById( "relative-time").innerText = en.formatRelativeTime( -35, 'second' ); + document.getElementById( "relative-time" ).innerText = en.formatRelativeTime( -35, "second" );

rxaviers · 2015-03-05T20:06:57Z

@mbirtwell sorry for the delayed response. Thanks for the updates. I have added a couple of review comments.

rxaviers · 2015-03-05T20:27:16Z

src/relative-time/format.js

+ */
+return function( value, numberFormatter, pluralGenerator, properties ) {
+
+	var relativeTime, message = properties[ "relative-type-" + value ];


message = needs to go in its own line http://contribute.jquery.org/style-guide/js/#assignments

rxaviers · 2015-03-05T20:59:43Z

@mbirtwell it's looking great. Please, just let me know when you have updates.

Most interesting is removing an incorrect example and adding instance method example

rxaviers · 2015-03-11T13:11:59Z

@mbirtwell thanks for your updates so far.

rxaviers · 2015-03-26T15:17:00Z

@mbirtwell, I went ahead and made the final fixes in a new commit.

I want to let you know I really appreciate your work here. Thanks and keep in touch. Your further contributions are welcome.

PS: Although CLA task has been marked as failing. I've checked it manually and it is correct (3/1/2015 8:47:57 Michael Birtwell michael.birtwell@starleaf.com I AGREE).

Ref #395

- Style updates. - Add default CLDR validation. - Minor edits. Amends 47cbe1207a30ac8d3ba9b0ab3ef8a6107e2b421a Ref #391 Ref #395

rxaviers added 2 commits February 16, 2015 11:53

Relative Time: Create it (Skeleton)

1642b91

Ref globalizejs#205

fixup! Relative Time: Create it (Skeleton)

cf9e57b

rxaviers and others added 2 commits February 16, 2015 12:23

fixup! Relative Time: Create it (Skeleton)

856456b

Add simple relative time support

cb79465

Only support numbers as value parameter for the moment

mbirtwell force-pushed the fix-205-format-duration branch from bc6ef1b to cb79465 Compare February 18, 2015 15:07

rxaviers reviewed Feb 19, 2015
View reviewed changes

mbirtwell added 2 commits March 1, 2015 12:59

Add missing semi-colons

d31ea90

Update examples

d92cef3

jquerybot added the CLA: Error label Mar 1, 2015

rxaviers reviewed Mar 5, 2015
View reviewed changes

Minor updates to relative time

3e99c5e

Most interesting is removing an incorrect example and adding instance method example

rxaviers mentioned this pull request Mar 11, 2015

Relative time: Additional features #391

Open

rxaviers mentioned this pull request Mar 26, 2015

Add relative time formatting #389

Closed

rxaviers changed the title ~~Add a formatTimeOffset method~~ Add relative time formatting Mar 26, 2015

rxaviers added a commit that referenced this pull request Mar 26, 2015

Relative time: Create it (Skeleton)

a79ac57

Ref #395

rxaviers closed this in 655d821 Mar 26, 2015

rxaviers added a commit that referenced this pull request Mar 26, 2015

Relative time: Amend "Add simple support"

7916eba

- Style updates. - Add default CLDR validation. - Minor edits. Amends 47cbe1207a30ac8d3ba9b0ab3ef8a6107e2b421a Ref #391 Ref #395

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add relative time formatting #395

Add relative time formatting #395

mbirtwell commented Feb 13, 2015

rxaviers commented Feb 16, 2015

mbirtwell commented Feb 18, 2015

rxaviers commented Feb 18, 2015

rxaviers Feb 19, 2015

rxaviers Feb 19, 2015

mbirtwell Feb 19, 2015

rxaviers Feb 19, 2015

mbirtwell commented Mar 1, 2015

rxaviers Mar 5, 2015

rxaviers commented Mar 5, 2015

rxaviers Mar 5, 2015

rxaviers commented Mar 5, 2015

rxaviers commented Mar 11, 2015

rxaviers commented Mar 26, 2015

Add relative time formatting #395

Add relative time formatting #395

Conversation

mbirtwell commented Feb 13, 2015

rxaviers commented Feb 16, 2015

mbirtwell commented Feb 18, 2015

rxaviers commented Feb 18, 2015

rxaviers Feb 19, 2015

Choose a reason for hiding this comment

rxaviers Feb 19, 2015

Choose a reason for hiding this comment

mbirtwell Feb 19, 2015

Choose a reason for hiding this comment

rxaviers Feb 19, 2015

Choose a reason for hiding this comment

mbirtwell commented Mar 1, 2015

rxaviers Mar 5, 2015

Choose a reason for hiding this comment

rxaviers commented Mar 5, 2015

rxaviers Mar 5, 2015

Choose a reason for hiding this comment

rxaviers commented Mar 5, 2015

rxaviers commented Mar 11, 2015

rxaviers commented Mar 26, 2015