Merge pull request #3342 from AvaloniaUI/fixes/3328-move-treeview

Fix DetachedFromLogicalTree event args parent, and allow TreeViews to be reparented

src/Avalonia.Controls/Generators/TreeItemContainerGenerator.cs

@@ -127,7 +127,7 @@ namespace Avalonia.Controls.Generators
                 Index = new TreeContainerIndex();
                 _treeView = treeViewOwner;
             }
-            else if (Owner.IsAttachedToLogicalTree)
+            else
             {
                 var treeView = Owner.GetSelfAndLogicalAncestors().OfType<TreeView>().FirstOrDefault();
                 
@@ -138,12 +138,6 @@ namespace Avalonia.Controls.Generators
                     _treeView = treeView;
                 }
             }
-            else
-            {
-                Clear();
-                Index = null;
-                _treeView = null;
-            }
         }
 
         class WrapperTreeDataTemplate : ITreeDataTemplate

src/Avalonia.Controls/Primitives/VisualLayerManager.cs

@@ -8,7 +8,7 @@ namespace Avalonia.Controls.Primitives
     {
         private const int AdornerZIndex = int.MaxValue - 100;
         private const int OverlayZIndex = int.MaxValue - 99;
-        private IStyleHost _styleRoot;
+        private IStyleRoot _styleRoot;
         private readonly List<Control> _layers = new List<Control>();
         
 
@@ -53,7 +53,7 @@ namespace Avalonia.Controls.Primitives
             layer.ZIndex = zindex;
             VisualChildren.Add(layer);
             if (((ILogical)this).IsAttachedToLogicalTree)
-                ((ILogical)layer).NotifyAttachedToLogicalTree(new LogicalTreeAttachmentEventArgs(_styleRoot));
+                ((ILogical)layer).NotifyAttachedToLogicalTree(new LogicalTreeAttachmentEventArgs(_styleRoot, layer, this));
             InvalidateArrange();
         }
         

src/Avalonia.Controls/TopLevel.cs

@@ -266,7 +266,7 @@ namespace Avalonia.Controls
         /// </summary>
         protected virtual void HandleClosed()
         {
-            var logicalArgs = new LogicalTreeAttachmentEventArgs(this);
+            var logicalArgs = new LogicalTreeAttachmentEventArgs(this, this, null);
             ((ILogical)this).NotifyDetachedFromLogicalTree(logicalArgs);
 
             var visualArgs = new VisualTreeAttachmentEventArgs(this, this);

src/Avalonia.Styling/LogicalTree/LogicalTreeAttachmentEventArgs.cs

@@ -16,16 +16,44 @@ namespace Avalonia.LogicalTree
         /// Initializes a new instance of the <see cref="LogicalTreeAttachmentEventArgs"/> class.
         /// </summary>
         /// <param name="root">The root of the logical tree.</param>
-        public LogicalTreeAttachmentEventArgs(IStyleHost root)
+        /// <param name="source">The control being attached/detached.</param>
+        /// <param name="parent">The <see cref="Parent"/>.</param>
+        public LogicalTreeAttachmentEventArgs(
+            IStyleRoot root,
+            ILogical source,
+            ILogical parent)
         {
             Contract.Requires<ArgumentNullException>(root != null);
+            Contract.Requires<ArgumentNullException>(source != null);
 
             Root = root;
+            Source = source;
+            Parent = parent;
         }
 
         /// <summary>
         /// Gets the root of the logical tree that the control is being attached to or detached from.
         /// </summary>
-        public IStyleHost Root { get; }
+        public IStyleRoot Root { get; }
+
+        /// <summary>
+        /// Gets the control that was attached or detached from the logical tree.
+        /// </summary>
+        /// <remarks>
+        /// Logical tree attachment events travel down the attached logical tree from the point of
+        /// attachment/detachment, so this control may be different from the control that the
+        /// event is being raised on.
+        /// </remarks>
+        public ILogical Source { get; }
+
+        /// <summary>
+        /// Gets the control that <see cref="Source"/> is being attached to or detached from.
+        /// </summary>
+        /// <remarks>
+        /// For logical tree attachment, holds the new logical parent of <see cref="Source"/>. For
+        /// detachment, holds the old logical parent of <see cref="Source"/>. If the detachment event
+        /// was caused by a top-level control being closed, then this property will be null.
+        /// </remarks>
+        public ILogical Parent { get; }
     }
 }

src/Avalonia.Styling/StyledElement.cs

@@ -59,7 +59,7 @@ namespace Avalonia
         private int _initCount;
         private string _name;
         private readonly Classes _classes = new Classes();
-        private bool _isAttachedToLogicalTree;
+        private IStyleRoot _styleRoot;
         private IAvaloniaList<ILogical> _logicalChildren;
         private IResourceDictionary _resources;
         private Styles _styles;
@@ -81,7 +81,7 @@ namespace Avalonia
         /// </summary>
         public StyledElement()
         {
-            _isAttachedToLogicalTree = this is IStyleRoot;
+            _styleRoot = this as IStyleRoot;
         }
 
         /// <summary>
