Add JHtmlDate class with relative time function #370

Merged
merged 18 commits into from Nov 18, 2011

Conversation

Projects
None yet
7 participants
Owner

mbabker commented Sep 17, 2011

JHtmlDate::relative allows users to convert a given date/time combo to a relative time in minutes, hours, days, or weeks, and returns the absolute time if the date is over a month in the past.

mbabker added some commits Sep 17, 2011

@mbabker mbabker Add JHtmlDate class with relative time function
JHtmlDate::relative allows users to convert a given date/time combo to a relative time in minutes, hours, days, or weeks, and returns the absolute time if the date is over a month in the past.
e703814
@mbabker mbabker Add test skeleton 846fb67
Contributor

eddieajau commented Sep 20, 2011

Michael, I did a similar thing for work. See: https://gist.github.com/1230563

Is it worth combining the your approach with that code? Looking at mine I'd also include another argument for the base date we are comparing to so that we can test the method better.

I think we should flesh the unit tests out on this one as they shouldn't be too hard. I can give you a hand crafting the basic structure.

We also need to put the language strings somewhere and we've been kicking around the idea of having a /meta/ folder for each package. So I think, for now, we'd put them in ``/joomla/html/meta/language/en-GB/en-GB.html_html_date.ini

What do you think?

Owner

mbabker commented Sep 21, 2011

It's definitely worth combining the two approaches. Obviously, if each of us has our own methods for setting a relative date, there's probably others with the same and creating a single approach would be beneficial for all.

The only reason I didn't work out a unit test for this is because the language string dependencies. But, creating tests won't be difficult I'd say.

Coming up with a location for language strings would definitely be helpful. Be it a centralized language folder or a folder for each individual package. Truth be told, I like the concept of the individual packages better and have been moving my own code to follow that approach; it's just that much easier to find the language files.

Contributor

mahagr commented Sep 22, 2011

@eddieajau: This code only works in PHP5.3+:
date_create('now')->diff($date);

I have also similar function, but it also works in PHP 5.2 (and J!1.5) and extends JDate class, so you can use it without JHtml class. You can find it from here:

https://github.com/Kunena/Kunena-2.0/blob/master/administrator/components/com_kunena/libraries/date.php

Functions are diff() and toTimeAgo()...

@mbabker: Language strings do not prevent you from making unit tests. Just load default language and compare translated strings. At least that's what I do in my component.

Contributor

eddieajau commented Sep 23, 2011

Interesting. Ok, what say we aim to add a "diff" override to JDate. If PHP 5.3, it just returns the parent method. If 5.2, we pull in the code that Kunena is using. Then you can pull the best of the 3 approaches out and combine them into the HTML helper for displaying the date chunks. It would be good to have an option to display the number of chunks. One chunk would be the largest ("1 minute ago", "1 week ago"); two chunks would be "2 hours 33 minutes ago", "4 days 12 hours ago", and so on - though, thinking about it, one or two chunks would be the most common - three chunks at a stretch.

What do you think?

Regarding language files, here's what I've done as I've been rebuilding JHttp:
https://github.com/eddieajau/joomla-platform/tree/jhttp/libraries/joomla/client

I need to open up a thread on the platform list about this so we can agree on the best approach to cover everyone's needs. Will do that soon.

Contributor

mahagr commented Sep 24, 2011

The code I'm using came originally from php.net comment (as I say in the code), but it has been heavily modified. I'm not sure why it's not PHP5.3 compatible -- if I remember correctly I just converted my own class to extend JDate without modifying the variables..

I agree, we should add diff() override for PHP5.2 and deal the rest in jHtml helper.

BTW: My code shows up to 2 chunks, but only shows chunks that aren't 0 and ignores seconds. So you get:

1 month 3 weeks ago
1 month 2 days ago
1 month 21 hours ago
1 month 52 minutes ago
1 month ago

