Skip to content

Fuzzy time #341

Closed
wants to merge 20 commits into from

5 participants

@andrew-smith

Feature request #141 - Live update fuzzy time.

Things to note:

  • Server side processing writes out "37 seconds" (example) and client side will change it to "about a minute ago". If user has a slow loading time then they will notice a jump when the plugin runs
  • The messaging pages don't have the dates in elements
@kemitche kemitche and 2 others commented on an outdated diff Feb 9, 2012
r2/example.ini
@@ -14,7 +14,7 @@ template_debug = false
reload_templates = true
# use uncompressed static files (out of /static/js and /static/css)
# rather than compressed files out of /static (for development if true)
-uncompressedJS = true
+uncompressedJS = false
@kemitche
kemitche added a note Feb 9, 2012

Be sure to undo this! :)

@andrew-smith
andrew-smith added a note Feb 9, 2012

Should there be some checks in place before accessing the r.strings array then? In dev mode it will throw a few errors because r.strings would not have been defined.

@chromakode
chromakode added a note Feb 22, 2012

Running with uncompressedJS == True worked for me. Perhaps you forgot to run make?

@andrew-smith
andrew-smith added a note Feb 22, 2012

Reverted back to true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andrew-smith andrew-smith commented on the diff Feb 9, 2012
r2/r2/templates/utils.html
@@ -516,7 +516,7 @@
<%def name="timestamp(date, since=None)">
## todo: use pubdate attribute once things are <article> tags.
## note: comment and link templates will pass a CachedVariable stub as since.
- <time title="${long_datetime(date)}" datetime="${date.isoformat()}">
+ <time class="fuzzy-time" title="${long_datetime(date)}" datetime="${date.isoformat()}">
${unsafe(since or timesince(date))}
@andrew-smith
andrew-smith added a note Feb 9, 2012

Could place the "ago" string here so that pages are rendered with "posted 37 seconds ago" and therefore will be a less noticeable jump when the plugin changes it to "about a minute ago"

@chromakode
chromakode added a note Feb 22, 2012

It looks like timesince itself has support for adding an "ago" suffix if you pass bare=False. It might make sense to add support for a language-specific prefix there to mirror timeago. Then you could just pass bare=False in this function.

What do you think @kemitche? Is a language-specific prefix and suffix sufficient for translators to handle fuzzy times, taking them out of the containing translatable string? (see the strings changes above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andrew-smith andrew-smith commented on the diff Feb 9, 2012
r2/r2/templates/utils.html
@@ -516,7 +516,7 @@
<%def name="timestamp(date, since=None)">
## todo: use pubdate attribute once things are <article> tags.
## note: comment and link templates will pass a CachedVariable stub as since.
- <time title="${long_datetime(date)}" datetime="${date.isoformat()}">
+ <time class="fuzzy-time" title="${long_datetime(date)}" datetime="${date.isoformat()}">
@andrew-smith
andrew-smith added a note Feb 9, 2012

Now that I think about it... because all the elements will have a datetime field - does it need class="fuzzy-time" ?

@chromakode
chromakode added a note Feb 22, 2012

I think it is sensible to include a class to avoid any other time elements being unintentionally affected someday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chromakode chromakode and 1 other commented on an outdated diff Feb 22, 2012
r2/r2/public/static/js/reddit.js
@@ -1235,6 +1235,28 @@ $(function() {
$("#shortlink-text").click(function() {
$(this).select();
});
+
+ /* Load fuzzy time i18n strings */
+ $.timeago.settings.strings = {
+ prefixAgo: null,
@chromakode
chromakode added a note Feb 22, 2012

Can you add the prefix strings to strings.py so that they can be used by localizations?

@chromakode
chromakode added a note Feb 22, 2012

Yep! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chromakode chromakode and 1 other commented on an outdated diff Feb 22, 2012
r2/r2/public/static/js/reddit.js
+ suffixFromNow: r.strings.ta_suffixFromNow,
+ seconds: r.strings.ta_seconds,
+ minute: r.strings.ta_minute,
+ minutes: r.strings.ta_minutes,
+ hour: r.strings.ta_hour,
+ hours: r.strings.ta_hours,
+ day: r.strings.ta_day,
+ days: r.strings.ta_days,
+ month: r.strings.ta_month,
+ months: r.strings.ta_months,
+ year: r.strings.ta_year,
+ years: r.strings.ta_years
+ };
+
+ /* Init fuzzy time on time elements */
+ $("time.fuzzy-time").timeago();
@chromakode
chromakode added a note Feb 22, 2012

I've been working on overhauling the JS code organization, and the mishmash of initialization code in reddit.js is best considered legacy. Could you please move this initialization code to its own file/object like r.timeago (see r.analytics as an example) and init it at the bottom of base.js?

@andrew-smith
andrew-smith added a note Feb 22, 2012

Would you prefer the file name to be r.timeago.js or just timeago.js?
The code for timeago is in lib/jquery.timeago.js - should I move that into the same file as the init function or leave it as it is?

@chromakode
chromakode added a note Feb 22, 2012

I'd suggest defining an object named r.timeago in a file named something like fuzzytime.js (to match the class name). Since the fuzzy time behavior is being provided by a library, I'd leave it as is in the js/lib dir.

@andrew-smith
andrew-smith added a note Feb 23, 2012

Cool - that's done now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chromakode

Looks good, thanks!

@andrew-smith

Interesting new library: http://momentjs.com/ (Found it on reddit!)

@xPaw
xPaw commented Sep 2, 2012

Looks good 👍

@spladug
reddit member
spladug commented Aug 30, 2013

I'm going to close this pull request as I think it'll work better if we build full-site live timestamps off of https://github.com/reddit/reddit-plugin-liveupdate/blob/master/reddit_liveupdate/public/static/js/timetext.js. Thank you a tonne for making this pull request as it pointed out many of the i18n-related issues we needed to take into account for such a system.

@spladug spladug closed this Aug 30, 2013
@andrew-smith

Cheers for that. I had fun checking it out so that's what counts, right? ;)

I'll look into something else that I'll contribute to reddit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.