Adding in "datetime" and "timespan" support

[jeffolmstead]

Implementing datetime and timespan taghelpers.
Fixes #2965

[jeffolmstead]

Implements solution for issue #2965

[sebastienros]

I don't think it's a good idea to implement a tag helper for every shape like this. Maybe @Skrypt missunderstood the suggestion when he acknowledged.

[Skrypt]

I think that if the idea is to follow the Liquid documentation for creating Liquid templates then we should provide most commonly used ones. I did not look at the details of this implementation but what would be the downside of providing more taghelpers ...?

So here it's a TagHelper for Razor but this PR should also include the proper related Liquid filters.

[ns8482e]

Following liquid syntax is already supported and works in OC.

https://shopify.github.io/liquid/filters/date/

{{ "March 14, 2016" | date: "%b %d, %y" }}
This page was last updated at {{ "now" | date: "%Y-%m-%d %H:%M" }}.

[Skrypt]

Let me think. Then why we need to use this in one of our theme template?

{% assign format = "MMMM dd, yyyy" | t %}
{% assign dateTime = "DateTime" | shape_new: utc: Model.ContentItem.CreatedUtc, format: format | shape_stringify %}
{{ "Posted by" | t }} <a href="#">{{ Model.ContentItem.Owner }}</a> {{ "on {0}" | t: dateTime | raw }}

We make the format translatable because we need to pass it to a shape so that we apply a string.Format()

And here some reading :
https://www.shopify.com/partners/blog/liquid-date-format
https://www.w3.org/TR/2011/WD-html5-author-20110809/the-time-element.html

So one benefit would be using a proper <time /> html element.

[mawendel]

Actually its not possible to get the UTC time in Liquid afaik? Yes you can read an existing from a content item, but if say you wanted to get the new/current time in UTC then that would not be possible.

In js you could do New Date().Utc, but that is not available in Liquid. So i think we need at least a special filter or tag for Utc time.

[ns8482e]

Actually its not possible to get the UTC time in Liquid afaik?

{{ "now" | date: "%z" }} will give you UTC offset.

[mawendel]

Actually its not possible to get the UTC time in Liquid afaik?

{{ "now" | date: "%z" }} will give you UTC offset.

Oh cool, i'll give it a try thanks!

[mawendel]

Actually its not possible to get the UTC time in Liquid afaik?

{{ "now" | date: "%z" }} will give you UTC offset.

No, it does not work. It just returns "+01:00" . I would like a way to just return the current actual UTC time (and not the timezone itself).

[ns8482e]

It would be nice to have some C# flavored liquid filters for date time similar to powerapps portal https://docs.microsoft.com/en-us/powerapps/maker/portals/liquid/liquid-filters#date-filters

[agriffard]

@Skrypt Can you please review this PR?

[Skrypt]

You can pass a format for localizing the date but I think it misses badly allowing to pass a timezone. Of course you could alter the datetime passed before rendering the shape but why not do this directly in the shape? Also we should consider rendering a proper HTML element like this : https://www.w3schools.com/tags/att_time_datetime.asp.

{{ article.createdUtc | time_tag: '%a, %b %d, %Y', datetime:'%Y-%m-%d' | time_zone: "America/New_York" }}
<-- <time datetime="2019-04-30">Tue, Apr 30, 2019</time> -->

This PR is incomplete as is. Also please look at DateTimeShapes.cs to compare and see if options cannot be added there directly. Also you can see that we convert dates automatically in our Shapes.

{% assign dateTime = "DateTime" | shape_new: utc: Model.ContentItem.CreatedUtc, format: format | shape_stringify %}

Also, here I think these 2 new proposed TagHelpers will render as <datetime /> and <timespan /> which are unexisting HTML elements. So these would need to be rendered as HTML Web Components. I don't think our TagHelpers should rely on any javascript lib for rendering.

[jeffolmstead]

@Skrypt and @agriffard I think first we need to decide what we want. This was written way back in February and even when it was written, I had stated you don't technically need the liquid helper, you can actually call the ""DateTime" and "TimeSpan" shape directly, just using longer liquid syntax.

