Maui icon indicating copy to clipboard operation
Maui copied to clipboard

[Bug] NumericValidationBehavior throws ArgumentNullException when Text is not set or bound to null value

Open jBijsterboschNL opened this issue 3 years ago • 4 comments

Description

When attaching the NumericValidationBehavior to an Entry element without setting the Entry's Text property, or when binding it to a Property that is (still) null, the NumericValidationBehavior throws an ArgumentNullException. Where it should properly validate the Entry instead. For example, the EmailValidationBehavior checks if value is null or empty string and then returns false instead of throwing an exception. This behavior should be the same through all validation behaviors.

Steps to Reproduce

  1. Add an Entry element to your Page. Don't set the Text property (so it remains null), or bind it to a string property that is set to null.
  2. Add the NumericValidationBehavior to that Entry element.
  3. Run the code and an ArgumentNullException will be thrown.

Expected Behavior

No ArgumentNullException should be thrown, instead false should be returned (as the value is not a proper numeric value).

Actual Behavior

An ArgumentNullException is thrown.

Basic Information

  • Version with issue: 1.2.0
  • Last known good version: N/A
  • IDE: Visual Studio 2022 17.4 preview 1
  • Platform Target Frameworks:
    • iOS: N/A
    • Android: N/A
    • UWP: N/A
  • Nuget Packages: None
  • Affected Devices: N/A

Workaround

Setting the Text property to an empty string in Xaml or through a binding property.

jBijsterboschNL avatar Aug 11 '22 07:08 jBijsterboschNL

@jBijsterboschNL thank you for this report and the associated PR. We have a slightly related discussion going on that covers whether we should be throwing exceptions and whether it should be only forced when nullable reference types are being used.

So to give a heads up we might not rush to get this PR in but I wanted to let you know we are considering this change and how it might need to be spread out to effect more parts of the toolkit.

bijington avatar Aug 11 '22 12:08 bijington

Hi @jBijsterboschNL! Are you blocked by this?

Workaround

A work-around is to set Text to string.Empty:

<Entry Keyboard="Numeric" Text={Binding UserInput}>
    <Entry.Behaviors>
        <toolkit:NumericValidationBehavior 
            InvalidStyle="{StaticResource InvalidEntryStyle}"
            ValidStyle="{StaticResource ValidEntryStyle}"
            Flags="ValidateOnValueChanged"
            MinimumValue="1.0"
            MaximumValue="100.0"
            MaximumDecimalPlaces="2" />
    </Entry.Behaviors>
</Entry>
// Ensure the default value is not null
string userInput = string.Empty;

public string UserInput
{
  get => userInput;
  set => SetProperty(ref userInput, value);
}

TheCodeTraveler avatar Aug 11 '22 16:08 TheCodeTraveler

Hi @jBijsterboschNL! Are you blocked by this?

Workaround

A work-around is to set Text to string.Empty:

<Entry Keyboard="Numeric" Text={Binding UserInput}>
    <Entry.Behaviors>
        <toolkit:NumericValidationBehavior 
            InvalidStyle="{StaticResource InvalidEntryStyle}"
            ValidStyle="{StaticResource ValidEntryStyle}"
            Flags="ValidateOnValueChanged"
            MinimumValue="1.0"
            MaximumValue="100.0"
            MaximumDecimalPlaces="2" />
    </Entry.Behaviors>
</Entry>
// Ensure the default value is not null
string userInput = string.Empty;

public string UserInput
{
  get => userInput;
  set => SetProperty(ref userInput, value);
}

Hi Brandon,

Thanks for thinking with me. This is indeed the workaround I've added for now.

jBijsterboschNL avatar Aug 11 '22 16:08 jBijsterboschNL

Ok good!

Yea, this has raised a bigger conversation around how we're handling the default .NET MAUI values in the Toolkit. For example, null is valid and it's the default value for Entry.Text, so shouldn't the Toolkit allow/handle default values accordingly? We'd prefer to avoid null since .NET is incrementally moving away from null with Nullable Reference Types, but at the same time it does make sense for us to flow-through .NET MAUI's implementation into the Toolkit.

Long story short, this discussion may go on for a while as we get more feedback from the community and I want to ensure that you're not blocked in the meantime!

TheCodeTraveler avatar Aug 11 '22 16:08 TheCodeTraveler