Skip to content
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

fix: Missing support of unsigned numerics in FluentNumberField #2882

Open
Eagle3386 opened this issue Oct 29, 2024 · 2 comments
Open

fix: Missing support of unsigned numerics in FluentNumberField #2882

Eagle3386 opened this issue Oct 29, 2024 · 2 comments
Assignees
Labels
community:contribution Issue will/can be addressed by community contribution

Comments

@Eagle3386
Copy link

🐛 Bug Report

I chose "bug" over "feature request" since a component named FluentNumberField should have supported the unsigned equivalents of int, long, sbyte & short right from the start.
Because I can't imagine a reason to not enable developers to limit sliders, steppers, etc. to positive values only (talking mostly age, RGB color & range pickers here, but applies to customer ID fields as well).

On a more personal note, I'm really having a hard time understanding why (C#) developers still prefer int CustomerId over uint CustomerId since such a property never really contains a negative integer.
But I assume it's just like with decimal which - besides scientific calculations - should always be preferred over float & double (AFAIK, even the C# docs explicitly state this).. 😅

💻 Repro or Code Sample

@* // … code left out for brevity… *@
<FluentNumberField HideStep="true" Immediate="true" Label="Customer #:" Max="999999" MaxLength="6"
                   Min="100000" MinLength="6" Required="true" @bind-Value="Account.CustomerId" />
@* // … code left out for brevity… *@
@code
{
  private AccountData Account { get; } = new();
  // … code left out for brevity…
  public record AccountData
  {
    internal uint? CustomerId { get; set; }
    // … code left out for brevity…
  }
}

🤔 Expected Behavior

As I explicitly declared HideStep="true" above, the following method shouldn't even get called.
However, if it's necessary for some reason, this code inside FluentNumberField.razor.cs:

private static string GetStepAttributeValue()
{
    // Unwrap Nullable<T>, because InputBase already deals with the Nullable aspect
    // of it for us. We will only get asked to parse the T for nonempty inputs.
    var targetType = Nullable.GetUnderlyingType(typeof(TValue)) ?? typeof(TValue);
    if (targetType == typeof(sbyte) ||
        targetType == typeof(int) || typeof(by)
        targetType == typeof(long) ||
        targetType == typeof(short) ||
        targetType == typeof(float) ||
        targetType == typeof(double) ||
        targetType == typeof(decimal))
    {
        return "1";
    }
    else
    {
        throw new InvalidOperationException($"The type '{targetType}' is not a supported numeric type.");
    }
}

should include the 4 unsigned numerics, too.

😯 Current Behavior

A InvalidOperationException is thrown:

System.InvalidOperationException: The type 'System.UInt32' is not a supported numeric type.
   at FluentNumberField`1[UInt32?].GetStepAttributeValue() in /_/src/Core/Components/NumberField/FluentNumberField.razor.cs:line 116
   at FluentNumberField`1[UInt32?]..cctor() in /_/src/Core/Components/NumberField/FluentNumberField.razor.cs:line 97

💁 Possible Solution

I'd like to do a PR to add those 4 types, if the maintainers would agree with me. 😉

🔦 Context

Provide my app's users with a couple of numeric input fields which are completely preventing any negative value.

On a side note, the XML Doc for Max & Min should clarify if Maxlength & Minlength are needed anyway or if the former already take care of that.

🌍 Your Environment

  • OS & Device: Windows 11 Pro 23H2
  • Browser: Firefox Dev Edition
  • .NET & Fluent UI Blazor library: 8.0.10 & 4.10.2
@microsoft-github-policy-service microsoft-github-policy-service bot added the triage New issue. Needs to be looked at label Oct 29, 2024
@vnbaaij
Copy link
Collaborator

vnbaaij commented Oct 29, 2024

I'd like to do a PR to add those 4 types, if the maintainers would agree with me. 😉

Please go for it! Just as a heads up, I did an initial quick try a while back and quickly ran into some issues...Don't say you were not warned!! (😂)

@vnbaaij vnbaaij added community:contribution Issue will/can be addressed by community contribution and removed triage New issue. Needs to be looked at labels Oct 29, 2024
@vnbaaij vnbaaij assigned vnbaaij and Eagle3386 and unassigned vnbaaij Oct 29, 2024
@davhdavh
Copy link

https://learn.microsoft.com/en-us/dotnet/api/system.numerics.iadditiveidentity-2?view=net-8.0

Isn't it more appropriate to use the defined interface that tells us if the type has the concept of a step rather than hardcoding the list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community:contribution Issue will/can be addressed by community contribution
Projects
None yet
Development

No branches or pull requests

3 participants