Radio Button Null Selection Fix (#31175)

* Added UI test for null selectin bug

* Handle when group update changes to no values

* Added unit test

* new file to file scoped namespace

* Ignore child added if layout is null

* Piggy backed off of existing UI sample

* removing unused using

* reverting whitespace

* removing unused using

* Have RadioButton default to itself for uniqueness

* Added comment justifying madness

* less hacky way

* Less hacky and added unit test

---------

Co-authored-by: Evin Ballantyne <[email protected]>

Author: 2 people

src/Controls/samples/Controls.Sample/Pages/Controls/RadioButtonGalleries/RadioButtonGroupBindingGallery.xaml

@@ -18,6 +18,7 @@
                     <RowDefinition></RowDefinition>
                     <RowDefinition></RowDefinition>
                     <RowDefinition></RowDefinition>
+                    <RowDefinition></RowDefinition>
                 </Grid.RowDefinitions>
 
                 <Grid.ColumnDefinitions>
@@ -28,15 +29,16 @@
                 <Label Grid.ColumnSpan="2" Text="The RadioButtons in this Grid have a GroupName and Selection bound to a ViewModel."></Label>
 
                 <Label Text="{Binding GroupName, StringFormat='The GroupName is {0}'}" Grid.Row="1" />
-                <Label Text="{Binding Selection, StringFormat='The Selection is {0}'}" Grid.Row="1" Grid.Column="1" />
+                <Label Text="{Binding Selection, StringFormat='The Selection is {0}', TargetNullValue='The Selection is (null)'}" Grid.Row="1" Grid.Column="1" />
 
                 <RadioButton Content="Option A" Value="A" Grid.Row="2"></RadioButton>
                 <RadioButton Content="Option B" Value="B" Grid.Row="2" Grid.Column="1"></RadioButton>
                 <RadioButton Content="Option C" Value="C" Grid.Row="3"></RadioButton>
                 <RadioButton Content="Option D" Value="D" Grid.Row="3" Grid.Column="1"></RadioButton>
 
-                <Button Margin="5" Grid.ColumnSpan="2" Grid.Row="4" Text="Set selection in view model to 'B'" Clicked="Button_Clicked"></Button>
-            </Grid>
+                <Button Margin="5" Grid.ColumnSpan="2" Grid.Row="4" Text="Set selection in view model to 'B'" Clicked="Set_Button_Clicked"></Button>
+                <Button Margin="5" Grid.ColumnSpan="2" Grid.Row="5" Text="Clear selection in view model to 'null'" Clicked="Clear_Button_Clicked"></Button>
+      </Grid>
         </StackLayout>
     </ContentPage.Content>
 </ContentPage>

src/Controls/samples/Controls.Sample/Pages/Controls/RadioButtonGalleries/RadioButtonGroupBindingGallery.xaml.cs

@@ -15,10 +15,15 @@ public RadioButtonGroupBindingGallery()
 			BindingContext = _viewModel;
 		}
 
-		private void Button_Clicked(object sender, System.EventArgs e)
+		private void Set_Button_Clicked(object sender, System.EventArgs e)
 		{
 			_viewModel.Selection = "B";
 		}
+
+		private void Clear_Button_Clicked(object sender, System.EventArgs e)
+		{
+			_viewModel.Selection = null;
+		}
 	}
 
 	public class RadioButtonGroupBindingModel : INotifyPropertyChanged

src/Controls/src/Core/RadioButton/RadioButton.cs

