Auto-select `type="text"` for `DateTimeOffset` values #6758

dougbu · 2017-09-03T18:38:02Z

Input fields don't emit (and thus round-trip) offset value when binding against DateTimeOffset fields #6648
a different take on Input tag helper for DateTime should use "datetime-local" instead of deprecated "datetime" #4871
DateTime can round-trip DateTimeKind.UTC with [DataType("datetimeoffset")] or [UIHint("datetimeoffset")]
since they're now handled differently by default, add more DateTime tests
expand tests involving Html5DateRenderingMode.CurrentCulture

Make VS-suggested changes to files updated in this PR

- #6648 - a different take on #4871 - `DateTime` can round-trip `DateTimeKind.UTC` with `[DataType("datetimeoffset")]` or `[UIHint("datetimeoffset")]` - since they're now handled differently by default, add more `DateTime` tests - expand tests involving `Html5DateRenderingMode.CurrentCulture`

dougbu · 2017-09-03T18:51:16Z

/cc @Eilon, @DamianEdwards for 2.0.x patch consideration.

This fix breaks one of the workarounds I described in #6648 (here in particular) but only if a developer uses the <input asp-for="ADateTimeOffset"/><select ...>...</select> approach and relies on auto-selection. In that case, adding type="datetime-offset" to the tag helper, adding [DataType("datetime-local")] to the property, or other kill-the-auto-select changes restore the browser's wizard.

dougbu · 2017-09-03T18:54:15Z

Working on failures. A few expected strings are obviously set for my locale ☹️

dougbu · 2017-09-03T19:20:46Z

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs

@@ -347,10 +346,10 @@ public static IHtmlContent EmailAddressInputTemplate(IHtmlHelper htmlHelper)
            return GenerateTextBox(htmlHelper, inputType: "email");
        }

-        public static IHtmlContent DateTimeInputTemplate(IHtmlHelper htmlHelper)
+        public static IHtmlContent DateTimeOffsetTemplate(IHtmlHelper htmlHelper)


FYI the DateTimeInputTemplate(...) method was un-referenced.

dougbu · 2017-09-03T19:23:30Z

src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs

+                {
+                    // Auto-select a format that round-trips Offset and sub-Second values in a DateTimeOffset. Not
+                    // done if user chose the "text" type in .cshtml file or with data annotations i.e. when
+                    // inputTypeHint==null or "text".


Avoiding auto-selection of the RFC3339 format if .cshtml file contains <input asp-for="ADateTimeOffset" type="text"/> is why this block is a bit more complicated.

dougbu · 2017-09-03T19:25:30Z

test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/InputTagHelperTest.cs

@@ -457,6 +457,77 @@ public void Process_GeneratesFormattedOutput(string specifiedType, string expect
            Assert.Equal(expectedTagName, output.TagName);
        }

+        [Theory]
+        [InlineData("datetime", "datetime")]
+        [InlineData(null, "datetime-local")]


FYI this demonstrates the default is still datetime-local for DateTime properties.

dougbu · 2017-09-03T19:27:46Z

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs

@@ -786,7 +788,7 @@ public void Editor_InputTypeDateTime_RendersAsDateTime()
                hour: 3,
                minute: 4,
                second: 5,
-                millisecond: 6,
+                millisecond: 60,


FYI I made a few of these changes to make the DateTime and DateTimeOffset values consistent.

dougbu · 2017-09-03T19:29:27Z

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs

+        [InlineData("date", "{0:d}", "2000-01-02", "date")]
+        [InlineData("datetime", null, "2000-01-02T03:04:05.060", "datetime-local")]
+        [InlineData("datetime-local", null, "2000-01-02T03:04:05.060", "datetime-local")]
+        [InlineData("DateTimeOffset", "{0:o}", "2000-01-02T03:04:05.060Z", "text")]


FYI this demonstrates user can opt into new type and format combination for DateTime properties.

dougbu · 2017-09-05T23:09:21Z

7e4a8fe
(thanks @pranavkm)

dougbu added 2 commits September 2, 2017 23:01

Make VS-suggested changes to files updated in this PR

cf5e66c

dnfclas added the cla-already-signed label Sep 3, 2017

dougbu requested review from ajaybhargavb and pranavkm September 3, 2017 18:38

Add a couple [ReplaceCulture] attributes

80d60dc

dougbu commented Sep 3, 2017

View reviewed changes

pranavkm approved these changes Sep 5, 2017

View reviewed changes

dougbu closed this Sep 5, 2017

dougbu deleted the dougbu/keep.offset.6648 branch September 5, 2017 23:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-select `type="text"` for `DateTimeOffset` values #6758

Auto-select `type="text"` for `DateTimeOffset` values #6758

dougbu commented Sep 3, 2017

dougbu commented Sep 3, 2017

dougbu commented Sep 3, 2017

dougbu Sep 3, 2017 •

edited

Loading

dougbu Sep 3, 2017

dougbu Sep 3, 2017

dougbu Sep 3, 2017

dougbu Sep 3, 2017

dougbu commented Sep 5, 2017

Auto-select type="text" for DateTimeOffset values #6758

Auto-select type="text" for DateTimeOffset values #6758

Conversation

dougbu commented Sep 3, 2017

dougbu commented Sep 3, 2017

dougbu commented Sep 3, 2017

dougbu Sep 3, 2017 • edited Loading

Choose a reason for hiding this comment

dougbu Sep 3, 2017

Choose a reason for hiding this comment

dougbu Sep 3, 2017

Choose a reason for hiding this comment

dougbu Sep 3, 2017

Choose a reason for hiding this comment

dougbu Sep 3, 2017

Choose a reason for hiding this comment

dougbu commented Sep 5, 2017

Auto-select `type="text"` for `DateTimeOffset` values #6758

Auto-select `type="text"` for `DateTimeOffset` values #6758

dougbu Sep 3, 2017 •

edited

Loading