-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Removed Value property coercion in RadioButton #32489
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
Conversation
Eliminated the coercion logic that set RadioButton.Value to the instance when null, allowing Value to be explicitly set to null. Updated related unit test to reflect the new behavior.
|
Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the automatic coercion of RadioButton.Value from null to the RadioButton instance itself, allowing developers to explicitly set Value to null. This fixes issue #32466 where the previous behavior prevented null values.
Key Changes:
- Removed the
coerceValuecallback and constructor initialization that forced Value to default to the RadioButton instance - Updated the unit test to verify that Value can now be explicitly set to and remain null
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/src/Core/RadioButton/RadioButton.cs | Removed coercion logic from ValueProperty and removed constructor initialization that set Value to this |
| src/Controls/tests/Core.UnitTests/RadioButtonTests.cs | Updated test to verify Value can be explicitly set to null instead of being coerced to the RadioButton instance |
|
/rebase |
|
@asi-evin do you have any comments on this fix? I'm curious about why you'd added this coercion in the first place and wanted to make sure we aren't missing something here. Also @kubaflo :-) I asked our friend copilot about this PR and it found one scenario we might want to consider (in a separate PR) [Fact(Skip = "Known issue: RadioButtonGroupController doesn't check buttons with Value=null when SelectedValue=null. See PR #32489 validation report.")]
public void CanSelectRadioButtonWithNullValue()
{
// This test proves the bug: when you have a RadioButton with Value=null
// and set SelectedValue=null, the button should be checked (like a "Don't know" option)
var layout = new Grid();
layout.SetValue(RadioButtonGroup.GroupNameProperty, "test");
var radioButtonA = new RadioButton() { Value = "A" };
var radioButtonB = new RadioButton() { Value = "B" };
var radioButtonNull = new RadioButton() { Value = null }; // "Don't know" option
layout.Children.Add(radioButtonA);
layout.Children.Add(radioButtonB);
layout.Children.Add(radioButtonNull);
// First, select "A"
layout.SetValue(RadioButtonGroup.SelectedValueProperty, "A");
Assert.True(radioButtonA.IsChecked);
Assert.False(radioButtonB.IsChecked);
Assert.False(radioButtonNull.IsChecked);
// Now select the "Don't know" option by setting SelectedValue to null
layout.SetValue(RadioButtonGroup.SelectedValueProperty, null);
// Expected: radioButtonNull should be checked (because its Value is null)
// Expected: radioButtonA should be unchecked
Assert.False(radioButtonA.IsChecked, "RadioButton A should be unchecked");
Assert.False(radioButtonB.IsChecked, "RadioButton B should remain unchecked");
Assert.True(radioButtonNull.IsChecked, "RadioButton with Value=null should be checked when SelectedValue=null");
} |
|
/backport to release/10.0.1xx-sr1 |
|
Started backporting to |
I'll be able to look at this tomorrow or even the weekend, but I took a breif look at my pull request. I didn't find the exact UI test (not unit test, I was mistaken there), but I think I recall it had something to do with multiple radio buttons in the same group all being NULL. ...Or I'm just making this all up in my head, hopefully I can find out tomorrow. |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [Microsoft.Maui.Controls](https://github.com/dotnet/maui) | nuget | patch | `10.0.10` -> `10.0.11` | --- ### Release Notes <details> <summary>dotnet/maui (Microsoft.Maui.Controls)</summary> ### [`v10.0.11`](https://github.com/dotnet/maui/releases/tag/10.0.11): SR1.1 [Compare Source](dotnet/maui@10.0.10...10.0.11) ##### What's Changed .NET MAUI 10.0.11 introduces significant improvements across all platforms with focus on quality, performance, and developer experience. This release includes 11 commits with various improvements, bug fixes, and enhancements. ##### .NET MAUI Product Fixes ##### Android - Fix content page title clipping on Android API < 30 with window insets compatibility by [@​Copilot](https://github.com/Copilot) in dotnet/maui#32738 <details> <summary>🔧 Fixes</summary> - [Shell content page title position incorrect/clipped](dotnet/maui#32526) </details> ##### Button - \[release/10.0.1xx-sr1] Removed Value property coercion in RadioButton by [@​github-actions](https://github.com/github-actions)\[bot] in dotnet/maui#32604 <details> <summary>🔧 Fixes</summary> - [Removed Value property coercion in RadioButton](dotnet/maui#32489) </details> ##### DateTimePicker - Fix crash when TimePicker.Time is set to null (backport from PR [#​32660](dotnet/maui#32660)) by [@​Copilot](https://github.com/Copilot) in dotnet/maui#32715 <details> <summary>🔧 Fixes</summary> - [Fix crash when TimePicker.Time is set to null](dotnet/maui#32660) </details> ##### Gestures - \[release/10.0.1xx-sr1] predictive back gesture support for Android 13+ by [@​github-actions](https://github.com/github-actions)\[bot] in dotnet/maui#32635 <details> <summary>🔧 Fixes</summary> - [predictive back gesture support for Android 13+](dotnet/maui#32461) </details> ##### Infrastructure - \[release/10.0.1xx-sr1] \[ci] Revert changes setting Creator by [@​github-actions](https://github.com/github-actions)\[bot] in dotnet/maui#32803 <details> <summary>🔧 Fixes</summary> - [\[ci\] Revert changes setting Creator](dotnet/maui#32743) </details> ##### Mediapicker - \[release/10.0.1xx-sr1] \[Android] Refactor selection limit handling in MediaPicker by [@​github-actions](https://github.com/github-actions)\[bot] in dotnet/maui#32628 <details> <summary>🔧 Fixes</summary> - [\[Android\] Refactor selection limit handling in MediaPicker](dotnet/maui#32571) ...
Eliminated the coercion logic that set RadioButton.Value to the instance when null, allowing Value to be explicitly set to null. Updated related unit test to reflect the new behavior.
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 of Change
Eliminated the coercion logic that set RadioButton.Value to the instance when null, allowing Value to be explicitly set to null. Updated related unit test to reflect the new behavior.
Issues Fixed
Fixes #32466