From 4fa3c98ca381f1f47706bf9ec3743c18a7f7637a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 30 Nov 2015 21:06:24 +0100 Subject: [PATCH] Detach styles when control removed from visual tree. --- src/Perspex.Controls/ItemsControl.cs | 7 ++ .../Primitives/TemplatedControl.cs | 17 ++-- src/Perspex.Layout/LayoutManager.cs | 4 +- .../Properties/AssemblyInfo.cs | 4 +- src/Perspex.Styling/Styling/Style.cs | 14 +++- src/Perspex.Styling/Styling/StyleBinding.cs | 13 +-- .../Perspex.Styling.UnitTests.csproj | 2 + .../StyleBindingTests.cs | 79 +++++++++++++++++++ tests/Perspex.Styling.UnitTests/StyleTests.cs | 27 ++++++- tests/Perspex.Styling.UnitTests/TestRoot.cs | 34 ++++++++ 10 files changed, 184 insertions(+), 17 deletions(-) create mode 100644 tests/Perspex.Styling.UnitTests/StyleBindingTests.cs create mode 100644 tests/Perspex.Styling.UnitTests/TestRoot.cs diff --git a/src/Perspex.Controls/ItemsControl.cs b/src/Perspex.Controls/ItemsControl.cs index 5c06dbc04e..02357c6f3f 100644 --- a/src/Perspex.Controls/ItemsControl.cs +++ b/src/Perspex.Controls/ItemsControl.cs @@ -152,6 +152,13 @@ namespace Perspex.Controls Presenter = nameScope.Find("PART_ItemsPresenter"); } + /// + protected override void OnTemplateChanged(PerspexPropertyChangedEventArgs e) + { + base.OnTemplateChanged(e); + ItemContainerGenerator.Clear(); + } + /// /// Caled when the property changes. /// diff --git a/src/Perspex.Controls/Primitives/TemplatedControl.cs b/src/Perspex.Controls/Primitives/TemplatedControl.cs index 9e98ad154c..92e1c4af88 100644 --- a/src/Perspex.Controls/Primitives/TemplatedControl.cs +++ b/src/Perspex.Controls/Primitives/TemplatedControl.cs @@ -81,12 +81,7 @@ namespace Perspex.Controls.Primitives /// static TemplatedControl() { - TemplateProperty.Changed.Subscribe(e => - { - var templatedControl = (TemplatedControl)e.Sender; - templatedControl._templateApplied = false; - templatedControl.InvalidateMeasure(); - }); + TemplateProperty.Changed.AddClassHandler(x => x.OnTemplateChanged); } /// @@ -224,6 +219,16 @@ namespace Perspex.Controls.Primitives { } + /// + /// Called when the property changes. + /// + /// The event args. + protected virtual void OnTemplateChanged(PerspexPropertyChangedEventArgs e) + { + _templateApplied = false; + InvalidateMeasure(); + } + /// /// Sets the TemplatedParent property for a control created from the control template and /// applies the templates of nested templated controls. Also adds each control to its name diff --git a/src/Perspex.Layout/LayoutManager.cs b/src/Perspex.Layout/LayoutManager.cs index f1c45693ab..326549ccae 100644 --- a/src/Perspex.Layout/LayoutManager.cs +++ b/src/Perspex.Layout/LayoutManager.cs @@ -226,12 +226,12 @@ namespace Perspex.Layout { var parent = item.Control.GetVisualParent(); - while (parent.PreviousMeasure == null) + while (parent != null && parent.PreviousMeasure == null) { parent = parent.GetVisualParent(); } - if (parent.GetVisualRoot() == Root) + if (parent != null && parent.GetVisualRoot() == Root) { parent.Measure(parent.PreviousMeasure.Value, true); } diff --git a/src/Perspex.Styling/Properties/AssemblyInfo.cs b/src/Perspex.Styling/Properties/AssemblyInfo.cs index f06c17710e..21034a0753 100644 --- a/src/Perspex.Styling/Properties/AssemblyInfo.cs +++ b/src/Perspex.Styling/Properties/AssemblyInfo.cs @@ -2,7 +2,9 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System.Reflection; +using System.Runtime.CompilerServices; using Perspex.Metadata; [assembly: AssemblyTitle("Perspex.Styling")] -[assembly: XmlnsDefinition("https://github.com/perspex", "Perspex.Styling")] \ No newline at end of file +[assembly: XmlnsDefinition("https://github.com/perspex", "Perspex.Styling")] +[assembly: InternalsVisibleTo("Perspex.Styling.UnitTests")] \ No newline at end of file diff --git a/src/Perspex.Styling/Styling/Style.cs b/src/Perspex.Styling/Styling/Style.cs index 61b05d1412..112bc817a0 100644 --- a/src/Perspex.Styling/Styling/Style.cs +++ b/src/Perspex.Styling/Styling/Style.cs @@ -56,9 +56,21 @@ namespace Perspex.Styling if (match.ImmediateResult != false) { + var visual = control as IVisual; + var activator = match.ObservableResult ?? + Observable.Never().StartWith(true); + + if (visual != null) + { + var detached = Observable.FromEventPattern( + x => visual.DetachedFromVisualTree += x, + x => visual.DetachedFromVisualTree -= x); + activator = activator.TakeUntil(detached); + } + foreach (var setter in Setters) { - setter.Apply(this, control, match.ObservableResult); + setter.Apply(this, control, activator); } } } diff --git a/src/Perspex.Styling/Styling/StyleBinding.cs b/src/Perspex.Styling/Styling/StyleBinding.cs index 9eb6f00235..b2ddbeee1b 100644 --- a/src/Perspex.Styling/Styling/StyleBinding.cs +++ b/src/Perspex.Styling/Styling/StyleBinding.cs @@ -61,7 +61,8 @@ namespace Perspex.Styling /// public object ActivatedValue { - get; } + get; + } /// /// Gets a description of the binding. @@ -90,16 +91,16 @@ namespace Perspex.Styling if (Source == null) { - return _activator.Subscribe( - active => observer.OnNext(active ? ActivatedValue : PerspexProperty.UnsetValue), - observer.OnError, - observer.OnCompleted); + return _activator + .Select(active => active ? ActivatedValue : PerspexProperty.UnsetValue) + .Subscribe(observer); } else { return _activator .CombineLatest(Source, (x, y) => new { Active = x, Value = y }) - .Subscribe(x => observer.OnNext(x.Active ? x.Value : PerspexProperty.UnsetValue)); + .Select(x => x.Active ? x.Value : PerspexProperty.UnsetValue) + .Subscribe(observer); } } } diff --git a/tests/Perspex.Styling.UnitTests/Perspex.Styling.UnitTests.csproj b/tests/Perspex.Styling.UnitTests/Perspex.Styling.UnitTests.csproj index 4099319b5a..95cb831480 100644 --- a/tests/Perspex.Styling.UnitTests/Perspex.Styling.UnitTests.csproj +++ b/tests/Perspex.Styling.UnitTests/Perspex.Styling.UnitTests.csproj @@ -85,10 +85,12 @@ + + diff --git a/tests/Perspex.Styling.UnitTests/StyleBindingTests.cs b/tests/Perspex.Styling.UnitTests/StyleBindingTests.cs new file mode 100644 index 0000000000..82a7f66d07 --- /dev/null +++ b/tests/Perspex.Styling.UnitTests/StyleBindingTests.cs @@ -0,0 +1,79 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Reactive.Linq; +using System.Reactive.Subjects; +using Xunit; + +namespace Perspex.Styling.UnitTests +{ + public class StyleBindingTests + { + [Fact] + public async void Should_Produce_UnsetValue_On_Activator_False() + { + var activator = new BehaviorSubject(false); + var target = new StyleBinding(activator, 1, string.Empty); + var result = await target.Take(1); + + Assert.Equal(PerspexProperty.UnsetValue, result); + } + + [Fact] + public async void Should_Produce_Value_On_Activator_True() + { + var activator = new BehaviorSubject(true); + var target = new StyleBinding(activator, 1, string.Empty); + var result = await target.Take(1); + + Assert.Equal(1, result); + } + + [Fact] + public void Should_Change_Value_On_Activator_Change() + { + var activator = new BehaviorSubject(false); + var target = new StyleBinding(activator, 1, string.Empty); + var result = new List(); + + target.Subscribe(x => result.Add(x)); + + activator.OnNext(true); + activator.OnNext(false); + + Assert.Equal(new[] { PerspexProperty.UnsetValue, 1, PerspexProperty.UnsetValue }, result); + } + + [Fact] + public void Should_Change_Value_With_Source_Observable() + { + var activator = new BehaviorSubject(false); + var source = new BehaviorSubject(1); + var target = new StyleBinding(activator, source, string.Empty); + var result = new List(); + + target.Subscribe(x => result.Add(x)); + + activator.OnNext(true); + source.OnNext(2); + activator.OnNext(false); + + Assert.Equal(new[] { PerspexProperty.UnsetValue, 1, 2, PerspexProperty.UnsetValue }, result); + } + + [Fact] + public void Should_Complete_When_Activator_Completes() + { + var activator = new BehaviorSubject(false); + var target = new StyleBinding(activator, 1, string.Empty); + var completed = false; + + target.Subscribe(_ => { }, () => completed = true); + activator.OnCompleted(); + + Assert.True(completed); + } + } +} diff --git a/tests/Perspex.Styling.UnitTests/StyleTests.cs b/tests/Perspex.Styling.UnitTests/StyleTests.cs index cda975b597..c484389cbf 100644 --- a/tests/Perspex.Styling.UnitTests/StyleTests.cs +++ b/tests/Perspex.Styling.UnitTests/StyleTests.cs @@ -166,7 +166,7 @@ namespace Perspex.Styling.UnitTests { var source = new BehaviorSubject("Foo"); - Style style = new Style(x => x.OfType().Class("foo")) + var style = new Style(x => x.OfType().Class("foo")) { Setters = new[] { @@ -187,6 +187,31 @@ namespace Perspex.Styling.UnitTests Assert.Equal("foodefault", target.Foo); } + [Fact] + public void Style_Should_Detach_When_Removed_From_Visual_Tree() + { + Border border; + + var style = new Style(x => x.OfType()) + { + Setters = new[] + { + new Setter(Border.BorderThicknessProperty, 4), + } + }; + + var root = new TestRoot + { + Child = border = new Border(), + }; + + style.Attach(border, null); + + Assert.Equal(4, border.BorderThickness); + root.Child = null; + Assert.Equal(0, border.BorderThickness); + } + private class Class1 : Control { public static readonly PerspexProperty FooProperty = diff --git a/tests/Perspex.Styling.UnitTests/TestRoot.cs b/tests/Perspex.Styling.UnitTests/TestRoot.cs new file mode 100644 index 0000000000..2f613e4e51 --- /dev/null +++ b/tests/Perspex.Styling.UnitTests/TestRoot.cs @@ -0,0 +1,34 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using Moq; +using Perspex.Controls; +using Perspex.Layout; +using Perspex.Platform; +using Perspex.Rendering; + +namespace Perspex.Styling.UnitTests +{ + internal class TestRoot : Decorator, ILayoutRoot, IRenderRoot + { + public Size ClientSize => new Size(100, 100); + + public ILayoutManager LayoutManager => new Mock().Object; + + public IRenderTarget RenderTarget + { + get { throw new NotImplementedException(); } + } + + public IRenderQueueManager RenderQueueManager + { + get { throw new NotImplementedException(); } + } + + public Point TranslatePointToScreen(Point p) + { + return new Point(); + } + } +}