Input tag helper for DateTime should use "datetime-local" instead of deprecated "datetime"

[Bartmax]

I'm not an expert, but <input> tag helper for model with DateTime type renders as type="datetime" but it look like that was deprecated in favor of type="datetime-local"

current tag helper:

<input type="datetime" id="Date" name="Date" value="">

it should be:

<input type="datetime-local" id="Date" name="Date" value="">

[dougbu]

Neither type="datetime" nor type="datetime-local" are mentioned in the HTML 5 specification. The <input> tag helper uses these types for consistency with @Html.EditorFor(). That, in turn, uses these types for compatibility with previous versions of MVC.

[Bartmax]

That would be ok but "datetime" doesn't work on any "current" browser but "datetime-local" does.

[Bartmax]

so?

[dougbu]

so?

Triage members will decide whether to break compatibility w/ 1.0.0 and, if yes, in which release.

In the meantime, does type="datetime" behave differently than type="text" in your experience? If this is primarily a spec-compliance issue, it would seem less urgent.

Separately, we can't choose type="datetime-local" everywhere type="datetime" is used today. The intended semantics and default display formats are different. I recommend we remove code that uses datetime entirely. But there may be a few cases where datetime-local works.

[Bartmax]

fair enough. In my experience I'm using type="datetime-local" on the html (so I override it). Without it it breaks datetime function-ability (aka same as using type="text"), but browser (chrome) complain with an error/warning.

[dougbu]

/cc @danroth27

[ctolkien]

Neither type="datetime" nor type="datetime-local" are mentioned in the HTML 5 specification.

Depends which specs you read :)

datetime-local is here:
https://w3c.github.io/html/sec-forms.html#element-attrdef-input-type

And also in the WHATWG specs:
https://html.spec.whatwg.org/multipage/forms.html#attr-input-type

Here's the issue where type="datetime" was removed from the spec:
whatwg/html#336

Further to this, Edge, Chrome, Android and IOS Safari support UI for datetime-local they do not for datetime.

[Eilon]

It's unfortunate we didn't see this before shipping 1.0.0.

I believe it would be a breaking change to change System.DateTime to render as <input type="datetime-local" ... /> instead of <input type="datetime" ... />, so a change like that would have to wait for 2.0.0.

We could in the meantime perhaps have it render as <input type="text" ... />, which would effectively just be an HTML rendering change, because apparently no browsers support datetime anyway (which is supposed to mean that they render it as the default of text).

Would that be a useful change? Who would it help? Perhaps it would help pass the W3C validator? (Not sure what checks it does on this.)

[Bartmax]

Would that be a useful change? Who would it help? Perhaps it would help pass the W3C validator? (Not sure what checks it does on this.)

At least it won't throw warnings on browsers and those who using it will clearly see that 'it's not supported' rather than it's supported something deprecated and had to research about it.

Not a huge difference, but it may help.

@Eilon

It's unfortunate we didn't see this before shipping 1.0.0.

Not trying to be mean, I reported this at June way before RTM ships, what could I done differently? I had no confidence on showing the specs @ctolkien shown. There are so many specs that I wasn't sure if they were valid or not, that spec was available at the time of the issue was created.

If there's anything I can do better when creating an issue i would really appreciate the feedback.

[Eilon]

@Bartmax the product was released on June 27th, just 12 days after this report. At that time the product was in escrow so we were taking only the most critical fixes (e.g. bad crashes, data loss, security issues, etc.). Unfortunately due to the timing, I don't think there was anything better anyone could have done.

But anyway, I think changing it to type="text" is fine for now. Perhaps we can change it to type="datetime-local" in 2.0.0 because it would be a breaking change.

[Bartmax]

@Eilon ah of course, forgot about the escrow thing. makes sense. Thanks for the feedback.

In my current project, where I'm using tag helpers, I'm overriding it as type="datetime-local" on the html which is very easy and fair workaround. Maybe that can be added in the corresponding documentation as a side note/workaround.

In the future I will try to discover things earlier 😄

[ctolkien]

@Bartmax, FYI there is an overload for the DataType attribute to provide a "custom data type". We're using this:

[DataType("datetime-local")]
public DateTime? StartDate { get; set; }

Along with the typical tag helper syntax:

<input asp-for="StartDate" />

To render:

<input type="datetime-local" id="StartDate" name="StartDate" value="">

You probably could write your own tag helper to come in over the top after the regular input tag helper has run to change the type. I might look at doing that in the next day or so.

@Eilon I'd actually prefer this is kept as datetime, rather than going back to text. This is consistent with MVC5, and it allows us to easier target via polyfill anyway (input[type=datetime]). Also, less churn if this is intended to change in v2. All browsers will render datetime the same as text.

I haven't been able to repro Chrome issuing a warning against it's use (v51 or Canary), but it may be in some setting I'm unaware of.

[Bartmax]

@ctolkien
That is indeed a better workaround. If that can be set globally then it would be even better.

[ctolkien]

@Bartmax super naive implementation:

//snipped crappy implementation that you should not use. See updated version below.

Works on my machine, probably slow as all hell and filled with bugs, etc. etc. yadda yadda...

Edit: Actually doesn't work if attribute.Value is a HtmlString! But you get the idea...

[ctolkien]

Less stringy version:

[HtmlTargetElement("input", Attributes = ForAttributeName, TagStructure = TagStructure.WithoutEndTag)]
public class DateTimeTagHelper : TagHelper
{
    private const string ForAttributeName = "asp-for";
    //https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs#L79
    //default order of the built in TagHelpers are -1000; being one higher, this will run after them.
    public override int Order { get; } = -999;

    [HtmlAttributeName(ForAttributeName)]
    public ModelExpression For { get; set; }

    public override void Process(TagHelperContext context, TagHelperOutput output)
    {
        var modelExplorer = For.ModelExplorer;
        if (!string.IsNullOrEmpty(modelExplorer.Metadata.TemplateHint) ||
            !string.IsNullOrEmpty(modelExplorer.Metadata.DataTypeName))
        {
            return;
        }

        var fieldType = modelExplorer.Metadata.UnderlyingOrModelType;
        if (typeof(DateTime) == fieldType)
        {
            output.Attributes.SetAttribute("type", "datetime-local");
        }

    }
}

[Eilon]

Moving this to 2.0.0 and marking as a breaking change. In 2.0.0 we'll change this all to be datetime-local.

[Bartmax]

@Eilon it's currently datetime I think you mean it will change this all to be datetime-local 😄

[Eilon]

Ah sorry, will update 😄

[dougbu]

Fix updated in 7e4a8fe. Now use type="text" and not type="datetime-local" for DateTimeOffset values.