From bca37513b4666b522104be1b1d1b0aa2cbecd19c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 13 Sep 2018 22:05:02 +0200 Subject: [PATCH 1/4] Added failing test for disposing completed binding. --- .../AvaloniaObjectTests_Binding.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs index 4638aa84a5..23984a7c8d 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs @@ -479,6 +479,18 @@ namespace Avalonia.Base.UnitTests Assert.False(source.SetterCalled); } + [Fact] + public void Disposing_Completed_Binding_Does_Not_Throw() + { + var target = new Class1(); + var source = new Subject(); + var subscription = target.Bind(Class1.FooProperty, source); + + source.OnCompleted(); + + subscription.Dispose(); + } + /// /// Returns an observable that returns a single value but does not complete. /// @@ -595,4 +607,4 @@ namespace Avalonia.Base.UnitTests public bool SetterCalled { get; private set; } } } -} \ No newline at end of file +} From e58b4d5152ac16e4bb9cafdb0e11ea710b2baef5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 13 Sep 2018 20:42:18 +0200 Subject: [PATCH 2/4] Don't dispose completed binding. --- src/Avalonia.Base/PriorityBindingEntry.cs | 7 +++++++ src/Avalonia.Base/PriorityLevel.cs | 14 +++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Base/PriorityBindingEntry.cs b/src/Avalonia.Base/PriorityBindingEntry.cs index 570bfe03dc..d4a47306a7 100644 --- a/src/Avalonia.Base/PriorityBindingEntry.cs +++ b/src/Avalonia.Base/PriorityBindingEntry.cs @@ -50,6 +50,11 @@ namespace Avalonia get; } + /// + /// Gets a value indicating whether the binding has completed. + /// + public bool HasCompleted { get; private set; } + /// /// The current value of the binding. /// @@ -129,6 +134,8 @@ namespace Avalonia private void Completed() { + HasCompleted = true; + if (Dispatcher.UIThread.CheckAccess()) { _owner.Completed(this); diff --git a/src/Avalonia.Base/PriorityLevel.cs b/src/Avalonia.Base/PriorityLevel.cs index 96661bd7ea..909558b0ce 100644 --- a/src/Avalonia.Base/PriorityLevel.cs +++ b/src/Avalonia.Base/PriorityLevel.cs @@ -112,12 +112,16 @@ namespace Avalonia return Disposable.Create(() => { - Bindings.Remove(node); - entry.Dispose(); - - if (entry.Index >= ActiveBindingIndex) + if (!entry.HasCompleted) { - ActivateFirstBinding(); + Bindings.Remove(node); + + entry.Dispose(); + + if (entry.Index >= ActiveBindingIndex) + { + ActivateFirstBinding(); + } } }); } From 74a5b1f65ad0fd947c4118854e64fe29327d515c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 13 Sep 2018 22:45:22 +0200 Subject: [PATCH 3/4] Added a failing MenuItem separator test. --- .../MenuItemTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/Avalonia.Controls.UnitTests/MenuItemTests.cs diff --git a/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs b/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs new file mode 100644 index 0000000000..e7352af23e --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Xunit; + +namespace Avalonia.Controls.UnitTests +{ + public class MenuItemTests + { + [Fact] + public void Header_Of_Minus_Should_Apply_Separator_Pseudoclass() + { + var target = new MenuItem { Header = "-" }; + + Assert.True(target.Classes.Contains(":separator")); + } + + [Fact] + public void Separator_Item_Should_Set_Focusable_False() + { + var target = new MenuItem { Header = "-" }; + + Assert.False(target.Focusable); + } + } +} From f55b556b1c42e810ab6e66ad9f986a59d8868c56 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 13 Sep 2018 22:45:35 +0200 Subject: [PATCH 4/4] Make separator MenuItem non-focusable. --- src/Avalonia.Controls/MenuItem.cs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 03a43b5164..055d49fb0b 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -99,13 +99,13 @@ namespace Avalonia.Controls SelectableMixin.Attach(IsSelectedProperty); CommandProperty.Changed.Subscribe(CommandChanged); FocusableProperty.OverrideDefaultValue(true); + HeaderProperty.Changed.AddClassHandler(x => x.HeaderChanged); IconProperty.Changed.AddClassHandler(x => x.IconChanged); IsSelectedProperty.Changed.AddClassHandler(x => x.IsSelectedChanged); ItemsPanelProperty.OverrideDefaultValue(DefaultPanel); ClickEvent.AddClassHandler(x => x.OnClick); SubmenuOpenedEvent.AddClassHandler(x => x.OnSubmenuOpened); IsSubMenuOpenProperty.Changed.AddClassHandler(x => x.SubMenuOpenChanged); - PseudoClass(HeaderProperty, x => x as string == "-", ":separator"); } public MenuItem() @@ -420,6 +420,24 @@ namespace Avalonia.Controls IsEnabled = Command == null || Command.CanExecute(CommandParameter); } + /// + /// Called when the property changes. + /// + /// The property change event. + private void HeaderChanged(AvaloniaPropertyChangedEventArgs e) + { + if (e.NewValue is string newValue && newValue == "-") + { + PseudoClasses.Add(":separator"); + Focusable = false; + } + else if (e.OldValue is string oldValue && oldValue == "-") + { + PseudoClasses.Remove(":separator"); + Focusable = true; + } + } + /// /// Called when the property changes. ///