Another related subject: I really like the way github deals with the same issue by using JavaScript. So if we are creating helper from this, why not give option also to use JavaScript?

Owner

mbabker commented Sep 24, 2011

+1 for JS type class. And I like the 2 chunks code also.

On 9/24/11 10:25 AM, "Matias Griese"
reply@reply.github.com
wrote:

The code I'm using came originally from php.net comment (as I say in the
code), but it has been heavily modified. I'm not sure why it's not PHP5.3
compatible -- if I remember correctly I just converted my own class to
extend JDate without modifying the variables..

I agree, we should add diff() override for PHP5.2 and deal the rest in
jHtml helper.

BTW: My code shows up to 2 chunks, but only shows chunks that aren't 0
and ignores seconds. So you get:

1 month 3 weeks ago
1 month 2 days ago
1 month 21 hours ago
1 month 52 minutes ago
1 month ago

Another related subject: I really like the way github deals with the same
issue by using JavaScript. So if we are creating helper from this, why
not give option also to use JavaScript?

Reply to this email directly or view it on GitHub:
#370 (comment)

Contributor

WebMechanic commented Sep 24, 2011

The language <metadata /> could (and should) provide much more useful, structured information for dates, calendars, as well as number formats (decimal, thousands), you name it, in a similar fashion as Zend_Locale does -- though more lightweight and less "excessive": http://is.gd/sVSelJ

