Create the Notify plugin to make use of the new Widget::Time helper.

[nilshoerrmann]

I'm not sure if getTimeAgo() should be part of the DateTimeObj or if it shouldn't be a widget instead:

https://github.com/symphonycms/symphony-2/blob/integration/symphony/lib/core/class.datetimeobj.php#L314-316

[brendo]

I believe it's left as part of the DateTimeObj class because otherwise the Widget class would need to check that the DateTimeObj class has been initialised.

Theoretically you could just include the Widget and XMLElement classes and know that the Widget class was going to work without having to load the Symphony stack.

I like this and we should strive to have more modular classes like this, so I think it can stay where it is to be honest.

[nilshoerrmann]

I think what I don't like is that the function generates markup whereas the rest of the class does not.

[brendo]

Agree. What about a Widget::abbr helper?

[nilshoerrmann]

I wasn't sure initially, but maybe that's a good idea.

[simoneeconomo]

Perhaps the method could return an array of two elements, the first being self::get(DateTime::RFC2822) and the second self::get($format). Then, in your markup, every time you need the abbr element, you can use Widget::abbr.

[nilshoerrmann]

In an ideal world, the returned array you described could be passed as variable to Widget::abbr directly. Something like:

$timeago = DateTimeObj->getTimeAgo();
Widget::abbr($timeago);

[simoneeconomo]

Yeah that's a good idea!

[brendo]

Sounds good to me. Either of you want to take it on?

[simoneeconomo]

eKoeS is assigned

;)

[brendo]

Awesome, thanks :D

[simoneeconomo]

eheh, we need you taking care of more important stuff!

[simoneeconomo]

Uhm, before getting my hands dirty with that: a quick grep "getTimeAgo" ./ -r shows that the current DateTimeObj::getTimeAgo() is being used in every core content page:

./symphony/content/content.blueprintssections.php:                                  DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintssections.php:                                  DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintsevents.php:                                    DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintsevents.php:                                    DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.publish.php:                                 DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.publish.php:                                 DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintspages.php:                         DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintspages.php:                                 DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintspages.php:                                 DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintsdatasources.php:                                   DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintsdatasources.php:                                   DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.systemauthors.php:                                   DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.systemauthors.php:                                   DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintsutilities.php:                                 DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/content/content.blueprintsutilities.php:                                 DateTimeObj::getTimeAgo(__SYM_TIME_FORMAT__),
./symphony/lib/core/class.datetimeobj.php:      public static function getTimeAgo($format){

It's relatively easy to bulk replace every occurrence with the new one, but I'd like to pause and think of the real benefit of that. Apart from making the class "semantically correct", is this change going to introduce more tangible benefits? How many extensions would be affected by that? Is it worth the pain?

Sorry for asking, I just want to make sure that we are doing the right thing here.

[brendo]

It will affect any extension that provides it's own /content pages and has pretty much copied the same code from the core for triggering the 'save/error' notices.

I don't see any real benefits either, so as I previously said, I'm happy for it to remain where it is.

[nilshoerrmann]

For me it's conceptually wrong, so I'm all for the change.

[simoneeconomo]

I think I'll let you fight to the death until there's a winner here! :) Any other opinions on that? /cc @nickdunn @czheng etc...

[nickdunn]

If it kills other extensions then I'm not sure. A quick search found a lot of extensions.

If you need the raw "time ago" value we could add another parameter to this method to return just the date and no markup, but it remains defaulting to markup. I agree that it's conceptually in the wrong place, but kind of a pain for extension developers to change everywhere.

[nilshoerrmann]

I've been thinking about this again, and maybe I found a solution we can all agree on:

• We deprecate getTimeAgo() in 2.3 leaving it in the Symphony source until 2.4.
• In Symphony 2.3, the core should switch from the abbr element to the new HTML5 time element. Therefore we should add a new Widget::time($string, $format, $pubdate = false) that automatically creates the needed output. (If $format is not given, it could default to the system date automatically). There would be no need for an extra function inside DateTimeObj.
• Symphony 2.3 will keep the styling for the abbr element so that extensions won't break.

In the long run, we'll have a cleaned-up DateTimeObj that no longer has to take care of any markup. And the backend will become more semantically accurate because the abbr element has always been a weak and unsemantic solution.

What do you think?

[simoneeconomo]

Handing this task off to Nils who's got a better understanding of the issue :)

[brendo]

So next step is to change the core to use the new Widget::Time method and create the new Notify plugin to use Widget::Time. The beauty of this is because we don't have an existing Notify plugin, we don't have to worry about backwards compatibility :D