Fix DataContext binding problem with SelectedItems.

This one has to be solved in SelectingItemsControl as the property is
not updated via a binding.

src/Perspex.Base/PerspexObject.cs

@@ -807,18 +807,33 @@ namespace Perspex
                 newValue,
                 priority);
 
-            OnPropertyChanged(e);
-            property.NotifyChanged(e);
-
-            if (PropertyChanged != null)
+            if (property.Notifying != null)
             {
-                PropertyChanged(this, e);
+                property.Notifying(this, true);
             }
 
-            if (_inpcChanged != null)
+            try
             {
-                PropertyChangedEventArgs e2 = new PropertyChangedEventArgs(property.Name);
-                _inpcChanged(this, e2);
+                OnPropertyChanged(e);
+                property.NotifyChanged(e);
+
+                if (PropertyChanged != null)
+                {
+                    PropertyChanged(this, e);
+                }
+
+                if (_inpcChanged != null)
+                {
+                    PropertyChangedEventArgs e2 = new PropertyChangedEventArgs(property.Name);
+                    _inpcChanged(this, e2);
+                }
+            }
+            finally
+            {
+                if (property.Notifying != null)
+                {
+                    property.Notifying(this, false);
+                }
             }
         }
 

src/Perspex.Base/PerspexProperty.cs

@@ -63,6 +63,11 @@ namespace Perspex
         /// <param name="inherits">Whether the property inherits its value.</param>
         /// <param name="defaultBindingMode">The default binding mode for the property.</param>
         /// <param name="validate">A validation function.</param>
+        /// <param name="notifying">
+        /// A method that gets called before and after the property starts being notified on an
+        /// object; the bool argument will be true before and false afterwards. This callback is
+        /// intended to support IsDataContextChanging.
+        /// </param>
         /// <param name="isAttached">Whether the property is an attached property.</param>
         public PerspexProperty(
             string name,
@@ -72,6 +77,7 @@ namespace Perspex
             bool inherits = false,
             BindingMode defaultBindingMode = BindingMode.Default,
             Func<PerspexObject, object, object> validate = null,
+            Action<PerspexObject, bool> notifying = null,
             bool isAttached = false)
         {
             Contract.Requires<ArgumentNullException>(name != null);
@@ -90,6 +96,7 @@ namespace Perspex
             Inherits = inherits;
             DefaultBindingMode = defaultBindingMode;
             IsAttached = isAttached;
+            Notifying = notifying;
             _id = s_nextId++;
 
             if (validate != null)
@@ -240,6 +247,16 @@ namespace Perspex
         /// </value>
         public IObservable<PerspexPropertyChangedEventArgs> Changed => _changed;
 
+        /// <summary>
+        /// The notifying callback.
+        /// </summary>
+        /// <remarks>
+        /// This is a method that gets called before and after the property starts being notified
+        /// on an object; the bool argument will be true before and false afterwards. This 
+        /// callback is intended to support IsDataContextChanging.
+        /// </remarks>
+        public Action<PerspexObject, bool> Notifying { get; }
+
         /// <summary>
         /// Provides access to a property's binding via the <see cref="PerspexObject"/>
         /// indexer.
@@ -323,13 +340,19 @@ namespace Perspex
         /// <param name="inherits">Whether the property inherits its value.</param>
         /// <param name="defaultBindingMode">The default binding mode for the property.</param>
         /// <param name="validate">A validation function.</param>
+        /// <param name="notifying">
+        /// A method that gets called before and after the property starts being notified on an
+        /// object; the bool argument will be true before and false afterwards. This callback is
+        /// intended to support IsDataContextChanging.
+        /// </param>
         /// <returns>A <see cref="PerspexProperty{TValue}"/></returns>
         public static PerspexProperty<TValue> Register<TOwner, TValue>(
             string name,
             TValue defaultValue = default(TValue),
             bool inherits = false,
             BindingMode defaultBindingMode = BindingMode.OneWay,
-            Func<TOwner, TValue, TValue> validate = null)
+            Func<TOwner, TValue, TValue> validate = null,
+            Action<PerspexObject, bool> notifying = null)
             where TOwner : PerspexObject
         {
             Contract.Requires<ArgumentNullException>(name != null);
@@ -341,6 +364,7 @@ namespace Perspex
                 inherits,
                 defaultBindingMode,
                 Cast(validate),
+                notifying,
                 false);
 
             PerspexObject.Register(typeof(TOwner), result);
@@ -404,6 +428,7 @@ namespace Perspex
                 inherits,
                 defaultBindingMode,
                 validate,
+                null,
                 true);
 
             PerspexObject.Register(typeof(THost), result);
@@ -440,6 +465,7 @@ namespace Perspex
                 inherits,
                 defaultBindingMode,
                 validate,
+                null,
                 true);
 
             PerspexObject.Register(typeof(THost), result);

