Skip to content

Support for additional date/time input types in <InputDate> and support for DateOnly/TimeOnly bindings. #34594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 23, 2021

Conversation

MackinnonBuck
Copy link
Member

@MackinnonBuck MackinnonBuck commented Jul 21, 2021

Support for additional date/time input types in <InputDate> and support for DateOnly/TimeOnly bindings.

These changes add support for month, datetime-local, and time input types for the <InputDate> component. Binding support for DateOnly and TimeOnly has also been added.

PR Description
Before these changes, <input type="datetime-local" ...> could not bind to a DateTime without specifying a @bind-format. This is no longer a requirement for use.

Developers can also bind to the value of date/time inputs using the new DateOnly and TimeOnly types. Currently, this works by first parsing as a DateTime using the appropriate format, then converting to a DateOnly or TimeOnly. This allows someone to bind a DateOnly when using datetime-local, for instance, if they wish to ignore time information. If this is not a scenario we want to support, we could change this behavior.

One can also now specify a type attribute for <InputDate> if they wish to use a date/time input type other than "date". This keeps the API surface small, but we could alternatively introduce an InputDateType enum parameter to <InputDate> if that makes the feature more visible to developers.

Validation

  • Unit tests for new bind parsers/formatters.
  • E2E tests for DateOnly/TimeOnly bindings for date/datetime-local/month/time inputs.
  • E2E tests for <InputDate> improvements

Addresses #12376

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jul 21, 2021
@MackinnonBuck MackinnonBuck marked this pull request as ready for review July 21, 2021 22:56
@MackinnonBuck MackinnonBuck requested review from Pilchie and a team as code owners July 21, 2021 22:56
@MackinnonBuck
Copy link
Member Author

MackinnonBuck commented Jul 22, 2021

Something else to consider is whether we want to change the default parsing error message when the type attribute in <InputDate> is something other than "date".

For, example, in the case of type="time", it could be "The _ field must be a date time."

@SteveSandersonMS
Copy link
Member

This is looking great! Superb job with all the tests too.

@MackinnonBuck MackinnonBuck merged commit 3eb0fa8 into main Jul 23, 2021
@MackinnonBuck MackinnonBuck deleted the t-mbuck/date-time-improvements branch July 23, 2021 16:10
@ghost ghost added this to the 6.0-rc1 milestone Jul 23, 2021
@MarvinKlein1508
Copy link

@MackinnonBuck
as I mentioned in #34624 there might be an issue when you try to validate datetime-local and time with CultureInfo.InvariantCulture

Validation has worked in my case until 12:00 for time. In Europe time is defined up to 23:59

23:00 is equivalent to 11PM.

I would suggest to check if your implementation works with 24 hour time format.

@ghost
Copy link

ghost commented Jul 27, 2021

Hi @MarvinKlein1508. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@MackinnonBuck
Copy link
Member Author

@MarvinKlein1508 Could you please clarify this a little?

Date and time HTML elements (including datetime-local and time) provide a culture-invariant format via the HTML DOM API regardless of the locale of the user's browser. I have tested <InputDate> with a browser locale with a 24-hour time format to verify this.

What issues with validation were you experiencing and how did you produce them?

Cheers.

@ghost
Copy link

ghost commented Jul 27, 2021

Hi @MackinnonBuck. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@MarvinKlein1508
Copy link

@MackinnonBuck

sure. I created a small video for you which shows you what I mean. This is using the approach I mentioned in #34624

You can see it here: https://www.youtube.com/watch?v=y5DUWSMJVAA

As you can see the input gets the invalid class when entering a value above 12:59 for time.

I just want to let you know about this. Since your solution is a different approach I thought you might have the same issue.

The component I used can be recreated with this code:

/// <summary>
/// An input component for editing date values.
/// Supported types are <see cref="DateTime"/> and <see cref="DateTimeOffset"/>.
/// </summary>
public class InputDate<TValue> : InputBase<TValue>
{
    /// <summary>
    /// Gets or sets the error message used when displaying an a parsing error.
    /// </summary>
    [Parameter] public string ParsingErrorMessage { get; set; } = "The {0} field must be a date.";
    
    /// <summary>
    /// Gets or sets the <see cref="InputDateFormat"/> for the input.
    /// </summary>
    [Parameter] public InputDateFormat Format { get; set; } = InputDateFormat.Date;

    /// <summary>
    /// Gets or sets the associated <see cref="ElementReference"/>.
    /// <para>
    /// May be <see langword="null"/> if accessed before the component is rendered.
    /// </para>
    /// </summary>
    [DisallowNull] public ElementReference? Element { get; protected set; }