@@ -307,7 +307,7 @@ namespace Avalonia
         /// <summary>
         /// Gets a value indicating whether the element is attached to a rooted logical tree.
         /// </summary>
-        bool ILogical.IsAttachedToLogicalTree => _isAttachedToLogicalTree;
+        bool ILogical.IsAttachedToLogicalTree => _styleRoot != null;
 
         /// <summary>
         /// Gets the styled element's logical parent.
@@ -367,7 +367,7 @@ namespace Avalonia
                 throw new InvalidOperationException("BeginInit was not called.");
             }
 
-            if (--_initCount == 0 && _isAttachedToLogicalTree)
+            if (--_initCount == 0 && _styleRoot != null)
             {
                 InitializeStylesIfNeeded();
 
@@ -435,19 +435,6 @@ namespace Avalonia
                     throw new InvalidOperationException("The Control already has a parent.");
                 }
 
-                if (_isAttachedToLogicalTree)
-                {
-                    var oldRoot = FindStyleRoot(old) ?? this as IStyleRoot;
-
-                    if (oldRoot == null)
-                    {
-                        throw new AvaloniaInternalException("Was attached to logical tree but cannot find root.");
-                    }
-
-                    var e = new LogicalTreeAttachmentEventArgs(oldRoot);
-                    OnDetachedFromLogicalTreeCore(e);
-                }
-
                 if (InheritanceParent == null || parent == null)
                 {
                     InheritanceParent = parent as AvaloniaObject;
@@ -455,6 +442,12 @@ namespace Avalonia
 
                 Parent = (IStyledElement)parent;
 
+                if (_styleRoot != null)
+                {
+                    var e = new LogicalTreeAttachmentEventArgs(_styleRoot, this, old);
+                    OnDetachedFromLogicalTreeCore(e);
+                }
+
                 if (old != null)
                 {
                     old.ResourcesChanged -= ThisResourcesChanged;
@@ -474,7 +467,7 @@ namespace Avalonia
                         throw new AvaloniaInternalException("Parent is attached to logical tree but cannot find root.");
                     }
 
-                    var e = new LogicalTreeAttachmentEventArgs(newRoot);
+                    var e = new LogicalTreeAttachmentEventArgs(newRoot, this, parent);
                     OnAttachedToLogicalTreeCore(e);
                 }
 
