From 8209a85193c3bf589de4b292d1b3a5f668282d91 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 26 Feb 2016 10:45:45 +0100 Subject: [PATCH] Make IBinding return an InstancedBinding. Instead of an ISubject as this was wasteful when a OneTime or OneWay binding was required. --- .../Context/PropertyAccessor.cs | 17 ++- .../Perspex.Markup.Xaml/Data/Binding.cs | 6 +- .../Perspex.Markup.Xaml/Data/MultiBinding.cs | 8 +- .../Data/StyleResourceBinding.cs | 53 ++------- src/Perspex.Base/Data/BindingOperations.cs | 71 ++++++++++++ src/Perspex.Base/Data/IBinding.cs | 32 ++---- src/Perspex.Base/Data/InstancedBinding.cs | 108 ++++++++++++++++++ src/Perspex.Base/Perspex.Base.csproj | 2 + src/Perspex.Base/PerspexObject.cs | 3 +- src/Perspex.Base/PerspexObjectExtensions.cs | 66 +++-------- .../Styling/ActivatedObservable.cs | 3 + src/Perspex.Styling/Styling/ISetter.cs | 2 +- src/Perspex.Styling/Styling/Setter.cs | 57 ++++----- .../Data/BindingTests.cs | 6 +- .../Data/MultiBindingTests.cs | 2 +- .../Xaml/StyleTests.cs | 26 +++++ .../Perspex.Styling.UnitTests/SetterTests.cs | 3 +- 17 files changed, 308 insertions(+), 157 deletions(-) create mode 100644 src/Perspex.Base/Data/BindingOperations.cs create mode 100644 src/Perspex.Base/Data/InstancedBinding.cs diff --git a/src/Markup/Perspex.Markup.Xaml/Context/PropertyAccessor.cs b/src/Markup/Perspex.Markup.Xaml/Context/PropertyAccessor.cs index 18b464a211..a15c2a3b2b 100644 --- a/src/Markup/Perspex.Markup.Xaml/Context/PropertyAccessor.cs +++ b/src/Markup/Perspex.Markup.Xaml/Context/PropertyAccessor.cs @@ -130,12 +130,25 @@ namespace Perspex.Markup.Xaml.Context } else { - IPerspexObject treeAnchor = context.TopDownValueContext.StoredInstances + // The target is not a control, so we need to find an anchor that will let us look + // up named controls and style resources. First look for the closest IControl in + // the TopDownValueContext. + object anchor = context.TopDownValueContext.StoredInstances .Select(x => x.Instance) .OfType() .LastOrDefault(); - ((IPerspexObject)instance).Bind(property, binding, treeAnchor); + // If a control was not found, then try to find the highest-level style as the XAML + // file could be a XAML file containing only styles. + if (anchor == null) + { + anchor = context.TopDownValueContext.StoredInstances + .Select(x => x.Instance) + .OfType() + .FirstOrDefault(); + } + + ((IPerspexObject)instance).Bind(property, binding, anchor); } return true; diff --git a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs index 27d4672097..bd116f3396 100644 --- a/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs +++ b/src/Markup/Perspex.Markup.Xaml/Data/Binding.cs @@ -62,7 +62,7 @@ namespace Perspex.Markup.Xaml.Data public object Source { get; set; } /// - public ISubject CreateSubject( + public InstancedBinding Initiate( IPerspexObject target, PerspexProperty targetProperty, object anchor = null) @@ -101,12 +101,14 @@ namespace Perspex.Markup.Xaml.Data throw new NotSupportedException(); } - return new ExpressionSubject( + var subject = new ExpressionSubject( observer, targetProperty?.PropertyType ?? typeof(object), Converter ?? DefaultValueConverter.Instance, ConverterParameter, FallbackValue); + + return new InstancedBinding(subject, Mode, Priority); } private static PathInfo ParsePath(string path) diff --git a/src/Markup/Perspex.Markup.Xaml/Data/MultiBinding.cs b/src/Markup/Perspex.Markup.Xaml/Data/MultiBinding.cs index f8b4767c30..747ef15972 100644 --- a/src/Markup/Perspex.Markup.Xaml/Data/MultiBinding.cs +++ b/src/Markup/Perspex.Markup.Xaml/Data/MultiBinding.cs @@ -50,7 +50,7 @@ namespace Perspex.Markup.Xaml.Data public RelativeSource RelativeSource { get; set; } /// - public ISubject CreateSubject( + public InstancedBinding Initiate( IPerspexObject target, PerspexProperty targetProperty, object anchor = null) @@ -62,10 +62,10 @@ namespace Perspex.Markup.Xaml.Data var targetType = targetProperty?.PropertyType ?? typeof(object); var result = new BehaviorSubject(PerspexProperty.UnsetValue); - var children = Bindings.Select(x => x.CreateSubject(target, null)); - var input = children.CombineLatest().Select(x => ConvertValue(x, targetType)); + var children = Bindings.Select(x => x.Initiate(target, null)); + var input = children.Select(x => x.Subject).CombineLatest().Select(x => ConvertValue(x, targetType)); input.Subscribe(result); - return result; + return new InstancedBinding(result, Mode, Priority); } /// diff --git a/src/Markup/Perspex.Markup.Xaml/Data/StyleResourceBinding.cs b/src/Markup/Perspex.Markup.Xaml/Data/StyleResourceBinding.cs index e10eb1a643..5dd656ca54 100644 --- a/src/Markup/Perspex.Markup.Xaml/Data/StyleResourceBinding.cs +++ b/src/Markup/Perspex.Markup.Xaml/Data/StyleResourceBinding.cs @@ -34,60 +34,31 @@ namespace Perspex.Markup.Xaml.Data public BindingPriority Priority => BindingPriority.LocalValue; /// - public ISubject CreateSubject( + public InstancedBinding Initiate( IPerspexObject target, PerspexProperty targetProperty, object anchor = null) { - return new Subject(target, Name, anchor); - } - - private class Subject : ISubject - { - private IPerspexObject _target; - private string _name; - private object _anchor; - - public Subject(IPerspexObject target, string name, object anchor) - { - _target = target; - _name = name; - this._anchor = anchor; - } + var host = (target as IControl) ?? (anchor as IControl); + var style = anchor as IStyle; + var resource = PerspexProperty.UnsetValue; - public void OnCompleted() + if (host != null) { + resource = host.FindStyleResource(Name); } - - public void OnError(Exception error) + else if (style != null) { + resource = style.FindResource(Name); } - public void OnNext(object value) + if (resource != PerspexProperty.UnsetValue) { + return new InstancedBinding(resource, Priority); } - - public IDisposable Subscribe(IObserver observer) + else { - var host = (_target as IControl) ?? (_anchor as IControl); - - if (host != null) - { - var resource = host.FindStyleResource(_name); - - if (resource != PerspexProperty.UnsetValue) - { - observer.OnNext(resource); - } - - observer.OnCompleted(); - } - else - { - // TODO: Log error. - } - - return Disposable.Empty; + return null; } } } diff --git a/src/Perspex.Base/Data/BindingOperations.cs b/src/Perspex.Base/Data/BindingOperations.cs new file mode 100644 index 0000000000..efa0c8500d --- /dev/null +++ b/src/Perspex.Base/Data/BindingOperations.cs @@ -0,0 +1,71 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using System.Linq; +using System.Reactive.Disposables; +using System.Reactive.Linq; + +namespace Perspex.Data +{ + public static class BindingOperations + { + /// + /// Applies an a property on an . + /// + /// The target object. + /// The property to bind. + /// The instanced binding. + /// + /// An optional anchor from which to locate required context. When binding to objects that + /// are not in the logical tree, certain types of binding need an anchor into the tree in + /// order to locate named controls or resources. The parameter + /// can be used to provice this context. + /// + /// An which can be used to cancel the binding. + public static IDisposable Apply( + IPerspexObject target, + PerspexProperty property, + InstancedBinding binding, + object anchor) + { + Contract.Requires(target != null); + Contract.Requires(property != null); + Contract.Requires(binding != null); + + var mode = binding.Mode; + + if (mode == BindingMode.Default) + { + mode = property.GetMetadata(target.GetType()).DefaultBindingMode; + } + + switch (mode) + { + case BindingMode.Default: + case BindingMode.OneWay: + return target.Bind(property, binding.Observable ?? binding.Subject, binding.Priority); + case BindingMode.TwoWay: + return new CompositeDisposable( + target.Bind(property, binding.Subject, binding.Priority), + target.GetObservable(property).Subscribe(binding.Subject)); + case BindingMode.OneTime: + var source = binding.Subject ?? binding.Observable; + + if (source != null) + { + return source.Take(1).Subscribe(x => target.SetValue(property, x, binding.Priority)); + } + else + { + target.SetValue(property, binding.Value, binding.Priority); + return Disposable.Empty; + } + case BindingMode.OneWayToSource: + return target.GetObservable(property).Subscribe(binding.Subject); + default: + throw new ArgumentException("Invalid binding mode."); + } + } + } +} diff --git a/src/Perspex.Base/Data/IBinding.cs b/src/Perspex.Base/Data/IBinding.cs index 1ad45913f9..0b7bbef108 100644 --- a/src/Perspex.Base/Data/IBinding.cs +++ b/src/Perspex.Base/Data/IBinding.cs @@ -1,8 +1,6 @@ // Copyright (c) The Perspex Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. -using System.Reactive.Subjects; - namespace Perspex.Data { /// @@ -11,28 +9,20 @@ namespace Perspex.Data public interface IBinding { /// - /// Gets the binding mode. - /// - BindingMode Mode { get; } - - /// - /// Gets the binding priority. - /// - BindingPriority Priority { get; } - - /// - /// Creates a subject that can be used to get and set the value of the binding. + /// Initiates the binding on a target object. /// /// The target instance. /// The target property. May be null. - /// An optional anchor from which to locate required context. - /// An . - /// - /// When binding to objects that are not in the logical tree, certain types of binding need - /// an anchor into the tree in order to locate named controls or resources. The - /// parameter can be used to provice this context. - /// - ISubject CreateSubject( + /// + /// An optional anchor from which to locate required context. When binding to objects that + /// are not in the logical tree, certain types of binding need an anchor into the tree in + /// order to locate named controls or resources. The parameter + /// can be used to provice this context. + /// + /// + /// A or null if the binding could not be resolved. + /// + InstancedBinding Initiate( IPerspexObject target, PerspexProperty targetProperty, object anchor = null); diff --git a/src/Perspex.Base/Data/InstancedBinding.cs b/src/Perspex.Base/Data/InstancedBinding.cs new file mode 100644 index 0000000000..545d690aa4 --- /dev/null +++ b/src/Perspex.Base/Data/InstancedBinding.cs @@ -0,0 +1,108 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using System.Reactive.Subjects; + +namespace Perspex.Data +{ + /// + /// Holds the result of calling . + /// + /// + /// Whereas an holds a description of a binding such as "Bind to the X + /// property on a control's DataContext"; this class represents a binding that has been + /// *instanced* by calling + /// on a target object. + /// + /// When a binding is initiated, it can return one of 3 possible sources for the binding: + /// - An which can be used for any type of binding. + /// - An which can be used for all types of bindings except + /// and . + /// - A plain object, which can only represent a binding. + /// + public class InstancedBinding + { + /// + /// Initializes a new instance of the class. + /// + /// + /// The value used for the binding. + /// + /// The binding priority. + public InstancedBinding(object value, BindingPriority priority = BindingPriority.LocalValue) + { + Mode = BindingMode.OneTime; + Priority = priority; + Value = value; + } + + /// + /// Initializes a new instance of the class. + /// + /// The observable for a one-way binding. + /// The binding mode. + /// The binding priority. + public InstancedBinding( + IObservable observable, + BindingMode mode = BindingMode.OneWay, + BindingPriority priority = BindingPriority.LocalValue) + { + Contract.Requires(observable != null); + + if (mode == BindingMode.OneWayToSource || mode == BindingMode.TwoWay) + { + throw new ArgumentException( + "Invalid BindingResult mode: OneWayToSource and TwoWay bindings" + + "require a Subject."); + } + + Mode = mode; + Priority = priority; + Observable = observable; + } + + /// + /// Initializes a new instance of the class. + /// + /// The subject for a two-way binding. + /// The binding mode. + /// The binding priority. + public InstancedBinding( + ISubject subject, + BindingMode mode = BindingMode.OneWay, + BindingPriority priority = BindingPriority.LocalValue) + { + Contract.Requires(subject != null); + + Mode = mode; + Priority = priority; + Subject = subject; + } + + /// + /// Gets the binding mode with which the binding was initiated. + /// + public BindingMode Mode { get; } + + /// + /// Gets the binding priority. + /// + public BindingPriority Priority { get; } + + /// + /// Gets the value used for a binding. + /// + public object Value { get; } + + /// + /// Gets the observable for a one-way binding. + /// + public IObservable Observable { get; } + + /// + /// Gets the subject for a two-way binding. + /// + public ISubject Subject { get; } + } +} diff --git a/src/Perspex.Base/Perspex.Base.csproj b/src/Perspex.Base/Perspex.Base.csproj index 2ffb1dc7f2..b886e9b08b 100644 --- a/src/Perspex.Base/Perspex.Base.csproj +++ b/src/Perspex.Base/Perspex.Base.csproj @@ -44,6 +44,8 @@ Properties\SharedAssemblyInfo.cs + + diff --git a/src/Perspex.Base/PerspexObject.cs b/src/Perspex.Base/PerspexObject.cs index c1f1d246d0..c3b6f321f0 100644 --- a/src/Perspex.Base/PerspexObject.cs +++ b/src/Perspex.Base/PerspexObject.cs @@ -188,7 +188,8 @@ namespace Perspex break; case BindingMode.TwoWay: var subject = sourceBinding.Source.GetSubject(sourceBinding.Property, sourceBinding.Priority); - this.Bind(binding.Property, subject, BindingMode.TwoWay, sourceBinding.Priority); + var instanced = new InstancedBinding(subject, BindingMode.TwoWay, sourceBinding.Priority); + BindingOperations.Apply(this, binding.Property, instanced, null); break; } } diff --git a/src/Perspex.Base/PerspexObjectExtensions.cs b/src/Perspex.Base/PerspexObjectExtensions.cs index 9f5ccaaf8a..dc0a6b6465 100644 --- a/src/Perspex.Base/PerspexObjectExtensions.cs +++ b/src/Perspex.Base/PerspexObjectExtensions.cs @@ -167,73 +167,35 @@ namespace Perspex /// /// Binds a property on an to an . /// - /// The object. + /// The object. /// The property to bind. /// The binding. - /// - /// For `ElementName` bindings to elements that are not themselves controls, describes - /// where in the logical tree to begin searching for the named element. + /// + /// An optional anchor from which to locate required context. When binding to objects that + /// are not in the logical tree, certain types of binding need an anchor into the tree in + /// order to locate named controls or resources. The parameter + /// can be used to provice this context. /// /// An which can be used to cancel the binding. public static IDisposable Bind( - this IPerspexObject o, + this IPerspexObject target, PerspexProperty property, IBinding binding, - IPerspexObject treeAnchor = null) + object anchor = null) { - Contract.Requires(o != null); + Contract.Requires(target != null); Contract.Requires(property != null); Contract.Requires(binding != null); - var mode = binding.Mode; + var result = binding.Initiate(target, property, anchor); - if (mode == BindingMode.Default) + if (result != null) { - mode = property.GetMetadata(o.GetType()).DefaultBindingMode; + return BindingOperations.Apply(target, property, result, anchor); } - - return o.Bind( - property, - binding.CreateSubject(o, property, treeAnchor), - mode, - binding.Priority); - } - - /// - /// Binds a property to a subject according to a . - /// - /// The object. - /// The property to bind. - /// The binding source. - /// The binding mode. - /// The binding priority. - /// An which can be used to cancel the binding. - public static IDisposable Bind( - this IPerspexObject o, - PerspexProperty property, - ISubject source, - BindingMode mode, - BindingPriority priority = BindingPriority.LocalValue) - { - Contract.Requires(o != null); - Contract.Requires(property != null); - Contract.Requires(source != null); - - switch (mode) + else { - case BindingMode.Default: - case BindingMode.OneWay: - return o.Bind(property, source, priority); - case BindingMode.TwoWay: - return new CompositeDisposable( - o.Bind(property, source, priority), - o.GetObservable(property).Subscribe(source)); - case BindingMode.OneTime: - return source.Take(1).Subscribe(x => o.SetValue(property, x, priority)); - case BindingMode.OneWayToSource: - return o.GetObservable(property).Subscribe(source); - default: - throw new ArgumentException("Invalid binding mode."); + return Disposable.Empty; } } diff --git a/src/Perspex.Styling/Styling/ActivatedObservable.cs b/src/Perspex.Styling/Styling/ActivatedObservable.cs index 97c1b73106..4ab2f04495 100644 --- a/src/Perspex.Styling/Styling/ActivatedObservable.cs +++ b/src/Perspex.Styling/Styling/ActivatedObservable.cs @@ -30,6 +30,9 @@ namespace Perspex.Styling IObservable source, string description) { + Contract.Requires(activator != null); + Contract.Requires(source != null); + Activator = activator; Description = description; Source = source; diff --git a/src/Perspex.Styling/Styling/ISetter.cs b/src/Perspex.Styling/Styling/ISetter.cs index b249f78d7b..105a8ec6ac 100644 --- a/src/Perspex.Styling/Styling/ISetter.cs +++ b/src/Perspex.Styling/Styling/ISetter.cs @@ -11,7 +11,7 @@ namespace Perspex.Styling public interface ISetter { /// - /// Applies the setter to the control. + /// Applies the setter to a control. /// /// The style that is being applied. /// The control. diff --git a/src/Perspex.Styling/Styling/Setter.cs b/src/Perspex.Styling/Styling/Setter.cs index 8fff896665..d31f7a3d66 100644 --- a/src/Perspex.Styling/Styling/Setter.cs +++ b/src/Perspex.Styling/Styling/Setter.cs @@ -58,7 +58,7 @@ namespace Perspex.Styling } /// - /// Applies the setter to the control. + /// Applies the setter to a control. /// /// The style that is being applied. /// The control. @@ -76,51 +76,52 @@ namespace Perspex.Styling var binding = Value as IBinding; - if (binding != null) + if (binding == null) { if (activator == null) { - control.Bind(Property, binding); + control.SetValue(Property, Value, BindingPriority.Style); } else { - var subject = binding.CreateSubject(control, Property); - var activated = new ActivatedSubject(activator, subject, description); - Bind(control, Property, binding, activated); + var activated = new ActivatedValue(activator, Value, description); + var instanced = new InstancedBinding( + activated, + BindingMode.OneWay, + BindingPriority.StyleTrigger); + BindingOperations.Apply(control, Property, instanced, null); } } else { if (activator == null) { - control.SetValue(Property, Value, BindingPriority.Style); + control.Bind(Property, binding); } else { - var activated = new ActivatedValue(activator, Value, description); - control.Bind(Property, activated, BindingPriority.StyleTrigger); - } - } - } + var sourceInstance = binding.Initiate(control, Property); + InstancedBinding activatedInstance; - private void Bind( - IStyleable control, - PerspexProperty property, - IBinding binding, - ISubject subject) - { - var mode = binding.Mode; + if (sourceInstance.Subject != null) + { + var activated = new ActivatedSubject(activator, sourceInstance.Subject, description); + activatedInstance = new InstancedBinding(activated, sourceInstance.Mode, sourceInstance.Priority); + } + else if (sourceInstance.Observable != null) + { + var activated = new ActivatedObservable(activator, sourceInstance.Observable, description); + activatedInstance = new InstancedBinding(activated, sourceInstance.Mode, sourceInstance.Priority); + } + else + { + var activated = new ActivatedValue(activator, sourceInstance.Value, description); + activatedInstance = new InstancedBinding(activated, sourceInstance.Mode, sourceInstance.Priority); + } - if (mode == BindingMode.Default) - { - mode = property.GetMetadata(control.GetType()).DefaultBindingMode; + BindingOperations.Apply(control, Property, activatedInstance, null); + } } - - control.Bind( - property, - subject, - mode, - binding.Priority); } } } diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests.cs b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests.cs index e70023fa35..8a9aa3ad79 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Data/BindingTests.cs @@ -168,7 +168,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data Path = "Foo", }; - var result = binding.CreateSubject(target, TextBox.TextProperty); + var result = binding.Initiate(target, TextBox.TextProperty).Subject; Assert.IsType(((ExpressionSubject)result).Converter); } @@ -184,7 +184,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data Path = "Foo", }; - var result = binding.CreateSubject(target, TextBox.TextProperty); + var result = binding.Initiate(target, TextBox.TextProperty).Subject; Assert.Same(converter.Object, ((ExpressionSubject)result).Converter); } @@ -201,7 +201,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data Path = "Bar", }; - var result = binding.CreateSubject(target, TextBox.TextProperty); + var result = binding.Initiate(target, TextBox.TextProperty).Subject; Assert.Same("foo", ((ExpressionSubject)result).ConverterParameter); } diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Data/MultiBindingTests.cs b/tests/Perspex.Markup.Xaml.UnitTests/Data/MultiBindingTests.cs index 13c74aca30..5b0da45ac6 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Data/MultiBindingTests.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Data/MultiBindingTests.cs @@ -33,7 +33,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Data var target = new Mock(); target.Setup(x => x.GetValue(Control.DataContextProperty)).Returns(source); - var subject = binding.CreateSubject(target.Object, null); + var subject = binding.Initiate(target.Object, null).Subject; var result = await subject.Take(1); Assert.Equal("1,2,3", result); diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Xaml/StyleTests.cs b/tests/Perspex.Markup.Xaml.UnitTests/Xaml/StyleTests.cs index a141c4e942..8bfb9e8377 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Xaml/StyleTests.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Xaml/StyleTests.cs @@ -172,5 +172,31 @@ namespace Perspex.Markup.Xaml.UnitTests.Xaml Assert.Equal(0xff506070, brush.Color.ToUint32()); } + + [Fact] + public void StyleResource_Can_Be_Found_In_Sibling_Styles() + { + var xaml = @" + + + +"; + + var loader = new PerspexXamlLoader(); + var styles = (Styles)loader.Load(xaml); + var brush = (Perspex.Media.Mutable.SolidColorBrush)styles.FindResource("brush"); + + Assert.Equal(0xff506070, brush.Color.ToUint32()); + } } } diff --git a/tests/Perspex.Styling.UnitTests/SetterTests.cs b/tests/Perspex.Styling.UnitTests/SetterTests.cs index cd327d1d3a..49fc4fcfdb 100644 --- a/tests/Perspex.Styling.UnitTests/SetterTests.cs +++ b/tests/Perspex.Styling.UnitTests/SetterTests.cs @@ -16,7 +16,8 @@ namespace Perspex.Styling.UnitTests { var control = new TextBlock(); var subject = new BehaviorSubject("foo"); - var binding = Mock.Of(x => x.CreateSubject(control, TextBlock.TextProperty, null) == subject); + var descriptor = new InstancedBinding(subject); + var binding = Mock.Of(x => x.Initiate(control, TextBlock.TextProperty, null) == descriptor); var style = Mock.Of(); var setter = new Setter(TextBlock.TextProperty, binding);