Merge pull request #258 from xamarin/ermau-split-validate-coerce

Validation/Coerce

8 files changed, 69 insertions, 28 deletions

diff --git a/Xamarin.PropertyEditing/ICoerce.cs b/Xamarin.PropertyEditing/ICoerce.cs
new file mode 100644
index 0000000..7723717
--- /dev/null
+++ b/Xamarin.PropertyEditing/ICoerce.cs
@@ -0,0 +1,7 @@
+namespace Xamarin.PropertyEditing
+{
+	public interface ICoerce<T>
+	{
+		T CoerceValue (T value);
+	}
+}
\ No newline at end of file
diff --git a/Xamarin.PropertyEditing/IValidator.cs b/Xamarin.PropertyEditing/IValidator.cs
index 15af468..6856d90 100644
--- a/Xamarin.PropertyEditing/IValidator.cs
+++ b/Xamarin.PropertyEditing/IValidator.cs
@@ -1,7 +1,7 @@
 namespace Xamarin.PropertyEditing
 {
-	public interface IValidator<T>
+	public interface IValidator<in T>
 	{
-		T ValidateValue (T value);
+		bool IsValid (T value);
 	}
-}
\ No newline at end of file
+}
diff --git a/Xamarin.PropertyEditing/ViewModels/CombinablePropertyViewModel.cs b/Xamarin.PropertyEditing/ViewModels/CombinablePropertyViewModel.cs
index 7d26e5f..5942cc2 100644
--- a/Xamarin.PropertyEditing/ViewModels/CombinablePropertyViewModel.cs
+++ b/Xamarin.PropertyEditing/ViewModels/CombinablePropertyViewModel.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.ComponentModel;
 using System.Linq;
 using System.Threading.Tasks;
 
@@ -52,6 +53,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 			if (this.predefinedValues.IsValueCombinable && !this.predefinedValues.IsConstrainedToPredefined)
 				throw new NotSupportedException ("Properties with combinable values can not be unconstrained currently");
 
+			this.coerce = property as ICoerce<IReadOnlyList<TValue>>;
 			this.validator = property as IValidator<IReadOnlyList<TValue>>;
 
 			var choices = new List<FlaggableChoiceViewModel<TValue>> (this.predefinedValues.PredefinedValues.Count);
@@ -115,22 +117,26 @@ namespace Xamarin.PropertyEditing.ViewModels
 
 		private bool fromUpdate;
 		private readonly IValidator<IReadOnlyList<TValue>> validator;
+		private readonly ICoerce<IReadOnlyList<TValue>> coerce;
 		private readonly IHavePredefinedValues<TValue> predefinedValues;
 
-		private async void OnChoiceVmPropertyChanged (object sender, System.ComponentModel.PropertyChangedEventArgs e)
+		private async void OnChoiceVmPropertyChanged (object sender, PropertyChangedEventArgs e)
 		{
 			if (this.fromUpdate)
 				return;
 
-			await PushValuesAsync ();
+			await PushValuesAsync (sender as FlaggableChoiceViewModel<TValue>);
 		}
 