The only strong argument for writing it was to have an equivalent helper to what was already implemented in Razor tag helpers. As it stands now, the "DateTime" and "TimeSpan" shapes might accept more parameters / render different tags, I haven't checked. I can update this to "modernize" it against what those shape helpers currently do, but only valuable to do that if we feel we actually need it (apparently in Razor we felt we needed it as someone implemented the tag helper).

Just let me know, because once these make it into core, the expectation I am sure is that they would stay equivalent to the Razor tag helpers.

[Skrypt]

I agree that the shape and razor tag helpers should return the same result if that's what you mean by equivalent. Here I noted the fact that the Shapes are returning dates only and that the TagHelpers will require to use 2 different non-existing html elements and which will render as a date in the end.

Though what I'm saying is that the TagHelpers are not using the _clock to apply a timezone to those UTC dates which the DateTimeShapes do by default and we should probably allow to use a W3C accepted HTML element to make this easier for everyone. Why not using <time /> tag in which we can pass/render a datetime or a timespan. At least make it an option to render as a HTML element or a date text but in the end the first should be used more often.

Examples :

{% assign format = "MMMM dd, yyyy" | t %}
{{ "Posted by" | t }} <a href="#">{{ Model.ContentItem.Owner }}</a> {{ "DateTime" | shape_new: utc: Model.ContentItem.CreatedUtc, format: format | time_tag: "on {0}" | t: format }}
<-- Posted by <a href="#">Admin</a><time datetime="2019-04-30">on Tue, Apr 30, 2019</time> -->

For a timespan :

<-- <time datetime="PT15H10M">15 hours and 10 minutes ago</time>

Here my examples are not perfectly adequate but you get the idea.

[agriffard]

Can you please make the suggested changes?

[jeffolmstead]

@agriffard I don't know what the suggested changes are. This pull request simply set out to expose a liquid equivalent for the shapes of "TimeSpan" and "DateTime" as they are documented to exist but didn't. Even now, re-reading the conversations, I don't know what anyone else is looking for. The conversation seems to have evolved that we should go and change the "TimeSpan" and "DateTime" shapes so they render in a " HTML tag. However, that, to me, should not be a part of this pull request (e.g. those shapes can be overwritten in another pull request without impacting this one). This pull request simply exposes those shapes via Liquid.

Perhaps I am missing something though and am very open to correction / clarification. Or perhaps there actually is the desire to use this pull request to clean up those other shapes, I just don't know for sure. Look forward to thoughts.

[Skrypt]

It's been too long and I forgot the discussion too. But, just like the PagerShape is returning html elements I think we should have a Shape that returns proper HTML elements for TimeSpan and DateTime. This is what is a shape meant for.

[jeffolmstead]

@Skrypt I will check to see what HTML elements are returned, I almost thought they just returned a "string" but will confirm. I am having a trouble with the latest "Dev" branch in my visual studio, I think it needs updated to get the .NET 5 SDK from what I can tell so will have to do that after hours.

[jeffolmstead]

I see this is on my Github assigments and thought I had better comment. I have been swamped and don't know when I will get back to this. If someone wants to take my code I have started and add in requested change to close it out, please feel free to do so. Sorry, wish I could have wrapped it up already...

[Skrypt]

I don't remember the details of that one but I remember that @agriffard worked on something that generates a localized datetime with a javascript component like Github does.

[PBMikeW]

Can we remove this from the documentation until this PR is sorted? It just cost me some time trying to work out why the tag helper was not working in Razor.

Cheers

[Piedone]

I checked out the PR, going over old ones, and this would be quite useful, and having misleading docs is bad. I've also read the discussion, but I don't see why wouldn't we merge it as-is. So, unless anybody has any objection, that's what I'll do in a week from now.

[Skrypt]

Ok, but I still think we should render a <time /> html tag for these optionally. The liquid tags should also do the same. We should try to never render a DateTime or Timespan without a <time /> tag in the HTML and it seems more appropriate to do it from the TagHelper / Liquid Filter.

[Skrypt]

Approved as I created a second step issue here : #15071

[Piedone]

A follow-up with a fix for the general shapes are the way to go, thanks.