Fix empty string binding to nullable value types#33536
Fix empty string binding to nullable value types#33536StephaneDelcroix merged 3 commits intonet11.0from
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes an issue where clearing an Entry bound to a nullable value type (e.g., int?) did not correctly set the property to null. The fix adds special handling in BindingExpressionHelper.TryConvert() to convert empty/whitespace strings to null when the target type is nullable.
Changes:
- Modified
BindingExpressionHelper.TryConvert()to detect empty/whitespace strings being converted to nullable types and convert them tonull - Added two unit tests verifying the behavior for both nullable and non-nullable int properties when binding with empty strings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/BindingExpressionHelper.cs | Added check to convert empty/whitespace strings to null for nullable value types before attempting type conversion |
| src/Controls/tests/Core.UnitTests/BindingUnitTests.cs | Added tests for empty string binding to int and int? properties, plus view models to support the tests |
| // Handle empty/whitespace string conversion to nullable types | ||
| // Empty string should convert to null for nullable value types | ||
| // See: https://github.com/dotnet/maui/issues/8342 | ||
| if (underlyingType != null && string.IsNullOrWhiteSpace(stringValue)) |
There was a problem hiding this comment.
Using IsNullOrWhiteSpace will convert whitespace-only strings (like " ", "\t", "\n") to null for nullable types. This might not be the intended behavior. Consider using string.IsNullOrEmpty instead to only handle truly empty strings. For example, if a user enters a space in an Entry bound to int?, it would convert to null, which may be unexpected - the conversion should probably fail and retain the previous value, similar to non-nullable types.
| public void TwoWayBindingToNullableIntPropertyWithEmptyStringBecomesNull() | ||
| { | ||
| // When binding to a nullable int, empty string should be converted to null | ||
| var vm = new NullableIntViewModel { IntValue = 123 }; | ||
| var entry = new Entry { BindingContext = vm }; | ||
| entry.SetBinding(Entry.TextProperty, "IntValue", BindingMode.TwoWay); | ||
|
|
||
| // Verify initial binding | ||
| Assert.Equal("123", entry.Text); | ||
|
|
||
| // Clear the entry - for nullable int, empty string should result in null | ||
| entry.SetValueFromRenderer(Entry.TextProperty, ""); | ||
|
|
||
| // Nullable int should become null when empty string is entered | ||
| // Note: Currently this also fails due to the same FormatException | ||
| // but conceptually nullable int should accept null for empty string | ||
| Assert.Null(vm.IntValue); | ||
| } |
There was a problem hiding this comment.
Add test coverage for whitespace strings (e.g., " ", "\t") to verify the behavior when binding to nullable value types. This will help ensure that the use of IsNullOrWhiteSpace in the fix is intentional and handles edge cases correctly.
| public void TwoWayBindingToNullableIntPropertyWithEmptyStringBecomesNull() | ||
| { | ||
| // When binding to a nullable int, empty string should be converted to null | ||
| var vm = new NullableIntViewModel { IntValue = 123 }; | ||
| var entry = new Entry { BindingContext = vm }; | ||
| entry.SetBinding(Entry.TextProperty, "IntValue", BindingMode.TwoWay); | ||
|
|
||
| // Verify initial binding | ||
| Assert.Equal("123", entry.Text); | ||
|
|
||
| // Clear the entry - for nullable int, empty string should result in null | ||
| entry.SetValueFromRenderer(Entry.TextProperty, ""); | ||
|
|
||
| // Nullable int should become null when empty string is entered | ||
| // Note: Currently this also fails due to the same FormatException | ||
| // but conceptually nullable int should accept null for empty string | ||
| Assert.Null(vm.IntValue); | ||
| } |
There was a problem hiding this comment.
The test only covers int?. Consider adding test coverage for other nullable value types (e.g., double?, decimal?, bool?, DateTime?) to ensure the fix works consistently across all nullable value types.
| // Nullable int should become null when empty string is entered | ||
| // Note: Currently this also fails due to the same FormatException | ||
| // but conceptually nullable int should accept null for empty string | ||
| Assert.Null(vm.IntValue); |
There was a problem hiding this comment.
Add assertion to verify what Entry.Text becomes after the nullable int is set to null. The test should verify whether Entry.Text remains "" or updates to represent the null value (likely ""). This ensures the two-way binding behavior is well-defined and tested.
| if (underlyingType != null && string.IsNullOrWhiteSpace(stringValue)) | ||
| { | ||
| value = null!; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
When the input value is not a string type, line 30 converts it to string.Empty, which will then match the IsNullOrWhiteSpace check on line 35. This could incorrectly convert non-string values to null when binding to nullable types. Consider adding a check to ensure value is actually a string before applying this empty string handling, for example: if (underlyingType != null && value is string str && string.IsNullOrWhiteSpace(str))
| public void TwoWayBindingToNullableIntPropertyWithEmptyStringBecomesNull() | ||
| { | ||
| // When binding to a nullable int, empty string should be converted to null | ||
| var vm = new NullableIntViewModel { IntValue = 123 }; | ||
| var entry = new Entry { BindingContext = vm }; | ||
| entry.SetBinding(Entry.TextProperty, "IntValue", BindingMode.TwoWay); | ||
|
|
||
| // Verify initial binding | ||
| Assert.Equal("123", entry.Text); | ||
|
|
||
| // Clear the entry - for nullable int, empty string should result in null | ||
| entry.SetValueFromRenderer(Entry.TextProperty, ""); | ||
|
|
||
| // Nullable int should become null when empty string is entered | ||
| // Note: Currently this also fails due to the same FormatException | ||
| // but conceptually nullable int should accept null for empty string | ||
| Assert.Null(vm.IntValue); | ||
| } |
There was a problem hiding this comment.
Add test coverage to verify that after setting a nullable int to null via empty string, entering a new valid value correctly updates the property. For example, after clearing the entry (setting to null), entering "456" should set IntValue to 456.
457e045 to
91ff0f0
Compare
5f0f6a1 to
6b2b3ba
Compare
When binding Entry.Text to a nullable value type (e.g., int?), clearing the Entry now correctly sets the property to null instead of retaining the previous value. Fixes #8342
- Use string.IsNullOrEmpty instead of IsNullOrWhiteSpace so whitespace strings fail conversion rather than silently becoming null - Add 'value is string' guard to prevent non-string values from being incorrectly converted to null via the string.Empty fallback - Add tests for whitespace retention, double? nullable type, and re-entering a value after clearing
…tation The original fix that converted all null values to empty string for string targets broke existing tests that expect null to remain null. Reverted that change. The test now expects Entry.Text to be null (not "") after the nullable int becomes null, which is correct - Entry displays empty for both null and "" text values.
6b2b3ba to
ecfc2f3
Compare
> [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description When binding `Entry.Text` to a nullable value type property (e.g., `int?`), clearing the Entry now correctly sets the property to `null` instead of retaining the previous value. ## Issue Fixes #8342 ## Root Cause In `BindingExpressionHelper.TryConvert()`, when converting an empty string to a nullable type like `int?`: 1. The underlying type was extracted (`int`) 2. `Convert.ChangeType("", int)` was called, which throws `FormatException` 3. The catch block returned `false`, keeping the old value ## Solution Added a check before `Convert.ChangeType()` to handle empty/whitespace strings when converting to nullable types. When the target type is nullable and the source is an empty/whitespace string, the value is set to `null` and the conversion succeeds. ## Behavior | Target Type | Empty String Input | Result | |-------------|-------------------|--------| | `int?`, `double?`, etc. (nullable) | `""` | `null` ✅ | | `int`, `double`, etc. (non-nullable) | `""` | Conversion fails, retains last valid value (unchanged) | ## Testing Added two unit tests in `BindingUnitTests.cs`: - `TwoWayBindingToIntPropertyWithEmptyStringRetainsLastValidValue` - Verifies non-nullable int behavior - `TwoWayBindingToNullableIntPropertyWithEmptyStringBecomesNull` - Verifies nullable int now converts to null
|
/backport to release/11.0.1xx-preview2 |
|
Started backporting to |
…4260) This PR merges commits from `net11.0` into `release/11.0.1xx-preview2`. Commits include: - Add Circle, Polygon, and Polyline click events for Map control (#29101) - [automated] Merge branch 'main' => 'net11.0' (#34203) - Fix empty string binding to nullable value types (#33536) - Add MapElement.IsVisible and MapElement.ZIndex properties (#33993) - Add MauiXamlHotReload property for IDE communication (#34028) ## Instructions for merging Complete this PR by creating a **merge commit**, *not* a squash or rebase commit.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
When binding
Entry.Textto a nullable value type property (e.g.,int?), clearing the Entry now correctly sets the property tonullinstead of retaining the previous value.Issue
Fixes #8342
Root Cause
In
BindingExpressionHelper.TryConvert(), when converting an empty string to a nullable type likeint?:int)Convert.ChangeType("", int)was called, which throwsFormatExceptionfalse, keeping the old valueSolution
Added a check before
Convert.ChangeType()to handle empty/whitespace strings when converting to nullable types. When the target type is nullable and the source is an empty/whitespace string, the value is set tonulland the conversion succeeds.Behavior
int?,double?, etc. (nullable)""null✅int,double, etc. (non-nullable)""Testing
Added two unit tests in
BindingUnitTests.cs:TwoWayBindingToIntPropertyWithEmptyStringRetainsLastValidValue- Verifies non-nullable int behaviorTwoWayBindingToNullableIntPropertyWithEmptyStringBecomesNull- Verifies nullable int now converts to null