src/Perspex.Base/PerspexProperty`1.cs

@@ -20,6 +20,11 @@ namespace Perspex
         /// <param name="inherits">Whether the property inherits its value.</param>
         /// <param name="defaultBindingMode">The default binding mode for the property.</param>
         /// <param name="validate">A validation function.</param>
+        /// <param name="notifying">
+        /// A method that gets called before and after the property starts being notified on an
+        /// object; the bool argument will be true before and false afterwards. This callback is
+        /// intended to support IsDataContextChanging.
+        /// </param>
         /// <param name="isAttached">Whether the property is an attached property.</param>
         public PerspexProperty(
             string name,
@@ -28,6 +33,7 @@ namespace Perspex
             bool inherits = false,
             BindingMode defaultBindingMode = BindingMode.Default,
             Func<PerspexObject, TValue, TValue> validate = null,
+            Action<PerspexObject, bool> notifying = null,
             bool isAttached = false)
             : base(
                 name,
@@ -37,6 +43,7 @@ namespace Perspex
                 inherits,
                 defaultBindingMode,
                 Cast(validate),
+                notifying,
                 isAttached)
         {
         }

src/Perspex.Controls/Control.cs

@@ -32,7 +32,10 @@ namespace Perspex.Controls
         /// Defines the <see cref="DataContext"/> property.
         /// </summary>
         public static readonly PerspexProperty<object> DataContextProperty =
-            PerspexProperty.Register<Control, object>(nameof(DataContext), inherits: true);
+            PerspexProperty.Register<Control, object>(
+                nameof(DataContext), 
+                inherits: true,
+                notifying: DataContextNotifying);
 
         /// <summary>
         /// Defines the <see cref="FocusAdorner"/> property.
@@ -262,6 +265,16 @@ namespace Perspex.Controls
         /// </remarks>
         Type IStyleable.StyleKey => GetType();
 
+        /// <summary>
+        /// Gets a value which indicates whether a change to the <see cref="DataContext"/> is in 
+        /// the process of being notified.
+        /// </summary>
+        protected bool IsDataContextChanging
+        {
+            get;
+            private set;
+        }
+
         /// <summary>
         /// Gets the control's logical children.
         /// </summary>
@@ -396,6 +409,14 @@ namespace Perspex.Controls
             styler.ApplyStyles(this);
         }
 
+        /// <summary>
+        /// Called when the <see cref="DataContext"/> is changed and all subscribers to that change
+        /// have been notified.
+        /// </summary>
+        protected virtual void OnDataContextFinishedChanging()
+        {
+        }
+
         /// <summary>
         /// Makes the control use a different control's logical children as its own.
         /// </summary>
@@ -404,5 +425,20 @@ namespace Perspex.Controls
         {
             _logicalChildren = collection;
         }
+
+        /// <summary>
+        /// Called when the <see cref="DataContext"/> property begins and ends being notified.
+        /// </summary>
+        /// <param name="o">The object on which the DataContext is changing.</param>
+        /// <param name="notifying">Whether the notifcation is beginning or ending.</param>
+        private static void DataContextNotifying(PerspexObject o, bool notifying)
+        {
+            var control = o as Control;
+
+            if (control != null)
+            {
+                control.IsDataContextChanging = notifying;
+            }
+        }
     }
 }

src/Perspex.Controls/Primitives/SelectingItemsControl.cs

@@ -82,6 +82,7 @@ namespace Perspex.Controls.Primitives
         private object _selectedItem;
         private IList _selectedItems;
         private bool _ignoreContainerSelectionChanged;
+        private IList _clearSelectedItemsAfterDataContextChanged;
 
         /// <summary>
         /// Initializes static members of the <see cref="SelectingItemsControl"/> class.
@@ -155,7 +156,21 @@ namespace Perspex.Controls.Primitives
                     }
                     else if (SelectedItems.Count > 0)
                     {
-                        SelectedItems.Clear();
+                        if (!IsDataContextChanging)
+                        {
+                            SelectedItems.Clear();
+                        }
+                        else
+                        {
+                            // The DataContext is changing, and it's quite possible that our 
+                            // selection is being cleared because both Items and SelectedItems
+                            // are bound to something on the DataContext. However, if we clear
+                            // the collection now, we may be clearing a the SelectedItems from
+                            // the DataContext which is being unbound, so do it after DataContext
+                            // has notified all interested parties, in 
+                            // the OnDataContextFinishedChanging method.
+                            _clearSelectedItemsAfterDataContextChanged = SelectedItems;
+                        }
                     }
                 }
             }
@@ -179,12 +194,9 @@ namespace Perspex.Controls.Primitives
 
             set
             {
-                if (value != null)
-                {
-                    UnsubscribeFromSelectedItems();
-                    _selectedItems = value;
-                    SubscribeToSelectedItems();
-                }
+                UnsubscribeFromSelectedItems();
+                _selectedItems = value ?? new PerspexList<object>();
+                SubscribeToSelectedItems();
             }
         }
 
@@ -271,6 +283,16 @@ namespace Perspex.Controls.Primitives
             }
         }
 
+        protected override void OnDataContextFinishedChanging()
+        {
+            if (_clearSelectedItemsAfterDataContextChanged == SelectedItems)
+            {
+                _clearSelectedItemsAfterDataContextChanged.Clear();
+            }
+
+            _clearSelectedItemsAfterDataContextChanged = null;
+        }
+
         /// <summary>
         /// Updates the selection for an item based on user interaction.
         /// </summary>

tests/Perspex.Controls.UnitTests/Perspex.Controls.UnitTests.csproj

@@ -120,6 +120,10 @@
     <Compile Include="Utils\HotKeyManagerTests.cs" />
   </ItemGroup>
   <ItemGroup>
+    <ProjectReference Include="..\..\src\Markup\Perspex.Markup.Xaml\Perspex.Markup.Xaml.csproj">
+      <Project>{3e53a01a-b331-47f3-b828-4a5717e77a24}</Project>
+      <Name>Perspex.Markup.Xaml</Name>
+    </ProjectReference>
     <ProjectReference Include="..\..\src\Perspex.Animation\Perspex.Animation.csproj">
       <Project>{D211E587-D8BC-45B9-95A4-F297C8FA5200}</Project>
       <Name>Perspex.Animation</Name>

tests/Perspex.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs

@@ -8,6 +8,7 @@ using Perspex.Collections;
 using Perspex.Controls.Presenters;
 using Perspex.Controls.Primitives;
 using Perspex.Controls.Templates;
+using Perspex.Markup.Xaml.Binding;
 using Xunit;
 
 namespace Perspex.Controls.UnitTests.Primitives
@@ -354,8 +355,74 @@ namespace Perspex.Controls.UnitTests.Primitives
             Assert.Equal(new[] { "baz", "qux", "qiz" }, target.SelectedItems.Cast<object>().ToList());
         }
 
+        /// <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 SelectedItems
+        ///   binding and so the unselection is sent back to the ViewModel
+        /// 
+        /// This is a similar problem to that tested by XamlBindingTest.Should_Not_Write_To_Old_DataContext.
+        /// However, that tests a general property binding problem: here we are writing directly
+        /// to the SelectedItems collection - not via a binding - so it's something that the 
+        /// binding system cannot solve. Instead we solve it by not clearing SelectedItems when
+        /// DataContext is in the process of changing.
+        /// </remarks>
+        [Fact]
+        public void Should_Not_Write_To_Old_DataContext()
+        {
+            var vm = new OldDataContextViewModel();
+            var target = new TestSelector();
+
+            var itemsBinding = new XamlBinding
+            {
+                SourcePropertyPath = "Items",
+                BindingMode = BindingMode.OneWay,
+            };
+
+            var selectedItemsBinding = new XamlBinding
+            {
+                SourcePropertyPath = "SelectedItems",
+                BindingMode = BindingMode.OneWay,
+            };
+
+            // Bind Items and SelectedItems to the VM.
+            itemsBinding.Bind(target, TestSelector.ItemsProperty);
+            selectedItemsBinding.Bind(target, TestSelector.SelectedItemsProperty);
+
+            // Set DataContext and SelectedIndex
+            target.DataContext = vm;
+            target.SelectedIndex = 1;
+
+            // Make sure SelectedItems are written back to VM.
+            Assert.Equal(new[] { "bar" }, vm.SelectedItems);
+
+            // Clear DataContext and ensure that SelectedItems is still set in the VM.
+            target.DataContext = null;
+            Assert.Equal(new[] { "bar" }, vm.SelectedItems);
+        }
+
+        private ControlTemplate Template()
+        {
+            return new ControlTemplate<SelectingItemsControl>(control =>
+                new ItemsPresenter
+                {
+                    Name = "itemsPresenter",
+                    [~ItemsPresenter.ItemsProperty] = control[~ItemsControl.ItemsProperty],
+                    [~ItemsPresenter.ItemsPanelProperty] = control[~ItemsControl.ItemsPanelProperty],
+                });
+        }
+
         private class TestSelector : SelectingItemsControl
         {
+            public static readonly new PerspexProperty<IList> SelectedItemsProperty = 
+                SelectingItemsControl.SelectedItemsProperty;
+
             public new IList SelectedItems
             {
                 get { return base.SelectedItems; }
@@ -374,15 +441,16 @@ namespace Perspex.Controls.UnitTests.Primitives
             }
         }
 
-        private ControlTemplate Template()
+        private class OldDataContextViewModel
         {
-            return new ControlTemplate<SelectingItemsControl>(control =>
-                new ItemsPresenter
-                {
-                    Name = "itemsPresenter",
-                    [~ItemsPresenter.ItemsProperty] = control[~ItemsControl.ItemsProperty],
-                    [~ItemsPresenter.ItemsPanelProperty] = control[~ItemsControl.ItemsPanelProperty],
-                });
+            public OldDataContextViewModel()
+            {
+                Items = new List<string> { "foo", "bar" };
+                SelectedItems = new List<string>();
+            }
+
+            public List<string> Items { get; } 
+            public List<string> SelectedItems { get; }
         }
     }
 }