Thus I'd rather put locale based meta-data and keys to the language packs where they belong. After all it's not JDate's business to provide translation services and files and polute the /joomla/html/* folder with even more .ini files.
If at all, the only legitimate place to dump such locale-metadata for the Platform (for fallbacks!) would be some place like /joomla/language/meta/*.
However, overrides by an application (site, admin, cli, foobar) should still be possible, and that's where "language packs" kick in.

Then "teach" JLanguage::get($property) to deal with nested/hierarchical keys like "date.month.abbreviated.long", "date.yesterday", "date.lastweek", or "number.format.decimal" etc. to access a locale's property (from the xml) and pass that value to JLanguage::_() doing whatever string transformation necessary to make this a "valid" language key. Although from PHP's perspective date.month.abbreviated.long already perfectly qualifies as an array key and won't harm the native INI parser.

just my 2ct

Contributor

mahagr commented Sep 25, 2011

Regarding to @WebMechanic comment on supporting "last week" and "tomorrow", I think that it would also be great addition when rendering absolute dates (we already handle today and yesterday in Kunena and it would be great to ditch our class and use JDate and JHtml helpers instead). But our current subject "x time ago" or "x time to" doesn't need to handle tomorrow or yesterday as there is no yesterday or tomorrow between relative times. Yesterday can be 1 hour ago or 23 hours ago depending your current time.

Also my code handles only past dates -- in a forum you don't usually have future dates. That said, it should be very easy to implement support for both past and future dates, as DateTime::diff() already supports it and in my class it's only one new variable that needs to be added.

I'm also wondering what to do with short time periods: should we use "just now", "few minutes ago" or similar terms to make the fast changing times more fuzzy? There's no point of saying "1 second ago" as that time has already passed when the user sees the time.

Contributor

WebMechanic commented Sep 25, 2011

Does anyone know whether PHP's DateInterval is unavailable on any given version or platform?
I was using it since 5.2.9 (in a plugin for the Cantao/TypoLight CMS) and I'm yet to run into a hosting provider where it didn't work. The PHP manual once stated "might only by in SVN" but I can't "confirm" that statement :-)

As long as DateTime and DateInterval are fed with sufficient granularity in the input format, all intervals from seconds to years are set and that's where the trick of translating them into a reasonable human readable date or time "phrase" comes in.
I can see various use cases where any kind of announcement of a future event, special offer, what-not could be labelled with "tomorrow", "next week" and alike, and the same holds true for stuff that happend.

@mahagr: I believe it's up to the developer, implementor, or designer to specify a granularity for the output. Given that even 1 second difference can become anything between "just now", "yesterday" (23.59.59 < 00:00:00) or "next year" (2012-01-01 00:00:00 > 2011-12-31 23:59.59) if the granularity -- or unit -- is small or big enough.
That's exactly what $unit provides in @mbabker's proposal.
The only use case I can see to have tiny fractions such as "1 second ago" available would be on the client side where some JS counter would grab those as JavaScript strings provided via JText.

Now the question for me is: how would such language keys look like in order to handle both "colloquial" time frames as well as "technical" 2-parted chunks?
I'd love to help to iron this out 'cos I have instant usage for it :-) How'd be the procedure to combine the code of @mbabker (API part), @mahagr (math part), and what I wrote for my Contao plugin (l10n part)?

Contributor

mahagr commented Oct 8, 2011

FYI: There's already Mootools script that does what we want:

http://mootools.net/forge/p/prettydate

Owner

mbabker commented Oct 9, 2011

Unless I FUBAR'd the implementation, that JS class is about as practical as the PHP I've been using. It doesn't auto-update the time like Twitter or GitHub do.

Contributor

mahagr commented Oct 10, 2011

I think that we need to have both php and javascript version of the same class. I'm currently using also php version, but there are some downsides with it.

One of the biggest downside is that you cannot easily cache a page with relative times, but you can do that if you only use absolute times + javascript. I'm having this issue in Kunena where I have a lot of static data in a page that takes some resources to render (mostly links into categories/topics and bbcode). Caching individual entries makes it a lot faster, but then I need to replace dates with placeholders and render them by using regexps. It works for normal users, but not if you're guest and more static (page/view level) caching is enabled.

Contributor

WebMechanic commented Oct 10, 2011

@mahagr: excellent point.
Maybe it'd be a good idea to restrict the granularity in the php code "somehow" to avoid too many variations, ie. based on the global cache time -- for a starter.
Like your comment right now reads "about 4 hours ago" it's maximum cache time might be 60 minutes before it needs an update on the server to become "about 5 hours ago" -- presuming Guthub handles each comment as a distict cacheable object in the first place :-)

If the fixed date however is also written into, let's say, a data-attribute, and/or wrapped in the HTML5 element, a client side script can be as granular as the designer/implementor feels necessary.
http://dev.w3.org/html5/spec-author-view/the-time-element.html

Contributor

mahagr commented Oct 11, 2011

The issue with html5 is that IE7/8 doesn't support it. But I agree that there needs to be a way to choose the granularity so that "just now" or "about 15 minutes ago" becomes something like "less than hour ago" to avoid the issues with caching.

But I would still prefer using dynamic time for example in a busy forum, because of that tells you right away how long ago you updated the page.

This is what I would do:
Let's first fix the PHP version and only after that add a new option to show the relative dates inside javascript. Are we all OK with this?

Contributor

gnomeontherun commented Oct 11, 2011

I don't know how far back it goes, but mootools has a time diff function

http://mootools.net/docs/more/Types/Date.Extras#Date:timeDiffInWords

Contributor

mahagr commented Oct 11, 2011

But it has no word from translation capabilities.. :) But well, it's good to know that there's something official for this, too.

Contributor

eddieajau commented Oct 18, 2011

Just following up, I think we'll go with Michael's original plans but we'll need full unit tests on it before we merge it. Thanks in advance.

Owner

mbabker commented Oct 18, 2011

OK, I've started building a test case, but I'm not quite 100% sure how to handle everything. For starters, I still don't have a language file pushed which contains the language strings for this; where should I put that?

Next, how should we handle the $diff variable in the relative function since it depends on PHP's time() function? To test consistently, we'd need to set time() to be something static.

I'm sure there's more than that to address, but for me, those are the two big items right now.

Contributor

eddieajau commented Oct 18, 2011

Michael, for the language, put them in /joomla/html/language/en-GB/en-GB.jhtmldate.ini for now.

For the testing, add a third argument, defaulting to null, that takes the comparison date. If it's null, use "now". Does that help?

Owner

mbabker commented Oct 18, 2011

Reworked things a little bit, the only test case that is failing locally for me at the moment is the one for right now.

@chdemko chdemko commented on an outdated diff Nov 5, 2011

libraries/joomla/html/html/date.php
+ * @return string The converted time string
+ *
+ * @since 11.3
+ */
+ public static function relative($date, $unit = null, $time = null)
+ {
+ // Convert $time to the appropriate measure
+ if (is_null($time))
+ {
+ $start = time();
+ }
+ else
+ {
+ $start = strtotime($time);
+ }
+ // Get the difference in seconds between now and the time
@chdemko

chdemko Nov 5, 2011

Contributor

For having correct unit test, you should replace

    // Convert $time to the appropriate measure
    if (is_null($time))
    {
        $start = time();
    }
    else
    {
        $start = strtotime($time);
    }
    // Get the difference in seconds between now and the time
    $diff = $start - strtotime($date);

by

    if (is_null($time))
    {
        // Get now
        $time = JFactory::getDate('now');
    }

    // Get the difference in seconds between now and the time
    $diff = strtotime($time) - strtotime($date);

it's because time() return your local time (not the UTC one)

Contributor

joomla-jenkins commented Nov 10, 2011

Build triggered by changes to the base.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 233 warnings and 4165 errors.

Owner

mbabker commented Nov 13, 2011

The only failure I'm getting on the tests locally is the one for the "Less than a minute ago" string. It appears that the call to JFactory::getDate('now') in the test suite is done at the beginning of running the tests, so when that test is actually run ~2-3 minutes later (on my system), it fails. Went ahead and commented it out for now. Otherwise, should be clear from the code style and test failures.

Contributor

joomla-jenkins commented Nov 13, 2011

Build triggered by changes to the head.

Unit testing complete. There were 1 failures and 0 errors from 1699 tests and 10615 assertions.
Checkstyle analysis reported 235 warnings and 0 errors.

Contributor

joomla-jenkins commented Nov 14, 2011

Build triggered by changes to the head.

Unit testing complete. There were 1 failures and 0 errors from 1708 tests and 10640 assertions.
Checkstyle analysis reported 235 warnings and 0 errors.

Contributor

joomla-jenkins commented Nov 14, 2011

Build triggered by changes to the head.

Unit testing complete. There were 0 failures and 0 errors from 1708 tests and 10641 assertions.
Checkstyle analysis reported 235 warnings and 0 errors.

@chdemko chdemko commented on an outdated diff Nov 14, 2011

libraries/joomla/html/html/date.php
+ {
+ return JText::plural('JLIB_HTML_DATE_RELATIVE_DAYS', $diff);
+ }
+
+ // Round to weeks
+ $diff = round($diff / 7);
+
+ // 1 to 4 weeks
+ if ($diff <= 4 || $unit == 'week')
+ {
+ return JText::plural('JLIB_HTML_DATE_RELATIVE_WEEKS', $diff);
+ }
+
+ // Over a month, return the absolute time
+ return JHtml::date($date);
+ }
@chdemko

chdemko Nov 14, 2011

Contributor

I think it's better to use JHtml::_('date', $date); (using the registered date function which could be different from the JHtml::date one)

Contributor

joomla-jenkins commented Nov 16, 2011

Build triggered by changes to the base.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 235 warnings and 0 errors.

Contributor

joomla-jenkins commented Nov 16, 2011

Build triggered by changes to the head.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 235 warnings and 0 errors.

@chdemko chdemko added a commit that referenced this pull request Nov 18, 2011

@chdemko chdemko Merge pull request #370 from mbabker/relative
Add JHtmlDate class with relative time function
7d562a2

@chdemko chdemko merged commit 7d562a2 into joomla:staging Nov 18, 2011

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