Browse Source

Fixed DataContext binding problem.

Described by XamlBindingTest.Should_Not_Write_To_Old_DataContext.
Problem still remains with ListBox.SelectedItems however, that's going
to require something more.
Steven Kirk 10 years ago
parent
commit
d2cc993a3e

+ 9 - 18
src/Markup/Perspex.Markup.Xaml/Binding/XamlBinding.cs

@@ -43,26 +43,17 @@ namespace Perspex.Markup.Xaml.Binding
             IObservablePropertyBag instance, 
             PerspexProperty property)
         {
-            IObservable<object> dataContext = null;
+            var dataContextHost = property != Control.DataContextProperty ? 
+                instance : 
+                instance.InheritanceParent as IObservablePropertyBag;
 
-            if (property != Control.DataContextProperty)
+            if (dataContextHost != null)
             {
-                dataContext = instance.GetObservable(Control.DataContextProperty);
-            }
-            else
-            {
-                var parent = instance.InheritanceParent as IObservablePropertyBag;
-
-                if (parent != null)
-                {
-                    dataContext = parent.GetObservable(Control.DataContextProperty);
-                }
-            }
-
-            if (dataContext != null)
-            {
-                var result = new ExpressionObserver(null, SourcePropertyPath);
-                dataContext.Subscribe(x => result.Root = x);
+                var result = new ExpressionObserver(
+                    () => dataContextHost.GetValue(Control.DataContextProperty),
+                    SourcePropertyPath);
+                dataContextHost.GetObservable(Control.DataContextProperty).Subscribe(x =>
+                    result.UpdateRoot());
                 return result;
             }
 

+ 27 - 26
src/Markup/Perspex.Markup/Binding/ExpressionObserver.cs

@@ -12,7 +12,7 @@ namespace Perspex.Markup.Binding
     /// </summary>
     public class ExpressionObserver : ObservableBase<object>, IDescription
     {
-        private object _root;
+        private Func<object> _root;
         private int _count;
         private ExpressionNode _node;
 
@@ -22,7 +22,20 @@ namespace Perspex.Markup.Binding
         /// <param name="root">The root object.</param>
         /// <param name="expression">The expression.</param>
         public ExpressionObserver(object root, string expression)
+            : this(() => root, expression)
         {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ExpressionObserver"/> class.
+        /// </summary>
+        /// <param name="root">A function which gets the root object.</param>
+        /// <param name="expression">The expression.</param>
+        public ExpressionObserver(Func<object> root, string expression)
+        {
+            Contract.Requires<ArgumentNullException>(root != null);
+            Contract.Requires<ArgumentNullException>(expression != null);
+
             _root = root;
             _node = ExpressionNodeBuilder.Build(expression);
             Expression = expression;
@@ -39,6 +52,7 @@ namespace Perspex.Markup.Binding
         public bool SetValue(object value)
         {
             IncrementCount();
+            UpdateRoot();
 
             try
             {
@@ -55,30 +69,6 @@ namespace Perspex.Markup.Binding
         /// </summary>
         public string Expression { get; }
 
-        /// <summary>
-        /// Gets or sets the root object that the expression is being observed on.
-        /// </summary>
-        public object Root
-        {
-            get
-            {
-                return _root;
-            }
-
-            set
-            {
-                if (_root != value)
-                {
-                    _root = value;
-
-                    if (_count > 0)
-                    {
-                        _node.Target = _root;
-                    }
-                }
-            }
-        }
-
         /// <summary>
         /// Gets the type of the expression result or null if the expression could not be 
         /// evaluated.
@@ -116,6 +106,17 @@ namespace Perspex.Markup.Binding
             }
         }
 
+        /// <summary>
+        /// Causes the root object to be re-read.
+        /// </summary>
+        public void UpdateRoot()
+        {
+            if (_count > 0)
+            {
+                _node.Target = _root();
+            }
+        }
+
         /// <inheritdoc/>
         protected override IDisposable SubscribeCore(IObserver<object> observer)
         {
@@ -134,7 +135,7 @@ namespace Perspex.Markup.Binding
         {
             if (_count++ == 0)
             {
-                _node.Target = Root;
+                _node.Target = _root();
             }
         }
 

+ 17 - 0
tests/Perspex.Base.UnitTests/PerspexObjectTests_Binding.cs

@@ -34,6 +34,20 @@ namespace Perspex.Base.UnitTests
             Assert.Equal("initial", target.GetValue(Class1.FooProperty));
         }
 
+        [Fact]
+        public void Bind_To_ValueType_Accepts_UnsetValue()
+        {
+            var target = new Class1();
+            var source = new Subject<object>();
+
+            target.Bind(Class1.QuxProperty, source);
+            source.OnNext(6.7);
+            source.OnNext(PerspexProperty.UnsetValue);
+
+            Assert.Equal(5.6, target.GetValue(Class1.QuxProperty));
+            Assert.False(target.IsSet(Class1.QuxProperty));
+        }
+
         [Fact]
         public void Bind_Throws_Exception_For_Unregistered_Property()
         {
@@ -277,6 +291,9 @@ namespace Perspex.Base.UnitTests
 
             public static readonly PerspexProperty<string> BazProperty =
                 PerspexProperty.Register<Class1, string>("Baz", "bazdefault", true);
+
+            public static readonly PerspexProperty<double> QuxProperty =
+                PerspexProperty.Register<Class1, double>("Qux", 5.6);
         }
 
         private class Class2 : Class1

+ 9 - 5
tests/Perspex.Markup.UnitTests/Binding/ExpressionObserverTests_Property.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Reactive.Linq;
+using System.Reactive.Subjects;
 using Perspex.Markup.Binding;
 using Xunit;
 
@@ -227,7 +228,7 @@ namespace Perspex.Markup.UnitTests.Binding
         [Fact]
         public async void Should_Handle_Null_Root()
         {
-            var target = new ExpressionObserver(null, "Foo");
+            var target = new ExpressionObserver((object)null, "Foo");
             var result = await target.Take(1);
 
             Assert.Equal(PerspexProperty.UnsetValue, result);
@@ -238,12 +239,15 @@ namespace Perspex.Markup.UnitTests.Binding
         {
             var first = new Class1 { Foo = "foo" };
             var second = new Class1 { Foo = "bar" };
-            var target = new ExpressionObserver(first, "Foo");
+            var root = first;
+            var target = new ExpressionObserver(() => root, "Foo");
             var result = new List<object>();
-
             var sub = target.Subscribe(x => result.Add(x));
-            target.Root = second;
-            target.Root = null;
+
+            root = second;
+            target.UpdateRoot();
+            root = null;
+            target.UpdateRoot();
 
             Assert.Equal(new[] { "foo", "bar", PerspexProperty.UnsetValue }, result);
 

+ 71 - 6
tests/Perspex.Markup.Xaml.UnitTests/XamlBindingTest.cs

@@ -1,21 +1,86 @@
 // 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 Moq;
+using System;
+using Perspex.Controls;
 using Perspex.Markup.Xaml.Binding;
-using OmniXaml.TypeConversion;
 using Xunit;
 
 namespace Perspex.Xaml.Base.UnitTest
 {
     public class XamlBindingTest
     {
+        /// <summary>
+        /// Tests a problem discovered with ListBox with selection.
+        /// </summary>
+        /// <remarks>
+        /// - Items is bound to DataContext first, followed by say SelectedIndex
+        /// - When the ListBox is removed from the visual tree, DataContext becomes null (as it's
+        ///   inherited)
+        /// - This changes Items to null, which changes SelectedIndex to null as there are no
+        ///   longer any items
+        /// - However, the news that DataContext is now null hasn't yet reached the SelectedIndex
+        ///   binding and so the unselection is sent back to the ViewModel
+        /// </remarks>
         [Fact]
-        public void TestNullDataContext()
+        public void Should_Not_Write_To_Old_DataContext()
         {
-            //var t = new Mock<ITypeConverterProvider>();
-            //var sut = new XamlBinding(t.Object);
-            //sut.BindTo(null);
+            var vm = new OldDataContextViewModel();
+            var target = new OldDataContextTest();
+
+            var fooBinding = new XamlBinding
+            {
+                SourcePropertyPath = "Foo",
+                BindingMode = BindingMode.TwoWay,
+            };
+
+            var barBinding = new XamlBinding
+            {
+                SourcePropertyPath = "Bar",
+                BindingMode = BindingMode.TwoWay,
+            };
+
+            // Bind Foo and Bar to the VM.
+            fooBinding.Bind(target, OldDataContextTest.FooProperty);
+            barBinding.Bind(target, OldDataContextTest.BarProperty);
+            target.DataContext = vm;
+
+            // Make sure the control's Foo and Bar properties are read from the VM
+            Assert.Equal(1, target.GetValue(OldDataContextTest.FooProperty));
+            Assert.Equal(2, target.GetValue(OldDataContextTest.BarProperty));
+
+            // Set DataContext to null.
+            target.DataContext = null;
+
+            // Foo and Bar are no longer bound so they return 0, their default value.
+            Assert.Equal(0, target.GetValue(OldDataContextTest.FooProperty));
+            Assert.Equal(0, target.GetValue(OldDataContextTest.BarProperty));
+
+            // The problem was here - DataContext is now null, setting Foo to 0. Bar is bound to 
+            // Foo so Bar also gets set to 0. However the Bar binding still had a reference to
+            // the VM and so vm.Bar was set to 0 erroneously.
+            Assert.Equal(1, vm.Foo);
+            Assert.Equal(2, vm.Bar);
+        }
+
+        private class OldDataContextViewModel
+        {
+            public int Foo { get; set; } = 1;
+            public int Bar { get; set; } = 2;
+        }
+
+        private class OldDataContextTest : Control
+        {
+            public static readonly PerspexProperty<int> FooProperty =
+                PerspexProperty.Register<OldDataContextTest, int>("Foo");
+
+            public static readonly PerspexProperty<int> BarProperty =
+              PerspexProperty.Register<OldDataContextTest, int>("Bar");
+
+            public OldDataContextTest()
+            {
+                Bind(BarProperty, GetObservable(FooProperty));
+            }
         }
     }
 }