@@ -59,7 +59,8 @@ public partial class RadioButton : TemplatedView, IElementConfiguration<RadioBut
 		/// <summary>Bindable property for <see cref="Value"/>.</summary>
 		public static readonly BindableProperty ValueProperty =
 			BindableProperty.Create(nameof(Value), typeof(object), typeof(RadioButton), null,
-			propertyChanged: (b, o, n) => ((RadioButton)b).OnValuePropertyChanged());
+			propertyChanged: (b, o, n) => ((RadioButton)b).OnValuePropertyChanged(),
+			coerceValue: (b,  o) => o ?? b);
 
 		/// <summary>Bindable property for <see cref="IsChecked"/>.</summary>
 		public static readonly BindableProperty IsCheckedProperty = BindableProperty.Create(
@@ -212,6 +213,9 @@ public RadioButton()
 		{
 			_platformConfigurationRegistry = new Lazy<PlatformConfigurationRegistry<RadioButton>>(() =>
 				new PlatformConfigurationRegistry<RadioButton>(this));
+
+			//initialize Value to prevent null value
+			Value = this;
 		}
 
 		/// <inheritdoc/>
@@ -433,12 +437,17 @@ void HandleRadioButtonGroupSelectionChanged(RadioButton selected, RadioButtonGro
 
 		void HandleRadioButtonGroupValueChanged(Element layout, RadioButtonGroupValueChanged args)
 		{
-			if (IsChecked || string.IsNullOrEmpty(GroupName) || GroupName != args.GroupName || !object.Equals(Value, args.Value) || !MatchesScope(args))
+			if (string.IsNullOrEmpty(GroupName) || GroupName != args.GroupName || !MatchesScope(args))
 			{
 				return;
 			}
 
-			SetValue(IsCheckedProperty, true, specificity: SetterSpecificity.FromHandler);
+			var isValueMatching = object.Equals(Value, args.Value);
+
+			if (IsChecked != isValueMatching)
+			{
+				SetValue(IsCheckedProperty, isValueMatching, specificity: SetterSpecificity.FromHandler);
+			}
 		}
 
 		static View BuildDefaultTemplate()

src/Controls/src/Core/RadioButton/RadioButtonGroupController.cs

@@ -74,7 +74,7 @@ void HandleRadioButtonValueChanged(RadioButton radioButton, RadioButtonValueChan
 
 		void ChildAdded(object sender, ElementEventArgs e)
 		{
-			if (string.IsNullOrEmpty(_groupName))
+			if (string.IsNullOrEmpty(_groupName) || _layout == null)
 			{
 				return;
 			}
@@ -135,15 +135,18 @@ void UpdateGroupNames(Element element, string name, string oldName = null)
 
 		void SetSelectedValue(object radioButtonValue)
 		{
+			if(object.Equals(_selectedValue, radioButtonValue))
+			{
+				return;
+			}
+
 			_selectedValue = radioButtonValue;
 
-			if (radioButtonValue != null)
-			{
 #pragma warning disable CS0618 // TODO: Remove when we internalize/replace MessagingCenter
 				MessagingCenter.Send<Element, RadioButtonGroupValueChanged>(_layout, RadioButtonGroup.GroupValueChangedMessage,
 					new RadioButtonGroupValueChanged(_groupName, RadioButtonGroup.GetVisualRoot(_layout), radioButtonValue));
 #pragma warning restore CS0618 // Type or member is obsolete
-			}
+
 		}
 
 		void SetGroupName(string groupName)

src/Controls/tests/Core.UnitTests/RadioButtonTests.cs

@@ -236,5 +236,38 @@ public void GroupSelectedValueUpdatesWhenSelectedButtonValueUpdates()
 
 			Assert.Equal("updated", layout.GetValue(RadioButtonGroup.SelectedValueProperty));
 		}
+
+		[Fact]
+		public void GroupNullSelectionClearsAnySelection()
+		{
+			var layout = new Grid();
+			layout.SetValue(RadioButtonGroup.GroupNameProperty, "foo");
+
+			var radioButton1 = new RadioButton() { Value = 1, IsChecked = true };
+			var radioButton2 = new RadioButton() { Value = 2 };
+			var radioButton3 = new RadioButton() { Value = 3 };
+
+			layout.Children.Add(radioButton1);
+			layout.Children.Add(radioButton2);
+			layout.Children.Add(radioButton3);
+
+			Assert.Equal(1, layout.GetValue(RadioButtonGroup.SelectedValueProperty));
+
+			layout.SetValue(RadioButtonGroup.SelectedValueProperty, null);
+
+			Assert.False(radioButton1.IsChecked);
+		}
+
+		[Fact]
+		public void ValuePropertyCoercedToItselfIfSetToNull()
+		{
+			var radioButton = new RadioButton();
+			
+			Assert.Equal(radioButton, radioButton.Value);
+
+			radioButton.Value = null;
+
+			Assert.Equal(radioButton, radioButton.Value);
+		}
 	}
 }