@@ -704,15 +697,21 @@ namespace Avalonia
 
         private void OnAttachedToLogicalTreeCore(LogicalTreeAttachmentEventArgs e)
         {
+            if (this.GetLogicalParent() == null && !(this is IStyleRoot))
+            {
+                throw new InvalidOperationException(
+                    $"AttachedToLogicalTreeCore called for '{GetType().Name}' but control has no logical parent.");
+            }
+
             // This method can be called when a control is already attached to the logical tree
             // in the following scenario:
             // - ListBox gets assigned Items containing ListBoxItem
             // - ListBox makes ListBoxItem a logical child
             // - ListBox template gets applied; making its Panel get attached to logical tree
             // - That AttachedToLogicalTree signal travels down to the ListBoxItem
-            if (!_isAttachedToLogicalTree)
+            if (_styleRoot == null)
             {
-                _isAttachedToLogicalTree = true;
+                _styleRoot = e.Root;
 
                 InitializeStylesIfNeeded(true);
 
@@ -728,9 +727,9 @@ namespace Avalonia
 
         private void OnDetachedFromLogicalTreeCore(LogicalTreeAttachmentEventArgs e)
         {
-            if (_isAttachedToLogicalTree)
+            if (_styleRoot != null)
             {
-                _isAttachedToLogicalTree = false;
+                _styleRoot = null;
                 _styleDetach.OnNext(this);
                 OnDetachedFromLogicalTree(e);
                 DetachedFromLogicalTree?.Invoke(this, e);

tests/Avalonia.Controls.UnitTests/TopLevelTests.cs

@@ -166,6 +166,31 @@ namespace Avalonia.Controls.UnitTests
             }
         }
 
+        [Fact]
+        public void Impl_Close_Should_Raise_DetachedFromLogicalTree_Event()
+        {
+            using (UnitTestApplication.Start(TestServices.StyledWindow))
+            {
+                var impl = new Mock<ITopLevelImpl>();
+                impl.SetupAllProperties();
+
+                var target = new TestTopLevel(impl.Object);
+                var raised = 0;
+
+                target.DetachedFromLogicalTree += (s, e) =>
+                {
+                    Assert.Same(target, e.Root);
+                    Assert.Same(target, e.Source);
+                    Assert.Null(e.Parent);
+                    ++raised;
+                };
+
+                impl.Object.Closed();
+
+                Assert.Equal(1, raised);
+            }
+        }
+
         [Fact]
         public void Impl_Input_Should_Pass_Input_To_InputManager()
         {

tests/Avalonia.Controls.UnitTests/TreeViewTests.cs

@@ -971,6 +971,40 @@ namespace Avalonia.Controls.UnitTests
             }
         }
 
+        [Fact]
+        public void Removing_TreeView_From_Root_Should_Preserve_TreeViewItems()
+        {
+            // Issue #3328
+            var tree = CreateTestTreeData();
+            var target = new TreeView
+            {
+                Template = CreateTreeViewTemplate(),
+                Items = tree,
+            };
+
+            var root = new TestRoot();
+            root.Child = target;
+
+            CreateNodeDataTemplate(target);
+            ApplyTemplates(target);
+            ExpandAll(target);
+
+            Assert.Equal(5, target.ItemContainerGenerator.Index.Containers.Count());
+
+            root.Child = null;
+
+            Assert.Equal(5, target.ItemContainerGenerator.Index.Containers.Count());
+            Assert.Equal(1, target.Presenter.Panel.Children.Count);
+
+            var rootNode = Assert.IsType<TreeViewItem>(target.Presenter.Panel.Children[0]);
+            Assert.Equal(3, rootNode.ItemContainerGenerator.Containers.Count());
+            Assert.Equal(3, rootNode.Presenter.Panel.Children.Count);
+
+            var child2Node = Assert.IsType<TreeViewItem>(rootNode.Presenter.Panel.Children[1]);
+            Assert.Equal(1, child2Node.ItemContainerGenerator.Containers.Count());
+            Assert.Equal(1, child2Node.Presenter.Panel.Children.Count);
+        }
+
         private void ApplyTemplates(TreeView tree)
         {
             tree.ApplyTemplate();

tests/Avalonia.Styling.UnitTests/StyledElementTests.cs

@@ -76,6 +76,29 @@ namespace Avalonia.Styling.UnitTests
             Assert.Null(target.InheritanceParent);
         }
 
+        [Fact]
+        public void Adding_Element_With_Null_Parent_To_Logical_Tree_Should_Throw()
+        {
+            var target = new Border();
+            var visualParent = new Panel();
+            var logicalParent = new Panel();
+            var root = new TestRoot();
+
+            // Set the logical parent...
+            ((ISetLogicalParent)target).SetParent(logicalParent);
+
+            // ...so that when it's added to `visualParent`, the parent won't be set again.
+            visualParent.Children.Add(target);
+
+            // Clear the logical parent. It's now a logical child of `visualParent` but doesn't have
+            // a logical parent itself.
+            ((ISetLogicalParent)target).SetParent(null);
+
+            // In this case, attaching the control to a logical tree should throw.
+            logicalParent.Children.Add(visualParent);
+            Assert.Throws<InvalidOperationException>(() => root.Child = logicalParent);
+        }
+
         [Fact]
         public void AttachedToLogicalParent_Should_Be_Called_When_Added_To_Tree()
         {
@@ -144,6 +167,50 @@ namespace Avalonia.Styling.UnitTests
             Assert.True(raised);
         }
 
+        [Fact]
+        public void AttachedToLogicalParent_Should_Have_Source_Set()
+        {
+            var root = new TestRoot();
+            var canvas = new Canvas();
+            var border = new Border { Child = canvas };
+            var raised = 0;
+
+            void Attached(object sender, LogicalTreeAttachmentEventArgs e)
+            {
+                Assert.Same(border, e.Source);
+                ++raised;
+            }
+
+            border.AttachedToLogicalTree += Attached;
+            canvas.AttachedToLogicalTree += Attached;
+
+            root.Child = border;
+
+            Assert.Equal(2, raised);
+        }
+
+        [Fact]
+        public void AttachedToLogicalParent_Should_Have_Parent_Set()
+        {
+            var root = new TestRoot();
+            var canvas = new Canvas();
+            var border = new Border { Child = canvas };
+            var raised = 0;
+
+            void Attached(object sender, LogicalTreeAttachmentEventArgs e)
+            {
+                Assert.Same(root, e.Parent);
+                ++raised;
+            }
+
+            border.AttachedToLogicalTree += Attached;
+            canvas.AttachedToLogicalTree += Attached;
+
+            root.Child = border;
+
+            Assert.Equal(2, raised);
+        }
+
         [Fact]
         public void DetachedFromLogicalParent_Should_Be_Called_When_Removed_From_Tree()
         {
@@ -190,6 +257,25 @@ namespace Avalonia.Styling.UnitTests
             Assert.True(raised);
         }
 
+        [Fact]
+        public void Parent_Should_Be_Null_When_DetachedFromLogicalParent_Called()
+        {
+            var target = new TestControl();
+            var root = new TestRoot(target);
+            var called = 0;
+
+            target.DetachedFromLogicalTree += (s, e) =>
+            {
+                Assert.Null(target.Parent);
+                Assert.Null(target.InheritanceParent);
+                ++called;
+            };
+
+            root.Child = null;
+
+            Assert.Equal(1, called);
+        }
+
         [Fact]
         public void Adding_Tree_To_IStyleRoot_Should_Style_Controls()
         {