From 82483d7dec6e906c4764c6123133648049ccd932 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 21 Jun 2017 09:57:39 +0200 Subject: [PATCH 1/3] Fix exception in ToolTip. Make sure old `ToolTip` is disposed before showing a new one. --- src/Avalonia.Controls/ToolTip.cs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Avalonia.Controls/ToolTip.cs b/src/Avalonia.Controls/ToolTip.cs index b8896a3acf..fef10d3510 100644 --- a/src/Avalonia.Controls/ToolTip.cs +++ b/src/Avalonia.Controls/ToolTip.cs @@ -105,13 +105,10 @@ namespace Avalonia.Controls { if (control != null && control.IsVisible && control.GetVisualRoot() != null) { - if (s_popup != null) - { - throw new AvaloniaInternalException("Previous ToolTip not disposed."); - } var cp = (control.GetVisualRoot() as IInputRoot)?.MouseDevice?.GetPosition(control); var position = control.PointToScreen(cp ?? new Point(0, 0)) + new Vector(0, 22); + DisposeTooltip(); s_popup = new PopupRoot(); ((ISetLogicalParent)s_popup).SetParent(control); s_popup.Content = new ToolTip { Content = GetTip(control) }; @@ -144,18 +141,22 @@ namespace Avalonia.Controls if (control == s_current) { - if (s_popup != null) - { - // Clear the ToolTip's Content in case it has control content: this will - // reset its visual parent allowing it to be used again. - ((ToolTip)s_popup.Content).Content = null; + DisposeTooltip(); + s_show.OnNext(null); + } + } - // Dispose of the popup. - s_popup.Dispose(); - s_popup = null; - } + private static void DisposeTooltip() + { + if (s_popup != null) + { + // Clear the ToolTip's Content in case it has control content: this will + // reset its visual parent allowing it to be used again. + ((ToolTip)s_popup.Content).Content = null; - s_show.OnNext(null); + // Dispose of the popup. + s_popup.Dispose(); + s_popup = null; } } } From a232b137b5d7aa2605431c65f33d9e8d9d8cf130 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 24 Jun 2017 12:40:41 +0200 Subject: [PATCH 2/3] Allow reuse of existing tooltip popup. To do this, had to fix a problem where templated children weren't notified of being re-attached to a logical tree. --- src/Avalonia.Controls/Control.cs | 11 +- .../Primitives/TemplatedControl.cs | 11 ++ src/Avalonia.Controls/ToolTip.cs | 21 +++- src/Avalonia.Styling/LogicalTree/ILogical.cs | 10 ++ .../Primitives/PopupRootTests.cs | 109 ++++++++++++++++++ .../Primitives/TemplatedControlTests.cs | 28 +++++ .../TopLevelTests.cs | 19 ++- .../SelectorTests_Child.cs | 5 + 8 files changed, 202 insertions(+), 12 deletions(-) create mode 100644 tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs diff --git a/src/Avalonia.Controls/Control.cs b/src/Avalonia.Controls/Control.cs index 758f7bbf55..83a76cb1a7 100644 --- a/src/Avalonia.Controls/Control.cs +++ b/src/Avalonia.Controls/Control.cs @@ -118,6 +118,7 @@ namespace Avalonia.Controls public Control() { _nameScope = this as INameScope; + _isAttachedToLogicalTree = this is IStyleRoot; } /// @@ -369,6 +370,12 @@ namespace Avalonia.Controls } } + /// + void ILogical.NotifyAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) + { + this.OnAttachedToLogicalTreeCore(e); + } + /// void ILogical.NotifyDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { @@ -418,7 +425,7 @@ namespace Avalonia.Controls if (_isAttachedToLogicalTree) { - var oldRoot = FindStyleRoot(old); + var oldRoot = FindStyleRoot(old) ?? this as IStyleRoot; if (oldRoot == null) { @@ -436,7 +443,7 @@ namespace Avalonia.Controls _parent = (IControl)parent; - if (_parent is IStyleRoot || _parent?.IsAttachedToLogicalTree == true) + if (_parent is IStyleRoot || _parent?.IsAttachedToLogicalTree == true || this is IStyleRoot) { var newRoot = FindStyleRoot(this); diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index 7a42c48053..1ddfb97c14 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -285,6 +285,17 @@ namespace Avalonia.Controls.Primitives return this; } + /// + protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) + { + if (VisualChildren.Count > 0) + { + ((ILogical)VisualChildren[0]).NotifyAttachedToLogicalTree(e); + } + + base.OnAttachedToLogicalTree(e); + } + /// protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { diff --git a/src/Avalonia.Controls/ToolTip.cs b/src/Avalonia.Controls/ToolTip.cs index fef10d3510..22bc589a36 100644 --- a/src/Avalonia.Controls/ToolTip.cs +++ b/src/Avalonia.Controls/ToolTip.cs @@ -108,10 +108,18 @@ namespace Avalonia.Controls var cp = (control.GetVisualRoot() as IInputRoot)?.MouseDevice?.GetPosition(control); var position = control.PointToScreen(cp ?? new Point(0, 0)) + new Vector(0, 22); - DisposeTooltip(); - s_popup = new PopupRoot(); + if (s_popup == null) + { + s_popup = new PopupRoot(); + s_popup.Content = new ToolTip(); + } + else + { + ((ISetLogicalParent)s_popup).SetParent(null); + } + ((ISetLogicalParent)s_popup).SetParent(control); - s_popup.Content = new ToolTip { Content = GetTip(control) }; + ((ToolTip)s_popup.Content).Content = GetTip(control); s_popup.Position = position; s_popup.Show(); @@ -141,8 +149,11 @@ namespace Avalonia.Controls if (control == s_current) { - DisposeTooltip(); - s_show.OnNext(null); + if (s_popup != null) + { + DisposeTooltip(); + s_show.OnNext(null); + } } } diff --git a/src/Avalonia.Styling/LogicalTree/ILogical.cs b/src/Avalonia.Styling/LogicalTree/ILogical.cs index f2291b42e9..006a9f5cc1 100644 --- a/src/Avalonia.Styling/LogicalTree/ILogical.cs +++ b/src/Avalonia.Styling/LogicalTree/ILogical.cs @@ -36,6 +36,16 @@ namespace Avalonia.LogicalTree /// IAvaloniaReadOnlyList LogicalChildren { get; } + /// + /// Notifies the control that it is being attached to a rooted logical tree. + /// + /// The event args. + /// + /// This method will be called automatically by the framework, you should not need to call + /// this method yourself. + /// + void NotifyAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e); + /// /// Notifies the control that it is being detached from a rooted logical tree. /// diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs new file mode 100644 index 0000000000..64344a1584 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupRootTests.cs @@ -0,0 +1,109 @@ +// Copyright (c) The Avalonia Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using Avalonia.Controls.Presenters; +using Avalonia.Controls.Primitives; +using Avalonia.Controls.Templates; +using Avalonia.LogicalTree; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Controls.UnitTests.Primitives +{ + public class PopupRootTests + { + [Fact] + public void PopupRoot_IsAttachedToLogicalTree_Is_True() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var target = CreateTarget(); + + Assert.True(((ILogical)target).IsAttachedToLogicalTree); + } + } + + [Fact] + public void Templated_Child_IsAttachedToLogicalTree_Is_True() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var target = CreateTarget(); + + Assert.True(target.Presenter.IsAttachedToLogicalTree); + } + } + + [Fact] + public void Attaching_PopupRoot_To_Parent_Logical_Tree_Raises_DetachedFromLogicalTree_And_AttachedToLogicalTree() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var child = new Decorator(); + var target = CreateTarget(); + var window = new Window(); + var detachedCount = 0; + var attachedCount = 0; + + target.Content = child; + + target.DetachedFromLogicalTree += (s, e) => ++detachedCount; + child.DetachedFromLogicalTree += (s, e) => ++detachedCount; + target.AttachedToLogicalTree += (s, e) => ++attachedCount; + child.AttachedToLogicalTree += (s, e) => ++attachedCount; + + ((ISetLogicalParent)target).SetParent(window); + + Assert.Equal(2, detachedCount); + Assert.Equal(2, attachedCount); + } + } + + [Fact] + public void Detaching_PopupRoot_From_Parent_Logical_Tree_Raises_DetachedFromLogicalTree_And_AttachedToLogicalTree() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var child = new Decorator(); + var target = CreateTarget(); + var window = new Window(); + var detachedCount = 0; + var attachedCount = 0; + + target.Content = child; + ((ISetLogicalParent)target).SetParent(window); + + target.DetachedFromLogicalTree += (s, e) => ++detachedCount; + child.DetachedFromLogicalTree += (s, e) => ++detachedCount; + target.AttachedToLogicalTree += (s, e) => ++attachedCount; + child.AttachedToLogicalTree += (s, e) => ++attachedCount; + + ((ISetLogicalParent)target).SetParent(null); + + // Despite being detached from the parent logical tree, we're still attached to a + // logical tree as PopupRoot itself is a logical tree root. + Assert.True(((ILogical)target).IsAttachedToLogicalTree); + Assert.True(((ILogical)child).IsAttachedToLogicalTree); + Assert.Equal(2, detachedCount); + Assert.Equal(2, attachedCount); + } + } + + private PopupRoot CreateTarget() + { + var result = new PopupRoot + { + Template = new FuncControlTemplate(_ => + new ContentPresenter + { + Name = "PART_ContentPresenter", + }), + }; + + result.ApplyTemplate(); + + return result; + } + } +} diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs index 636492ed1c..72c8073f21 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/TemplatedControlTests.cs @@ -527,6 +527,34 @@ namespace Avalonia.Controls.UnitTests.Primitives } } + [Fact] + public void Moving_To_New_LogicalTree_Should_Detach_Attach_Template_Child() + { + using (UnitTestApplication.Start(TestServices.RealStyler)) + { + TestTemplatedControl target; + var root = new TestRoot + { + Child = target = new TestTemplatedControl + { + Template = new FuncControlTemplate(_ => new Decorator()), + } + }; + + Assert.NotNull(target.Template); + target.ApplyTemplate(); + + var templateChild = (ILogical)target.GetVisualChildren().Single(); + Assert.True(templateChild.IsAttachedToLogicalTree); + + root.Child = null; + Assert.False(templateChild.IsAttachedToLogicalTree); + + var newRoot = new TestRoot { Child = target }; + Assert.True(templateChild.IsAttachedToLogicalTree); + } + } + private static IControl ScrollingContentControlTemplate(ContentControl control) { return new Border diff --git a/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs b/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs index 5cd3c57e2e..da30336be6 100644 --- a/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs @@ -2,24 +2,33 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive; -using System.Reactive.Subjects; -using Moq; using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; using Avalonia.Input; using Avalonia.Input.Raw; using Avalonia.Layout; +using Avalonia.LogicalTree; using Avalonia.Platform; -using Avalonia.Rendering; -using Avalonia.Styling; using Avalonia.UnitTests; +using Moq; using Xunit; namespace Avalonia.Controls.UnitTests { public class TopLevelTests { + [Fact] + public void IsAttachedToLogicalTree_Is_True() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var impl = new Mock(); + var target = new TestTopLevel(impl.Object); + + Assert.True(((ILogical)target).IsAttachedToLogicalTree); + } + } + [Fact] public void ClientSize_Should_Be_Set_On_Construction() { diff --git a/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs b/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs index d97cc74c95..b40c66e061 100644 --- a/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs +++ b/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs @@ -144,6 +144,11 @@ namespace Avalonia.Styling.UnitTests throw new NotImplementedException(); } + public void NotifyAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) + { + throw new NotImplementedException(); + } + public void NotifyDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { throw new NotImplementedException(); From af50118162714e3e12523a8582faef71db9a13f9 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 24 Jun 2017 13:11:07 +0200 Subject: [PATCH 3/3] Added missing method. Why didn't you notice that before, VS? --- tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs b/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs index b4c284e7c9..7cf8c3dd1c 100644 --- a/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs +++ b/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs @@ -175,6 +175,11 @@ namespace Avalonia.Styling.UnitTests throw new NotImplementedException(); } + public void NotifyAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) + { + throw new NotImplementedException(); + } + public void NotifyDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { throw new NotImplementedException();