From 0ef83d908f97616e724986cf41917dc02bb8d5af Mon Sep 17 00:00:00 2001 From: Eric Maupin Date: Fri, 13 Jul 2018 11:54:33 -0400 Subject: [Core] Split SourceDescriptor out from ValueDescriptor ValueDescriptor ended up being used for overlapping purposes: binding/resources for the source but then for some values like objects and predfined values it had extra information not contained in the Value. SourceDescriptor was added to hold things like bindings or resources that describe where the value came from, while ValueDescriptor will continue to hold extra details about the value. There were two possible approaches to this, to add the object property as-is in this commit, or to add bespoke properties for a Resource and Binding. Ultimately both approaches have fairly similar level pros and cons. What it ended up coming down to for me is that these properties are mutually exclusive. While we can guard against this in a few ways (target the same backing property, throw an exception), both of these cases require extra code to be mindful of in some usage where it does not with a single property, or worse it is silently unexpected behavior. While it is cleaner to not have to cast, the risk of introducing extra code complexity for the sake of structure tips the scales. --- .../CombinablePredefinedViewModelTests.cs | 1 + .../MaterialDesignColorViewModelTests.cs | 2 +- Xamarin.PropertyEditing.Tests/MockObjectEditor.cs | 27 ++++++++++++++-------- .../PropertyViewModelTests.cs | 8 +++---- 4 files changed, 23 insertions(+), 15 deletions(-) (limited to 'Xamarin.PropertyEditing.Tests') diff --git a/Xamarin.PropertyEditing.Tests/CombinablePredefinedViewModelTests.cs b/Xamarin.PropertyEditing.Tests/CombinablePredefinedViewModelTests.cs index 4cd84ce..ac00575 100644 --- a/Xamarin.PropertyEditing.Tests/CombinablePredefinedViewModelTests.cs +++ b/Xamarin.PropertyEditing.Tests/CombinablePredefinedViewModelTests.cs @@ -552,6 +552,7 @@ namespace Xamarin.PropertyEditing.Tests return new ValueInfo> { Value = setValues, Source = valueInfo.Source, + SourceDescriptor = valueInfo.SourceDescriptor, CustomExpression = valueInfo.CustomExpression, ValueDescriptor = valueInfo.ValueDescriptor }; diff --git a/Xamarin.PropertyEditing.Tests/MaterialDesignColorViewModelTests.cs b/Xamarin.PropertyEditing.Tests/MaterialDesignColorViewModelTests.cs index d211599..2611002 100644 --- a/Xamarin.PropertyEditing.Tests/MaterialDesignColorViewModelTests.cs +++ b/Xamarin.PropertyEditing.Tests/MaterialDesignColorViewModelTests.cs @@ -261,7 +261,7 @@ namespace Xamarin.PropertyEditing.Tests await mockEditor.SetValueAsync (mockProperty.Object, new ValueInfo { Source = ValueSource.Resource, Value = resource.Value, - ValueDescriptor = resource + SourceDescriptor = resource }); var vm = new BrushPropertyViewModel (platform, mockProperty.Object, new[] { mockEditor }); diff --git a/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs b/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs index 8c0f6a5..14ea035 100644 --- a/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs +++ b/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs @@ -69,7 +69,7 @@ namespace Xamarin.PropertyEditing.Tests public event EventHandler PropertyChanged; - public Func ValueEvaluator + public Func ValueEvaluator { get; set; @@ -150,11 +150,12 @@ namespace Xamarin.PropertyEditing.Tests CustomExpression = value.CustomExpression, Source = value.Source, ValueDescriptor = value.ValueDescriptor, + SourceDescriptor = value.SourceDescriptor, Value = value.Value }; if (value.Source != ValueSource.Local && ValueEvaluator != null) { - value.Value = (T)ValueEvaluator (property, value.ValueDescriptor); + value.Value = (T)ValueEvaluator (property, value.ValueDescriptor, value.SourceDescriptor); } else if (value.Source == ValueSource.Unset || (property.ValueSources.HasFlag (ValueSources.Default) && Equals (value.Value, default(T))) && value.ValueDescriptor == null) { this.values.Remove (property); PropertyChanged?.Invoke (this, new EditorPropertyChangedEventArgs (property)); @@ -171,7 +172,9 @@ namespace Xamarin.PropertyEditing.Tests var softType = typeof(ValueInfo<>).MakeGenericType (property.Type); softValue = Activator.CreateInstance (softType); softType.GetProperty ("Value").SetValue (softValue, v); + softType.GetProperty ("ValueDescriptor").SetValue (softValue, value.ValueDescriptor); softType.GetProperty ("Source").SetValue (softValue, value.Source); + softType.GetProperty ("SourceDescriptor").SetValue (softValue, value.SourceDescriptor); } if (typeof(T).Name == "IReadOnlyList`1") { @@ -189,18 +192,18 @@ namespace Xamarin.PropertyEditing.Tests } // Set to resource won't pass values so we will store it on the info since we just pass it back in GetValue - if (value.Source == ValueSource.Resource && value.ValueDescriptor is Resource) { - Type rt = value.ValueDescriptor.GetType(); + if (value.Source == ValueSource.Resource && value.SourceDescriptor is Resource) { + Type rt = value.SourceDescriptor.GetType(); if (rt.IsGenericType) { Type ta = rt.GetGenericArguments ()[0]; if (typeof (T).IsAssignableFrom (ta)) { PropertyInfo pi = rt.GetProperty ("Value"); - value.Value = (T)pi.GetValue (value.ValueDescriptor); + value.Value = (T)pi.GetValue (value.SourceDescriptor); } else { TypeConverter converter = TypeDescriptor.GetConverter (ta); if (converter != null && converter.CanConvertTo(typeof(T))) { PropertyInfo pi = rt.GetProperty ("Value"); - value.Value = (T)converter.ConvertTo (pi.GetValue (value.ValueDescriptor), typeof (T)); + value.Value = (T)converter.ConvertTo (pi.GetValue (value.SourceDescriptor), typeof (T)); } } } @@ -225,6 +228,7 @@ namespace Xamarin.PropertyEditing.Tests CustomExpression = info.CustomExpression, Source = info.Source, ValueDescriptor = info.ValueDescriptor, + SourceDescriptor = info.SourceDescriptor, Value = info.Value }; } else if (value == null || value is T) { @@ -258,12 +262,13 @@ namespace Xamarin.PropertyEditing.Tests Source = underlyingInfo?.Source ?? ValueSource.Local }, typeof(ValueInfo)); } else { - object descriptor = null; + object sourceDescriptor = null, valueDescriptor = null; ValueSource source = ValueSource.Local; Type valueType = value.GetType (); if (valueType.IsConstructedGenericType && valueType.GetGenericTypeDefinition () == typeof(ValueInfo<>)) { source = (ValueSource)valueType.GetProperty ("Source").GetValue (value); - descriptor = valueType.GetProperty (nameof (ValueInfo.ValueDescriptor)).GetValue (value); + sourceDescriptor = valueType.GetProperty (nameof (ValueInfo.SourceDescriptor)).GetValue (value); + valueDescriptor = valueType.GetProperty (nameof (ValueInfo.ValueDescriptor)).GetValue (value); value = valueType.GetProperty ("Value").GetValue (value); valueType = valueType.GetGenericArguments ()[0]; } @@ -274,13 +279,15 @@ namespace Xamarin.PropertyEditing.Tests return new ValueInfo { Source = source, Value = (T)newValue, - ValueDescriptor = descriptor + ValueDescriptor = valueDescriptor, + SourceDescriptor = sourceDescriptor }; } else if (typeof(T).IsAssignableFrom (valueType)) { return new ValueInfo { Source = source, Value = (T)value, - ValueDescriptor = descriptor + ValueDescriptor = valueDescriptor, + SourceDescriptor = sourceDescriptor }; } } diff --git a/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs b/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs index 04a23af..a24b2ee 100644 --- a/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs +++ b/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs @@ -434,8 +434,8 @@ namespace Xamarin.PropertyEditing.Tests var resourcesMock = new Mock (); resourcesMock.Setup (rp => rp.GetResourcesAsync (editor.Target, mockProperty.Object, It.IsAny ())).ReturnsAsync (new[] { resource }); - editor.ValueEvaluator = (info, o) => { - if (o == resource) + editor.ValueEvaluator = (info, val, source) => { + if (source == resource) return value; return default(TValue); @@ -464,7 +464,7 @@ namespace Xamarin.PropertyEditing.Tests var editor = new MockObjectEditor (mockProperty.Object); await editor.SetValueAsync (mockProperty.Object, new ValueInfo { Source = ValueSource.Resource, - ValueDescriptor = resource, + SourceDescriptor = resource, Value = value }); @@ -490,7 +490,7 @@ namespace Xamarin.PropertyEditing.Tests var editor = new MockObjectEditor (mockProperty.Object); await editor.SetValueAsync (mockProperty.Object, new ValueInfo { Source = ValueSource.Resource, - ValueDescriptor = resource, + SourceDescriptor = resource, Value = value }); -- cgit v1.2.3 From 91de39d5576fb952cd12559456359606619f865c Mon Sep 17 00:00:00 2001 From: Eric Maupin Date: Fri, 13 Jul 2018 12:05:31 -0400 Subject: [Core] Move ObjectPropertyViewModel base to PVM We don't really need this to be separate, it only helped to diverge capabilities that arrived in the base value view model but not in the object one. While it might be a little weird to have Value available for something that is meant to be edited at a sub-property level, it wouldn't be the first VM that has a separate "real" value property and we actually do need a PVM that we can set the value on for bindings. This also moves the assignable types request to be fired only when actually requested. --- Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Xamarin.PropertyEditing.Tests') diff --git a/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs b/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs index a24b2ee..f87f930 100644 --- a/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs +++ b/Xamarin.PropertyEditing.Tests/PropertyViewModelTests.cs @@ -253,7 +253,7 @@ namespace Xamarin.PropertyEditing.Tests Assume.That (vm.Value, Is.EqualTo (default (TValue))); Assume.That (vm.MultipleValues, Is.True); - Assert.That (vm.ValueSource, Is.EqualTo (ValueSource.Default)); + Assert.That (vm.ValueSource, Is.EqualTo (ValueSource.Unknown)); } [Test] -- cgit v1.2.3 From 322c01ba86bbf0bd29e656d264947c32a7acb7e6 Mon Sep 17 00:00:00 2001 From: Eric Maupin Date: Wed, 18 Jul 2018 17:19:51 -0400 Subject: [Docs] Add extra documentation around value/source descriptors --- Xamarin.PropertyEditing.Tests/MockObjectEditor.cs | 3 +++ 1 file changed, 3 insertions(+) (limited to 'Xamarin.PropertyEditing.Tests') diff --git a/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs b/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs index 14ea035..fc1a1eb 100644 --- a/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs +++ b/Xamarin.PropertyEditing.Tests/MockObjectEditor.cs @@ -69,6 +69,9 @@ namespace Xamarin.PropertyEditing.Tests public event EventHandler PropertyChanged; + /// + /// Test helper for non-local values, passes in the property, , + /// public Func ValueEvaluator { get; -- cgit v1.2.3