    /// <summary>
    /// Gets the corresponding input type for the provided <see cref="Format"/>
    /// </summary>
    protected string InputType => Format switch
    {
        InputDateFormat.Date => "date",
        InputDateFormat.DateTimeLocal => "datetime-local",
        InputDateFormat.Time => "time",
        _ => throw new InvalidOperationException($"The format '{Format} is not supported'")
    };

    /// <summary>
    /// Gets the corresponding input format for the provided <see cref="Format"/>
    /// </summary>
    protected string InputFormat => Format switch
    {
        InputDateFormat.Date => "yyyy-MM-dd",
        InputDateFormat.DateTimeLocal => "yyyy-MM-ddThh:mm",
        InputDateFormat.Time => "hh:mm:ss",
        _ => throw new InvalidOperationException($"The format '{Format} is not supported'")
    };

    /// <inheritdoc />
    protected override void BuildRenderTree(RenderTreeBuilder builder)
    {
        builder.OpenElement(0, "input");
        builder.AddMultipleAttributes(1, AdditionalAttributes);
        builder.AddAttribute(2, "type", InputType);
        builder.AddAttribute(3, "class", CssClass);
        builder.AddAttribute(4, "value", BindConverter.FormatValue(CurrentValueAsString));
        builder.AddAttribute(5, "onchange", EventCallback.Factory.CreateBinder<string?>(this, __value => CurrentValueAsString = __value, CurrentValueAsString));
        builder.AddElementReferenceCapture(6, __inputReference => Element = __inputReference);
        builder.CloseElement();
    }

    /// <inheritdoc />
    protected override string FormatValueAsString(TValue? value)
    {
        switch (value)
        {
            case DateTime dateTimeValue:
                return BindConverter.FormatValue(dateTimeValue, InputFormat, CultureInfo.InvariantCulture);
            case DateTimeOffset dateTimeOffsetValue:
                return BindConverter.FormatValue(dateTimeOffsetValue, InputFormat, CultureInfo.InvariantCulture);
            default:
                return string.Empty; // Handles null for Nullable<DateTime>, etc.
        }
    }

    /// <inheritdoc />
    protected override bool TryParseValueFromString(string? value, [MaybeNullWhen(false)] out TValue result, [NotNullWhen(false)] out string? validationErrorMessage)
    {
        // Unwrap nullable types. We don't have to deal with receiving empty values for nullable
        // types here, because the underlying InputBase already covers that.
        var targetType = Nullable.GetUnderlyingType(typeof(TValue)) ?? typeof(TValue);

        bool success;
        if (targetType == typeof(DateTime))
        {
            success = TryParseDateTime(value, InputFormat, out result);
        }
        else if (targetType == typeof(DateTimeOffset))
        {
            success = TryParseDateTimeOffset(value, InputFormat, out result);
        }
        else
        {
            throw new InvalidOperationException($"The type '{targetType}' is not a supported date type.");
        }

        if (success)
        {
            Debug.Assert(result != null);
            validationErrorMessage = null;
            return true;
        }
        else
        {
            validationErrorMessage = string.Format(CultureInfo.InvariantCulture, ParsingErrorMessage, DisplayName ?? FieldIdentifier.FieldName);
            return false;
        }
    }

    private static bool TryParseDateTime(string? value, string format, [MaybeNullWhen(false)] out TValue result)
    {
        var success = BindConverter.TryConvertToDateTime(value, CultureInfo.InvariantCulture, format, out var parsedValue);
        if (success)
        {
            result = (TValue)(object)parsedValue;
            return true;
        }
        else
        {
            result = default;
            return false;
        }
    }

    private static bool TryParseDateTimeOffset(string? value, string format, [MaybeNullWhen(false)] out TValue result)
    {
        var success = BindConverter.TryConvertToDateTimeOffset(value, CultureInfo.InvariantCulture, format, out var parsedValue);
        if (success)
        {
            result = (TValue)(object)parsedValue;
            return true;
        }
        else
        {
            result = default;
            return false;
        }
    }
}
/// <summary>
/// Defines the types of an <see cref="InputDate{TValue}"/> component.
/// </summary>
public enum InputDateFormat
{
    Date,
    DateTimeLocal,
    Time
}

I have the same behaviour in all of my browsers. I am running Windows 10 in German with a German version of Firefox 90.

@ghost
Copy link

ghost commented Jul 27, 2021

Hi @MarvinKlein1508. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@MackinnonBuck
Copy link
Member Author

Thank you for the detailed response, @MarvinKlein1508!

I have replicated your test and it appears to work fine with the implementation in this PR.

2021-07-27.10-29-36.mp4

Thanks again for your help 😃

@ghost
Copy link

ghost commented Jul 27, 2021

Hi @MackinnonBuck. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants