diff --git a/src/Avalonia.Controls/Generators/TreeItemContainerGenerator.cs b/src/Avalonia.Controls/Generators/TreeItemContainerGenerator.cs index 9200490668b..22e06a40b1a 100644 --- a/src/Avalonia.Controls/Generators/TreeItemContainerGenerator.cs +++ b/src/Avalonia.Controls/Generators/TreeItemContainerGenerator.cs @@ -127,7 +127,7 @@ public void UpdateIndex() Index = new TreeContainerIndex(); _treeView = treeViewOwner; } - else if (Owner.IsAttachedToLogicalTree) + else { var treeView = Owner.GetSelfAndLogicalAncestors().OfType().FirstOrDefault(); @@ -138,12 +138,6 @@ public void UpdateIndex() _treeView = treeView; } } - else - { - Clear(); - Index = null; - _treeView = null; - } } class WrapperTreeDataTemplate : ITreeDataTemplate diff --git a/src/Avalonia.Controls/Primitives/VisualLayerManager.cs b/src/Avalonia.Controls/Primitives/VisualLayerManager.cs index b7229eb1211..00652f21ba7 100644 --- a/src/Avalonia.Controls/Primitives/VisualLayerManager.cs +++ b/src/Avalonia.Controls/Primitives/VisualLayerManager.cs @@ -8,7 +8,7 @@ public class VisualLayerManager : Decorator { private const int AdornerZIndex = int.MaxValue - 100; private const int OverlayZIndex = int.MaxValue - 99; - private IStyleHost _styleRoot; + private IStyleRoot _styleRoot; private readonly List _layers = new List(); @@ -53,7 +53,7 @@ void AddLayer(Control layer, int zindex) 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(); } diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index 5a8711a21c0..799e6c81d61 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -266,7 +266,7 @@ protected virtual void HandlePaint(Rect rect) /// 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); diff --git a/src/Avalonia.Styling/LogicalTree/LogicalTreeAttachmentEventArgs.cs b/src/Avalonia.Styling/LogicalTree/LogicalTreeAttachmentEventArgs.cs index 1b0eb2b61be..dc1b801a5c2 100644 --- a/src/Avalonia.Styling/LogicalTree/LogicalTreeAttachmentEventArgs.cs +++ b/src/Avalonia.Styling/LogicalTree/LogicalTreeAttachmentEventArgs.cs @@ -16,16 +16,44 @@ public class LogicalTreeAttachmentEventArgs : EventArgs /// Initializes a new instance of the class. /// /// The root of the logical tree. - public LogicalTreeAttachmentEventArgs(IStyleHost root) + /// The control being attached/detached. + /// The . + public LogicalTreeAttachmentEventArgs( + IStyleRoot root, + ILogical source, + ILogical parent) { Contract.Requires(root != null); + Contract.Requires(source != null); Root = root; + Source = source; + Parent = parent; } /// /// Gets the root of the logical tree that the control is being attached to or detached from. /// - public IStyleHost Root { get; } + public IStyleRoot Root { get; } + + /// + /// Gets the control that was attached or detached from the logical tree. + /// + /// + /// 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. + /// + public ILogical Source { get; } + + /// + /// Gets the control that is being attached to or detached from. + /// + /// + /// For logical tree attachment, holds the new logical parent of . For + /// detachment, holds the old logical parent of . If the detachment event + /// was caused by a top-level control being closed, then this property will be null. + /// + public ILogical Parent { get; } } } diff --git a/src/Avalonia.Styling/StyledElement.cs b/src/Avalonia.Styling/StyledElement.cs index b5deb9a4a19..33962b84997 100644 --- a/src/Avalonia.Styling/StyledElement.cs +++ b/src/Avalonia.Styling/StyledElement.cs @@ -59,7 +59,7 @@ public class StyledElement : Animatable, IDataContextProvider, IStyledElement, I private int _initCount; private string _name; private readonly Classes _classes = new Classes(); - private bool _isAttachedToLogicalTree; + private IStyleRoot _styleRoot; private IAvaloniaList _logicalChildren; private IResourceDictionary _resources; private Styles _styles; @@ -81,7 +81,7 @@ static StyledElement() /// public StyledElement() { - _isAttachedToLogicalTree = this is IStyleRoot; + _styleRoot = this as IStyleRoot; } /// @@ -307,7 +307,7 @@ protected IAvaloniaList LogicalChildren /// /// Gets a value indicating whether the element is attached to a rooted logical tree. /// - bool ILogical.IsAttachedToLogicalTree => _isAttachedToLogicalTree; + bool ILogical.IsAttachedToLogicalTree => _styleRoot != null; /// /// Gets the styled element's logical parent. @@ -367,7 +367,7 @@ public virtual void EndInit() throw new InvalidOperationException("BeginInit was not called."); } - if (--_initCount == 0 && _isAttachedToLogicalTree) + if (--_initCount == 0 && _styleRoot != null) { InitializeStylesIfNeeded(); @@ -435,19 +435,6 @@ void ISetLogicalParent.SetParent(ILogical parent) 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 @@ void ISetLogicalParent.SetParent(ILogical parent) 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 @@ void ISetLogicalParent.SetParent(ILogical parent) 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 @@ private static void ValidateLogicalChild(ILogical c) 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 @@ private void OnAttachedToLogicalTreeCore(LogicalTreeAttachmentEventArgs e) private void OnDetachedFromLogicalTreeCore(LogicalTreeAttachmentEventArgs e) { - if (_isAttachedToLogicalTree) + if (_styleRoot != null) { - _isAttachedToLogicalTree = false; + _styleRoot = null; _styleDetach.OnNext(this); OnDetachedFromLogicalTree(e); DetachedFromLogicalTree?.Invoke(this, e); diff --git a/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs b/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs index 72c81659c3b..91592b1ff72 100644 --- a/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs @@ -166,6 +166,31 @@ public void Impl_Close_Should_Call_Raise_Closed_Event() } } + [Fact] + public void Impl_Close_Should_Raise_DetachedFromLogicalTree_Event() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var impl = new Mock(); + 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() { diff --git a/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs b/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs index ed8a39d0635..a2892be5c25 100644 --- a/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TreeViewTests.cs @@ -971,6 +971,40 @@ public void Auto_Expanding_In_Style_Should_Not_Break_Range_Selection() } } + [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(target.Presenter.Panel.Children[0]); + Assert.Equal(3, rootNode.ItemContainerGenerator.Containers.Count()); + Assert.Equal(3, rootNode.Presenter.Panel.Children.Count); + + var child2Node = Assert.IsType(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(); diff --git a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs index c40e6c735e0..ae519ed6747 100644 --- a/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyledElementTests.cs @@ -76,6 +76,29 @@ public void InheritanceParent_Should_Be_Cleared_When_Removed_From_Parent_When_Ha 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(() => root.Child = logicalParent); + } + [Fact] public void AttachedToLogicalParent_Should_Be_Called_When_Added_To_Tree() { @@ -144,6 +167,50 @@ public void AttachedToLogicalParent_Should_Not_Be_Called_With_GlobalStyles_As_Ro 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 @@ public void DetachedFromLogicalParent_Should_Not_Be_Called_With_GlobalStyles_As_ 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() {