[DatePicker] Prevent silently ignoring date formatting failures

[Tyme-Bleyaert]

Pull Request

📖 Description

This pr will prevent the silent ignore of failed formatting, so the user can correct the input when they type 02022000 instead of 02/02/2000.

🎫 Issues

#4495

✅ Checklist

General

• I have added tests for my changes.
• I have tested my changes.
• I have updated the project documentation to reflect my changes.
• I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

• I have added a new component
• I have added Unit Tests for my new component
• I have modified an existing component
• I have validated the Unit Tests for an existing component

[vnbaaij]

Agree with the remark form Denis. Rest is ok

[dvoituron]

😱 I don't think it's a good idea to add a parameter to InputBase. Because that means you have to manage it for all child components. There are apparently more than 15 of them.

[Tyme-Bleyaert]

Every derived component already must deal with parsing errors via:

/// <summary>
/// Parses a string to create an instance of <typeparamref name="TValue"/>. Derived classes can override this to change how
/// <see cref="CurrentValueAsString"/> interprets incoming values.
/// </summary>
/// <param name="value">The string value to be parsed.</param>
/// <param name="result">An instance of <typeparamref name="TValue"/>.</param>
/// <param name="validationErrorMessage">If the value could not be parsed, provides a validation error message.</param>
/// <returns>True if the value could be parsed; otherwise false.</returns>
protected abstract bool TryParseValueFromString(string? value, [MaybeNullWhen(false)] out TValue result, [NotNullWhen(false)] out string? validationErrorMessage);

So the claim “you now have to manage it for all child components” is slightly misleading. They already do, just in a fragmented way.

Meanwhile this parameter:

• is optional
• has a sane default
• does not require overrides
• does not introduce breaking changes

And before this, we got hardcoded text sitting in either FluentInputExtensions.TryParseSelectableValueFromString() .
Or in any derived component where we had the formatting actually implemented.

However, we could implement interfaces such as IParsableComponent and ICultureSensitiveComponent in relevant derived components and move the parsing logic there.
And move the whole parsing part to IParsableComponent
This way we can use simple checks in an extension method eg:

public static bool TryParseValueFromString<TInput, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TValue>(
    this TInput input, string? value,
    [MaybeNullWhen(false)] out TValue result,
    [NotNullWhen(false)] out string? validationErrorMessage) where TInput : FluentInputBase<TValue>, IParsableComponent
{
    try
    {
        // We special-case bool values because BindConverter reserves bool conversion for conditional attributes.
        if ((typeof(TValue) == typeof(bool) && TryConvertToBool(value, out result)) ||
            (typeof(TValue) == typeof(bool?) && TryConvertToNullableBool(value, out result)))
        {
            validationErrorMessage = null;
            return true;
        }

        var culture =
            input is ICultureSensitiveComponent c
                ? c.Culture
                : CultureInfo.CurrentCulture;

        if (BindConverter.TryConvertTo<TValue>(value, culture, out var parsedValue))
        {
            result = parsedValue;
            validationErrorMessage = null;
            return true;
        }

        result = default;
        validationErrorMessage = string.Format(input.ParsingErrorMessage, input.FieldDisplayName);
        return false;
    }
    catch (InvalidOperationException ex)
    {
        throw new InvalidOperationException($"{input.GetType()} does not support the type '{typeof(TValue)}'.", ex);
    }
}

This approach would require additional work and testing. However, since this PR targets v4 and not the upcoming v5, which is where we’re planning bigger changes, I don’t see a strong benefit to implementing it right now.

[vnbaaij]

No, We should not do that for v4. Having a sane default is enough, I think.