From 37a034505aedf1a2cd559aee7ef0abc01839674f Mon Sep 17 00:00:00 2001 From: David Oliver Date: Thu, 25 Jul 2019 13:57:28 -0400 Subject: [PATCH] TextBox - reset Text when pushing null to bound value Fix regression where a null pushed to Text via binding was being ignored, rather than clearing the Text. --- doc/ReleaseNotes/_ReleaseNotes.md | 1 + .../TextBoxTests/Given_TextBox.cs | 82 +++++++++++++++++-- .../UI/Xaml/Controls/TextBox/TextBox.cs | 11 ++- 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/doc/ReleaseNotes/_ReleaseNotes.md b/doc/ReleaseNotes/_ReleaseNotes.md index 984a4dbbbec3..b5835c14a100 100644 --- a/doc/ReleaseNotes/_ReleaseNotes.md +++ b/doc/ReleaseNotes/_ReleaseNotes.md @@ -37,6 +37,7 @@ * `TextBox` no longer raises TextChanged when its template is applied, in line with UWP. * `TextBox.TextChanged` is now called asynchronously after the UI is updated, in line with UWP. For most uses `TextChanging` should be preferred. * [Android] `TextBox.IsSpellCheckEnabled = false` is now enforced in a way that may cause issues in certain use cases (see https://stackoverflow.com/a/5188119/1902058). The old behavior can be restored by setting `ShouldForceDisableSpellCheck = false`, per `TextBox`. +* `TextBox.Text = null` will now throw an exception, as on UWP. Pushing `null` via a binding is still valid. ### Bug fixes * #1276 retrieving non-existent setting via indexer should not throw and `ApplicationDataContainer` allowed clearing value by calling `Add(null)` which was not consistent with UWP. diff --git a/src/Uno.UI.Tests/Windows_UI_XAML_Controls/TextBoxTests/Given_TextBox.cs b/src/Uno.UI.Tests/Windows_UI_XAML_Controls/TextBoxTests/Given_TextBox.cs index 7c7b1154878f..8defbce1eb97 100644 --- a/src/Uno.UI.Tests/Windows_UI_XAML_Controls/TextBoxTests/Given_TextBox.cs +++ b/src/Uno.UI.Tests/Windows_UI_XAML_Controls/TextBoxTests/Given_TextBox.cs @@ -1,10 +1,14 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Windows.UI.Xaml; using Windows.UI.Xaml.Controls; +using Windows.UI.Xaml.Data; namespace Uno.UI.Tests.TextBoxTests { @@ -40,17 +44,45 @@ public void When_Setting_Text_Null() textBox.Text = "Rhubarb"; Assert.AreEqual("Rhubarb", textBox.Text); Assert.AreEqual(1, callbackCount); - -#if NETFX_CORE + Assert.ThrowsException(() => textBox.Text = null); -#else - textBox.Text = null; -#endif - ; + Assert.AreEqual("Rhubarb", textBox.Text); Assert.AreEqual(1, callbackCount); } + [TestMethod] + public void When_Binding_Set_Null() + { + var textBox = new TextBox(); + var source = new MySource() { SourceText = "Spinach" }; + + textBox.DataContext = source; + textBox.SetBinding(TextBox.TextProperty, new Binding() { Path = new PropertyPath("SourceText") }); + + Assert.AreEqual("Spinach", textBox.Text); + + source.SourceText = null; + + Assert.AreEqual("", textBox.Text); + } + + [TestMethod] + public void When_Binding_Set_Non_String() + { + var textBox = new TextBox(); + var source = new MySource() { SourceInt = 12 }; + + textBox.DataContext = source; + textBox.SetBinding(TextBox.TextProperty, new Binding() { Path = new PropertyPath("SourceInt") }); + + Assert.AreEqual("12", textBox.Text); + + source.SourceInt = 19; + + Assert.AreEqual("19", textBox.Text); + } + [TestMethod] public void When_BeforeTextChanging_Cancel() { @@ -80,5 +112,43 @@ public void When_BeforeTextChanging_Cancel() textBox.Text = "Chirimoya"; Assert.AreEqual(2, beforeTextChangingCount); } + + public class MySource : INotifyPropertyChanged + { + private string _sourceText; + + public string SourceText + { + get => _sourceText; + set + { + if (_sourceText != value) + { + _sourceText = value; + OnPropertyChanged(); + } + } + } + + private int _sourceInt; + + public int SourceInt + { + get => _sourceInt; + set + { + if (_sourceInt != value) + { + _sourceInt = value; + OnPropertyChanged(); + } + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) + => PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } } } diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs index f320ce0ceb50..eea44ddb3388 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs @@ -153,7 +153,14 @@ private DependencyPropertyChangedEventArgs CreateInitialValueChangerEventArgs(De public string Text { get { return (string)this.GetValue(TextProperty); } - set { this.SetValue(TextProperty, value); } + set { + if (value == null) + { + throw new ArgumentNullException(); + } + + this.SetValue(TextProperty, value); + } } public static readonly DependencyProperty TextProperty = @@ -243,7 +250,7 @@ private object CoerceText(object baseValue) { if (!(baseValue is string baseString)) { - return DependencyProperty.UnsetValue; //TODO: UWP throws ArgumentNullException, in principle we should do the same. + return ""; //Pushing null to the binding resets the text. (Setting null to the Text property directly throws an exception.) } var args = new TextBoxBeforeTextChangingEventArgs(baseString);