-		private async Task PushValuesAsync ()
+		private async Task PushValuesAsync (FlaggableChoiceViewModel<TValue> changedChoice)
 		{
 			SetError (null);
 
 			using (await AsyncWork.RequestAsyncWork (this)) {
 				try {
+					// Snapshot current choices so we don't catch updates mid-push for multi-editors
+					var currentChoices = Choices.ToDictionary (c => c, c => c.IsFlagged);
+
 					foreach (IObjectEditor editor in Editors) {
 						ValueInfo<IReadOnlyList<TValue>> value = await editor.GetValueAsync<IReadOnlyList<TValue>> (Property, Variation);
 						HashSet<TValue> current;
@@ -139,19 +145,33 @@ namespace Xamarin.PropertyEditing.ViewModels
 						else
 							current = new HashSet<TValue> (value.Value);
 
-						foreach (var choice in Choices) {
-							if (!choice.IsFlagged.HasValue)
+						foreach (var choice in currentChoices) {
+							if (!choice.Value.HasValue)
 								continue;
 
-							if (choice.IsFlagged.Value)
-								current.Add (choice.Value);
+							if (choice.Value.Value)
+								current.Add (choice.Key.Value);
 							else
-								current.Remove (choice.Value);
+								current.Remove (choice.Key.Value);
 						}
 
 						IReadOnlyList<TValue> values = current.ToArray ();
-						if (this.validator != null)
-							values = this.validator.ValidateValue (values);
+						if (this.validator != null) {
+							if (!this.validator.IsValid (values)) {
+								// Some combinables simply don't have a valid "none", but if we're going from indeterminate we still need to
+								// update the value, so we'll flip the changed value to true in that case so we don't go right back to indeterminate
+								if (values.Count == 0) {
+									changedChoice.IsFlagged = true;
+									// We're explicitly triggering a change and need the update here so we need to update our snapshot.
+									currentChoices = Choices.ToDictionary (c => c, c => c.IsFlagged);
+								}
+
+								continue;
+							}
+						}
+
+						if (this.coerce != null)
+							values = this.coerce.CoerceValue (values);
 
 						await editor.SetValueAsync (Property, new ValueInfo<IReadOnlyList<TValue>> {
 							Source = ValueSource.Local,
diff --git a/Xamarin.PropertyEditing/ViewModels/ConstrainedPropertyViewModel.cs b/Xamarin.PropertyEditing/ViewModels/ConstrainedPropertyViewModel.cs
index 6d545e7..5912bce 100644
--- a/Xamarin.PropertyEditing/ViewModels/ConstrainedPropertyViewModel.cs
+++ b/Xamarin.PropertyEditing/ViewModels/ConstrainedPropertyViewModel.cs
@@ -57,7 +57,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 			}
 		}
 
-		protected override T ValidateValue (T validationValue)
+		protected override T CoerceValue (T validationValue)
 		{
 			if (IsConstrained) {
 				if (Compare (validationValue, MaximumValue) > 0)
@@ -66,7 +66,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 					validationValue = MinimumValue;
 			}
 
-			return base.ValidateValue (validationValue);
+			return base.CoerceValue (validationValue);
 		}
 
 		protected int Compare (T left, T right)
diff --git a/Xamarin.PropertyEditing/ViewModels/NumericPropertyViewModel.cs b/Xamarin.PropertyEditing/ViewModels/NumericPropertyViewModel.cs
index f6944f7..2666421 100644
--- a/Xamarin.PropertyEditing/ViewModels/NumericPropertyViewModel.cs
+++ b/Xamarin.PropertyEditing/ViewModels/NumericPropertyViewModel.cs
@@ -14,14 +14,14 @@ namespace Xamarin.PropertyEditing.ViewModels
 				Value = Numeric<T>.Increment (Value);
 			}, () => {
 				T value = Numeric<T>.Increment (Value);
-				return Compare (value, ValidateValue (value)) == 0;
+				return Compare (value, CoerceValue (value)) == 0;
 			});
 
 			this.lowerValue = new RelayCommand(() => {
 				Value = Numeric<T>.Decrement (Value);
 			}, () => {
 				T value = Numeric<T>.Decrement (Value);
-				return Compare (value, ValidateValue (value)) == 0;
+				return Compare (value, CoerceValue (value)) == 0;
 			});
 		}
 
diff --git a/Xamarin.PropertyEditing/ViewModels/PredefinedValuesViewModel.cs b/Xamarin.PropertyEditing/ViewModels/PredefinedValuesViewModel.cs
index 750d6d3..3a6744d 100644
--- a/Xamarin.PropertyEditing/ViewModels/PredefinedValuesViewModel.cs
+++ b/Xamarin.PropertyEditing/ViewModels/PredefinedValuesViewModel.cs
@@ -43,10 +43,10 @@ namespace Xamarin.PropertyEditing.ViewModels
 
 		public bool IsConstrainedToPredefined => this.predefinedValues.IsConstrainedToPredefined;
 
-		protected override TValue ValidateValue (TValue validationValue)
+		protected override TValue CoerceValue (TValue validationValue)
 		{
 			if (!IsConstrainedToPredefined || IsValueDefined (validationValue))
-				return base.ValidateValue (validationValue);
+				return base.CoerceValue (validationValue);
 
 			return Value;
 		}
diff --git a/Xamarin.PropertyEditing/ViewModels/PropertyViewModel.cs b/Xamarin.PropertyEditing/ViewModels/PropertyViewModel.cs
index d54b3b0..f3aa8ca 100644
--- a/Xamarin.PropertyEditing/ViewModels/PropertyViewModel.cs
+++ b/Xamarin.PropertyEditing/ViewModels/PropertyViewModel.cs
@@ -23,6 +23,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 		public PropertyViewModel (TargetPlatform platform, IPropertyInfo property, IEnumerable<IObjectEditor> editors)
 			: base (platform, property, editors)
 		{
+			this.coerce = property as ICoerce<TValue>;
 			this.validator = property as IValidator<TValue>;
 			this.isNullable = (!property.ValueSources.HasFlag (ValueSources.Default) || property.Type.Name == NullableName);
 
@@ -39,7 +40,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 			get { return (this.value != null) ? this.value.Value : default(TValue); }
 			set
 			{
-				value = ValidateValue (value);
+				value = CoerceValue (value);
 				SetValue (new ValueInfo<TValue> {
 					Source = ValueSource.Local,
 					Value = value
@@ -79,14 +80,14 @@ namespace Xamarin.PropertyEditing.ViewModels
 			}
 		}
 
-		protected virtual TValue ValidateValue (TValue validationValue)
+		protected virtual TValue CoerceValue (TValue validationValue)
 		{
 			if (!this.isNullable && validationValue == null) {
 				validationValue = DefaultValue;
 			}
 
-			if (this.validator != null)
-				validationValue = this.validator.ValidateValue (validationValue);
+			if (this.coerce != null)
+				validationValue = this.coerce.CoerceValue (validationValue);
 
 			return validationValue;
 		}
@@ -153,6 +154,12 @@ namespace Xamarin.PropertyEditing.ViewModels
 
 			SetError (null);
 
+			// We may need to be more careful about value sources here
+			if (this.validator != null && !this.validator.IsValid (newValue.Value)) {
+				SignalValueChange(); // Ensure UI refresh its own value
+				return;
+			}
+
 			using (await AsyncWork.RequestAsyncWork (this)) {
 				try {
 					Task[] setValues = new Task[Editors.Count];
@@ -176,11 +183,20 @@ namespace Xamarin.PropertyEditing.ViewModels
 			}
 		}
 
+		private readonly ICoerce<TValue> coerce;
 		private readonly IValidator<TValue> validator;
 		private const string NullableName = "Nullable`1";
 		private bool isNullable;
 		private ValueInfo<TValue> value;
 
+		private void SignalValueChange ()
+		{
+			OnPropertyChanged (nameof (Value));
+			OnPropertyChanged (nameof (ValueSource));
+			OnPropertyChanged (nameof (CustomExpression));
+			OnPropertyChanged (nameof (Resource));
+		}
+
 		private bool SetCurrentValue (ValueInfo<TValue> newValue)
 		{
 			if (!this.isNullable && newValue != null && newValue.Value == null)
@@ -191,10 +207,7 @@ namespace Xamarin.PropertyEditing.ViewModels
 
 			this.value = newValue;
 			OnValueChanged ();
-			OnPropertyChanged (nameof (Value));
-			OnPropertyChanged (nameof (ValueSource));
-			OnPropertyChanged (nameof (CustomExpression));
-			OnPropertyChanged (nameof (Resource));
+			SignalValueChange();
 
 			((RelayCommand) ClearValueCommand)?.ChangeCanExecute ();
 
diff --git a/Xamarin.PropertyEditing/Xamarin.PropertyEditing.csproj b/Xamarin.PropertyEditing/Xamarin.PropertyEditing.csproj
index f7d2a50..91c10ec 100644
--- a/Xamarin.PropertyEditing/Xamarin.PropertyEditing.csproj
+++ b/Xamarin.PropertyEditing/Xamarin.PropertyEditing.csproj
@@ -74,7 +74,7 @@
     <Compile Include="IAvailabilityConstraint.cs" />
     <Compile Include="IClampedPropertyInfo.cs" />
     <Compile Include="IColorSpaced.cs" />
-    <Compile Include="IValidator.cs" />
+    <Compile Include="ICoerce.cs" />
     <Compile Include="IEditorProvider.cs" />
     <Compile Include="IObjectEventEditor.cs" />
     <Compile Include="IEventInfo.cs" />
@@ -85,6 +85,7 @@
     <Compile Include="IResourceProvider.cs" />
     <Compile Include="ISelfConstrainedPropertyInfo.cs" />
     <Compile Include="ITypeInfo.cs" />
+    <Compile Include="IValidator.cs" />
     <Compile Include="MultiAvailabilityConstraint.cs" />
     <Compile Include="NotifyingObject.cs" />
     <Compile Include="Numeric.cs" />