Merge pull request #1032 from AvaloniaUI/fixes/787-contentpresenter-updatechild

Refactored ContentPresenter

src/Avalonia.Controls/Presenters/ContentPresenter.cs

@@ -8,6 +8,7 @@ using Avalonia.Controls.Templates;
 using Avalonia.Layout;
 using Avalonia.LogicalTree;
 using Avalonia.Media;
+using Avalonia.VisualTree;
 
 namespace Avalonia.Controls.Presenters
 {
@@ -88,6 +89,7 @@ namespace Avalonia.Controls.Presenters
         static ContentPresenter()
         {
             ContentProperty.Changed.AddClassHandler<ContentPresenter>(x => x.ContentChanged);
+            ContentTemplateProperty.Changed.AddClassHandler<ContentPresenter>(x => x.ContentChanged);
             TemplatedParentProperty.Changed.AddClassHandler<ContentPresenter>(x => x.TemplatedParentChanged);
         }
 
@@ -313,27 +315,22 @@ namespace Avalonia.Controls.Presenters
 
             if (content != null && newChild == null)
             {
-                // We have content and it isn't a control, so first try to recycle the existing
-                // child control to display the new data by querying if the template that created
-                // the child can recycle items and that it also matches the new data.
-                if (oldChild != null &&
-                    _dataTemplate != null &&
-                    _dataTemplate.SupportsRecycling &&
-                    _dataTemplate.Match(content))
+                var dataTemplate = this.FindDataTemplate(content, ContentTemplate) ?? FuncDataTemplate.Default;
+
+                // We have content and it isn't a control, so if the new data template is the same
+                // as the old data template, try to recycle the existing child control to display
+                // the new data.
+                if (dataTemplate == _dataTemplate && dataTemplate.SupportsRecycling)
                 {
                     newChild = oldChild;
                 }
                 else
                 {
-                    // We couldn't recycle an existing control so find a data template for the data
-                    // and use it to create a control.
-                    _dataTemplate = this.FindDataTemplate(content, ContentTemplate) ?? FuncDataTemplate.Default;
+                    _dataTemplate = dataTemplate;
                     newChild = _dataTemplate.Build(content);
 
-                    // Try to give the new control its own name scope.
-                    var controlResult = newChild as Control;
-
-                    if (controlResult != null)
+                    // Give the new control its own name scope.
+                    if (newChild is Control controlResult)
                     {
                         NameScope.SetNameScope(controlResult, new NameScope());
                     }
@@ -424,6 +421,19 @@ namespace Avalonia.Controls.Presenters
         private void ContentChanged(AvaloniaPropertyChangedEventArgs e)
         {
             _createdChild = false;
+
+            if (((ILogical)this).IsAttachedToLogicalTree)
+            {
+                UpdateChild();
+            }
+            else if (Child != null)
+            {
+                VisualChildren.Remove(Child);
+                LogicalChildren.Remove(Child);
+                Child = null;
+                _dataTemplate = null;
+            }
+
             InvalidateMeasure();
         }
 

tests/Avalonia.Controls.UnitTests/Avalonia.Controls.UnitTests.csproj

@@ -8,6 +8,9 @@
   <Import Project="..\..\build\XUnit.props" />
   <Import Project="..\..\build\Rx.props" />
   <Import Project="..\..\build\Microsoft.Reactive.Testing.props" />
+  <ItemGroup>
+    <PackageReference Include="System.ValueTuple" Version="4.3.1" />
+  </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\src\Markup\Avalonia.Markup.Xaml\Avalonia.Markup.Xaml.csproj" />
     <ProjectReference Include="..\..\src\Markup\Avalonia.Markup\Avalonia.Markup.csproj" />

tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_InTemplate.cs

@@ -0,0 +1,291 @@
+// 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.Linq;
+using Avalonia.Controls.Presenters;
+using Avalonia.Controls.Templates;
+using Avalonia.LogicalTree;
+using Avalonia.UnitTests;
+using Avalonia.VisualTree;
+using Moq;
+using Xunit;
+
+namespace Avalonia.Controls.UnitTests.Presenters
+{
+    /// <summary>
+    /// Tests for ContentControls that are hosted in a control template.
+    /// </summary>
+    public class ContentPresenterTests_InTemplate
+    {
+        [Fact]
+        public void Should_Register_With_Host_When_TemplatedParent_Set()
+        {
+            var host = new Mock<IContentPresenterHost>();
+            var target = new ContentPresenter();
+
+            target.SetValue(Control.TemplatedParentProperty, host.Object);
+
+            host.Verify(x => x.RegisterContentPresenter(target));
+        }
+
+        [Fact]
+        public void Setting_Content_To_Control_Should_Set_Child()
+        {
+            var (target, _) = CreateTarget();
+            var child = new Border();
+
+            target.Content = child;
+
+            Assert.Equal(child, target.Child);
+        }
+
+        [Fact]
+        public void Setting_Content_To_Control_Should_Update_Logical_Tree()
+        {
+            var (target, parent) = CreateTarget();
+            var child = new Border();
+
+            target.Content = child;
+
+            Assert.Equal(parent, child.GetLogicalParent());
+            Assert.Equal(new[] { child }, parent.GetLogicalChildren());
+        }
+
+        [Fact]
+        public void Setting_Content_To_Control_Should_Update_Visual_Tree()
+        {
+            var (target, _) = CreateTarget();
+            var child = new Border();
+
+            target.Content = child;
+
+            Assert.Equal(target, child.GetVisualParent());
+            Assert.Equal(new[] { child }, target.GetVisualChildren());
+        }
+
+        [Fact]
+        public void Setting_Content_To_String_Should_Create_TextBlock()
+        {
+            var (target, _) = CreateTarget();
+
+            target.Content = "Foo";
+
+            Assert.IsType<TextBlock>(target.Child);
+            Assert.Equal("Foo", ((TextBlock)target.Child).Text);
+        }
+
+        [Fact]
+        public void Setting_Content_To_String_Should_Update_Logical_Tree()
+        {
+            var (target, parent) = CreateTarget();
+
+            target.Content = "Foo";
+
+            var child = target.Child;
+            Assert.Equal(parent, child.GetLogicalParent());
+            Assert.Equal(new[] { child }, parent.GetLogicalChildren());
+        }
+
+        [Fact]
+        public void Setting_Content_To_String_Should_Update_Visual_Tree()
+        {
+            var (target, _) = CreateTarget();
+
+            target.Content = "Foo";
+
+            var child = target.Child;
+            Assert.Equal(target, child.GetVisualParent());
+            Assert.Equal(new[] { child }, target.GetVisualChildren());
+        }
+
+        [Fact]
+        public void Clearing_Control_Content_Should_Update_Logical_Tree()
+        {
+            var (target, _) = CreateTarget();
+            var child = new Border();
+
+            target.Content = child;
+            target.Content = null;
+
+            Assert.Equal(null, child.GetLogicalParent());
+            Assert.Empty(target.GetLogicalChildren());
+        }
+
+        [Fact]
+        public void Clearing_Control_Content_Should_Update_Visual_Tree()
+        {
+            var (target, _) = CreateTarget();
+            var child = new Border();
+
+            target.Content = child;
+            target.Content = null;
+
+            Assert.Equal(null, child.GetVisualParent());
+            Assert.Empty(target.GetVisualChildren());
+        }
+
+        [Fact]
+        public void Control_Content_Should_Not_Be_NameScope()
+        {
+            var (target, _) = CreateTarget();
+
+            target.Content = new TextBlock();
+
+            Assert.IsType<TextBlock>(target.Child);
+            Assert.Null(NameScope.GetNameScope((Control)target.Child));
+        }
+
+        [Fact]
+        public void DataTemplate_Created_Control_Should_Be_NameScope()
+        {
+            var (target, _) = CreateTarget();
+
+            target.Content = "Foo";
+
+            Assert.IsType<TextBlock>(target.Child);
+            Assert.NotNull(NameScope.GetNameScope((Control)target.Child));
+        }
+
+        [Fact]
+        public void Assigning_Control_To_Content_Should_Not_Set_DataContext()
+        {
+            var (target, _) = CreateTarget();
+            target.Content = new Border();
+
+            Assert.False(target.IsSet(Control.DataContextProperty));
+        }
+
+        [Fact]
+        public void Assigning_NonControl_To_Content_Should_Set_DataContext_On_UpdateChild()
+        {
+            var (target, _) = CreateTarget();
+            target.Content = "foo";
+
+            Assert.Equal("foo", target.DataContext);
+        }
+
+        [Fact]
+        public void Should_Use_ContentTemplate_If_Specified()
+        {
+            var (target, _) = CreateTarget();
+
+            target.ContentTemplate = new FuncDataTemplate<string>(_ => new Canvas());
+            target.Content = "Foo";
+
+            Assert.IsType<Canvas>(target.Child);
+        }
+
+        [Fact]
+        public void Should_Update_If_ContentTemplate_Changed()
+        {
+            var (target, _) = CreateTarget();
+
+            target.Content = "Foo";
+            Assert.IsType<TextBlock>(target.Child);
+
+            target.ContentTemplate = new FuncDataTemplate<string>(_ => new Canvas());
+            Assert.IsType<Canvas>(target.Child);
+
+            target.ContentTemplate = null;
+            Assert.IsType<TextBlock>(target.Child);
+        }
+
+        [Fact]
+        public void Assigning_Control_To_Content_After_NonControl_Should_Clear_DataContext()
+        {
+            var (target, _) = CreateTarget();
+
+            target.Content = "foo";
+
+            Assert.True(target.IsSet(Control.DataContextProperty));
+
+            target.Content = new Border();
+
+            Assert.False(target.IsSet(Control.DataContextProperty));
+        }
+
+        [Fact]
+        public void Recycles_DataTemplate()
+        {
+            var (target, _) = CreateTarget();
+            target.DataTemplates.Add(new FuncDataTemplate<string>(_ => new Border(), true));
+
+            target.Content = "foo";
+
+            var control = target.Child;
+            Assert.IsType<Border>(control);
+
+            target.Content = "bar";
+            Assert.Same(control, target.Child);
+        }
+
+        [Fact]
+        public void Detects_DataTemplate_Doesnt_Match_And_Doesnt_Recycle()
+        {
+            var (target, _) = CreateTarget();
+            target.DataTemplates.Add(new FuncDataTemplate<string>(x => x == "foo", _ => new Border(), true));
+
+            target.Content = "foo";
+
+            var control = target.Child;
+            Assert.IsType<Border>(control);
+
+            target.Content = "bar";
+            Assert.IsType<TextBlock>(target.Child);
+        }
+
+        [Fact]
+        public void Detects_DataTemplate_Doesnt_Support_Recycling()
+        {
+            var (target, _) = CreateTarget();
+            target.DataTemplates.Add(new FuncDataTemplate<string>(_ => new Border(), false));
+
+            target.Content = "foo";
+
+            var control = target.Child;
+            Assert.IsType<Border>(control);
+
+            target.Content = "bar";
+            Assert.NotSame(control, target.Child);
+        }
+
+        [Fact]
+        public void Reevaluates_DataTemplates_When_Recycling()
+        {
+            var (target, _) = CreateTarget();
+
+            target.DataTemplates.Add(new FuncDataTemplate<string>(x => x == "bar", _ => new Canvas(), true));
+            target.DataTemplates.Add(new FuncDataTemplate<string>(_ => new Border(), true));
+
+            target.Content = "foo";
+
+            var control = target.Child;
+            Assert.IsType<Border>(control);
+
+            target.Content = "bar";
+            Assert.IsType<Canvas>(target.Child);
+        }
+
+        (ContentPresenter presenter, ContentControl templatedParent) CreateTarget()
+        {
+            var templatedParent = new ContentControl
+            {
+                Template = new FuncControlTemplate<ContentControl>(x => 
+                    new ContentPresenter
+                    {
+                        Name = "PART_ContentPresenter",
+                    }),
+            };
+            var root = new TestRoot { Child = templatedParent };
+
+            templatedParent.ApplyTemplate();
+
+            return ((ContentPresenter)templatedParent.Presenter, templatedParent);
+        }
+
+        private class TestContentControl : ContentControl
+        {
+            public IControl Child { get; set; }
+        }
+    }
+}

tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests.cs → tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_Standalone.cs

@@ -15,91 +15,13 @@ using Xunit;
 
 namespace Avalonia.Controls.UnitTests.Presenters
 {
-    public class ContentPresenterTests
+    /// <summary>
+    /// Tests for ContentControls that aren't hosted in a control template.
+    /// </summary>
+    public class ContentPresenterTests_Standalone
     {
         [Fact]
-        public void Should_Register_With_Host_When_TemplatedParent_Set()
-        {
-            var host = new Mock<IContentPresenterHost>();
-            var target = new ContentPresenter();
-
-            target.SetValue(Control.TemplatedParentProperty, host.Object);
-
-            host.Verify(x => x.RegisterContentPresenter(target));
-        }
-
-        [Fact]
-        public void Setting_Content_To_Control_Should_Set_Child()
-        {
-            var target = new ContentPresenter();
-            var child = new Border();
-
-            target.Content = child;
-
-            Assert.Null(target.Child);
-            target.UpdateChild();
-            Assert.Equal(child, target.Child);
-        }
-
-        [Fact]
-        public void Setting_Content_To_String_Should_Create_TextBlock()
-        {
-            var target = new ContentPresenter();
-
-            target.Content = "Foo";
-
-            Assert.Null(target.Child);
-            target.UpdateChild();
-            Assert.IsType<TextBlock>(target.Child);
-            Assert.Equal("Foo", ((TextBlock)target.Child).Text);
-        }
-
-        [Fact]
-        public void Control_Content_Should_Not_Be_NameScope()
-        {
-            var target = new ContentPresenter();
-
-            target.Content = new TextBlock();
-
-            Assert.Null(target.Child);
-            target.UpdateChild();
-            Assert.IsType<TextBlock>(target.Child);
-            Assert.Null(NameScope.GetNameScope((Control)target.Child));
-        }
-
-        [Fact]
-        public void DataTemplate_Created_Control_Should_Be_NameScope()
-        {
-            var target = new ContentPresenter();
-
-            target.Content = "Foo";
-
-            Assert.Null(target.Child);
-            target.UpdateChild();
-            Assert.IsType<TextBlock>(target.Child);
-            Assert.NotNull(NameScope.GetNameScope((Control)target.Child));
-        }
-
-        [Fact]
-        public void Should_Set_Childs_Parent_To_TemplatedParent()
-        {
-            var content = new Border();
-            var target = new TestContentControl
-            {
-                Template = new FuncControlTemplate<TestContentControl>(parent =>
-                    new ContentPresenter { Content = parent.Child }),
-                Child = content,
-            };
-
-            target.ApplyTemplate();
-            var presenter = ((ContentPresenter)target.GetVisualChildren().Single());
-            presenter.UpdateChild();
-
-            Assert.Same(target, content.Parent);
-        }
-
-        [Fact]
-        public void Should_Set_Childs_Parent_To_Itself_Outside_Template()
+        public void Should_Set_Childs_Parent_To_Itself_Standalone()
         {
             var content = new Border();
             var target = new ContentPresenter { Content = content };
@@ -110,7 +32,7 @@ namespace Avalonia.Controls.UnitTests.Presenters
         }
 
         [Fact]
-        public void Should_Add_Child_To_Own_LogicalChildren_Outside_Template()
+        public void Should_Add_Child_To_Own_LogicalChildren_Standalone()
         {
             var content = new Border();
             var target = new ContentPresenter { Content = content };
@@ -124,94 +46,7 @@ namespace Avalonia.Controls.UnitTests.Presenters
         }
 
         [Fact]
-        public void Adding_To_Logical_Tree_Should_Reevaluate_DataTemplates()
-        {
-            var target = new ContentPresenter
-            {
-                Content = "Foo",
-            };
-
-            target.UpdateChild();
-            Assert.IsType<TextBlock>(target.Child);
-
-            var root = new TestRoot
-            {
-                DataTemplates = new DataTemplates
-                {
-                    new FuncDataTemplate<string>(x => new Decorator()),
-                },
-            };
-
-            root.Child = target;
-            target.ApplyTemplate();
-            Assert.IsType<Decorator>(target.Child);
-        }
-
-        [Fact]
-        public void Assigning_Control_To_Content_Should_Not_Set_DataContext()
-        {
-            var target = new ContentPresenter
-            {
-                Content = new Border(),
-            };
-
-            Assert.False(target.IsSet(Control.DataContextProperty));
-        }
-
-        [Fact]
-        public void Assigning_NonControl_To_Content_Should_Set_DataContext_On_UpdateChild()
-        {
-            var target = new ContentPresenter
-            {
-                Content = "foo",
-            };
-
-            target.UpdateChild();
-
-            Assert.Equal("foo", target.DataContext);
-        }
-
-        [Fact]
-        public void Assigning_Control_To_Content_After_NonControl_Should_Clear_DataContext()
-        {
-            var target = new ContentPresenter();
-
-            target.Content = "foo";
-            target.UpdateChild();
-
-            Assert.True(target.IsSet(Control.DataContextProperty));
-
-            target.Content = new Border();
-            target.UpdateChild();
-
-            Assert.False(target.IsSet(Control.DataContextProperty));
-        }
-
-        [Fact]
-        public void Tries_To_Recycle_DataTemplate()
-        {
-            var target = new ContentPresenter
-            {
-                DataTemplates = new DataTemplates
-                {
-                    new FuncDataTemplate<string>(_ => new Border(), true),
-                },
-                Content = "foo",
-            };
-
-            target.UpdateChild();
-            var control = target.Child;
-
-            Assert.IsType<Border>(control);
-
-            target.Content = "bar";
-            target.UpdateChild();
-
-            Assert.Same(control, target.Child);
-        }
-
-        [Fact]
-        public void Should_Raise_DetachedFromLogicalTree_On_Content_Changed_OutsideTemplate()
+        public void Should_Raise_DetachedFromLogicalTree_On_Content_Changed_Standalone()
         {
             var target = new ContentPresenter
             {
@@ -250,7 +85,7 @@ namespace Avalonia.Controls.UnitTests.Presenters
         }
 
         [Fact]
-        public void Should_Raise_DetachedFromLogicalTree_In_ContentControl_On_Content_Changed_OutsideTemplate()
+        public void Should_Raise_DetachedFromLogicalTree_In_ContentControl_On_Content_Changed_Standalone()
         {
             var contentControl = new ContentControl
             {
@@ -292,13 +127,14 @@ namespace Avalonia.Controls.UnitTests.Presenters
             var tbbar = target.Child as ContentControl;
 
             Assert.NotNull(tbbar);
+
             Assert.True(tbbar != tbfoo);
             Assert.False((tbfoo as IControl).IsAttachedToLogicalTree);
             Assert.True(foodetached);
         }
 
         [Fact]
-        public void Should_Raise_DetachedFromLogicalTree_On_Detached_OutsideTemplate()
+        public void Should_Raise_DetachedFromLogicalTree_On_Detached_Standalone()
         {
             var target = new ContentPresenter
             {
@@ -332,7 +168,7 @@ namespace Avalonia.Controls.UnitTests.Presenters
         }
 
         [Fact]
-        public void Should_Remove_Old_Child_From_LogicalChildren_On_ContentChanged_OutsideTemplate()
+        public void Should_Remove_Old_Child_From_LogicalChildren_On_ContentChanged_Standalone()
         {
             var target = new ContentPresenter
             {
@@ -363,9 +199,5 @@ namespace Avalonia.Controls.UnitTests.Presenters
             Assert.NotEqual(foo, logicalChildren.First());
         }
 
-        private class TestContentControl : ContentControl
-        {
-            public IControl Child { get; set; }
-        }
     }
 }

tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_Unrooted.cs

@@ -0,0 +1,102 @@
+// 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 Avalonia.Controls.Presenters;
+using Avalonia.Controls.Templates;
+using Avalonia.UnitTests;
+using Xunit;
+
+namespace Avalonia.Controls.UnitTests.Presenters
+{
+    /// <summary>
+    /// Tests for ContentControls that are not attached to a logical tree.
+    /// </summary>
+    public class ContentPresenterTests_Unrooted
+    {
+        [Fact]
+        public void Setting_Content_To_Control_Should_Not_Set_Child_Unless_UpdateChild_Called()
+        {
+            var target = new ContentPresenter();
+            var child = new Border();
+
+            target.Content = child;
+            Assert.Null(target.Child);
+
+            target.ApplyTemplate();
+            Assert.Null(target.Child);
+
+            target.UpdateChild();
+            Assert.Equal(child, target.Child);
+        }
+
+        [Fact]
+        public void Setting_Content_To_String_Should_Not_Create_TextBlock_Unless_UpdateChild_Called()
+        {
+            var target = new ContentPresenter();
+
+            target.Content = "Foo";
+            Assert.Null(target.Child);
+
+            target.ApplyTemplate();
+            Assert.Null(target.Child);
+
+            target.UpdateChild();
+            Assert.IsType<TextBlock>(target.Child);
+            Assert.Equal("Foo", ((TextBlock)target.Child).Text);
+        }
+
+        [Fact]
+        public void Clearing_Control_Content_Should_Remove_Child_Immediately()
+        {
+            var target = new ContentPresenter();
+            var child = new Border();
+
+            target.Content = child;
+            target.UpdateChild();
+            Assert.Equal(child, target.Child);
+
+            target.Content = null;
+            Assert.Null(target.Child);
+        }
+
+        [Fact]
+        public void Clearing_String_Content_Should_Remove_Child_Immediately()
+        {
+            var target = new ContentPresenter();
+
+            target.Content = "Foo";
+            target.UpdateChild();
+            Assert.IsType<TextBlock>(target.Child);
+
+            target.Content = null;
+            Assert.Null(target.Child);
+        }
+
+        [Fact]
+        public void Adding_To_Logical_Tree_Should_Reevaluate_DataTemplates()
+        {
+            var root = new TestRoot();
+            var target = new ContentPresenter();
+
+            target.Content = "Foo";
+            Assert.Null(target.Child);
+
+            root.Child = target;
+            target.ApplyTemplate();
+            Assert.IsType<TextBlock>(target.Child);
+
+            root.Child = null;
+            root = new TestRoot
+            {
+                DataTemplates = new DataTemplates
+                {
+                    new FuncDataTemplate<string>(x => new Decorator()),
+                },
+            };
+
+            root.Child = target;
+            target.ApplyTemplate();
+            Assert.IsType<Decorator>(target.Child);
+        }
+    }
+}