From 5265b0592a86b3e36df1e82d7e494518b5b7f1df Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Thu, 29 Apr 2021 17:10:33 +0200 Subject: [PATCH 1/5] Implement MenuItem.StaysOpenOnClick. --- .../ControlCatalog/Pages/ContextMenuPage.xaml | 1 + src/Avalonia.Controls/IMenuItem.cs | 6 ++++++ src/Avalonia.Controls/MenuItem.cs | 16 ++++++++++++++++ .../Platform/DefaultMenuInteractionHandler.cs | 6 +++++- 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/samples/ControlCatalog/Pages/ContextMenuPage.xaml b/samples/ControlCatalog/Pages/ContextMenuPage.xaml index 260162ddb9..25f305151d 100644 --- a/samples/ControlCatalog/Pages/ContextMenuPage.xaml +++ b/samples/ControlCatalog/Pages/ContextMenuPage.xaml @@ -31,6 +31,7 @@ + diff --git a/src/Avalonia.Controls/IMenuItem.cs b/src/Avalonia.Controls/IMenuItem.cs index 94d761f725..d92625218a 100644 --- a/src/Avalonia.Controls/IMenuItem.cs +++ b/src/Avalonia.Controls/IMenuItem.cs @@ -23,6 +23,12 @@ namespace Avalonia.Controls /// bool IsSubMenuOpen { get; set; } + /// + /// Gets or sets a value that indicates the submenu that this is + /// within should not close when this item is clicked. + /// + bool StaysOpenOnClick { get; set; } + /// /// Gets a value that indicates whether the is a top-level main menu item. /// diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 94099a970e..0995ce0e0b 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -69,6 +69,12 @@ namespace Avalonia.Controls public static readonly StyledProperty IsSubMenuOpenProperty = AvaloniaProperty.Register(nameof(IsSubMenuOpen)); + /// + /// Defines the property. + /// + public static readonly StyledProperty StaysOpenOnClickProperty = + AvaloniaProperty.Register(nameof(StaysOpenOnClick)); + /// /// Defines the event. /// @@ -265,6 +271,16 @@ namespace Avalonia.Controls set { SetValue(IsSubMenuOpenProperty, value); } } + /// + /// Gets or sets a value that indicates the submenu that this is + /// within should not close when this item is clicked. + /// + public bool StaysOpenOnClick + { + get { return GetValue(StaysOpenOnClickProperty); } + set { SetValue(StaysOpenOnClickProperty, value); } + } + /// /// Gets or sets a value that indicates whether the has a submenu. /// diff --git a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs index e3e9e84d7e..984faa4d60 100644 --- a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs +++ b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs @@ -449,7 +449,11 @@ namespace Avalonia.Controls.Platform protected void Click(IMenuItem item) { item.RaiseClick(); - CloseMenu(item); + + if (!item.StaysOpenOnClick) + { + CloseMenu(item); + } } protected void CloseMenu(IMenuItem item) From 2635a8a7ef42707bcd55dafc980f8d7818b0ef0a Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Thu, 29 Apr 2021 17:13:48 +0200 Subject: [PATCH 2/5] Fix copy pasta. --- src/Avalonia.Controls/MenuItem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 0995ce0e0b..4c801c2e06 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -70,7 +70,7 @@ namespace Avalonia.Controls AvaloniaProperty.Register(nameof(IsSubMenuOpen)); /// - /// Defines the property. + /// Defines the property. /// public static readonly StyledProperty StaysOpenOnClickProperty = AvaloniaProperty.Register(nameof(StaysOpenOnClick)); From d9510f77efeb3b28d1f4be682f707a7dbac0db44 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 30 Apr 2021 10:44:45 +0200 Subject: [PATCH 3/5] Update ApiCompatBaseline.txt --- src/Avalonia.Controls/ApiCompatBaseline.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ApiCompatBaseline.txt b/src/Avalonia.Controls/ApiCompatBaseline.txt index 3a6810eed9..7199c15d21 100644 --- a/src/Avalonia.Controls/ApiCompatBaseline.txt +++ b/src/Avalonia.Controls/ApiCompatBaseline.txt @@ -1,4 +1,7 @@ Compat issues with assembly Avalonia.Controls: +InterfacesShouldHaveSameMembers : Interface member 'public System.Boolean Avalonia.Controls.IMenuItem.StaysOpenOnClick' is present in the implementation but not in the contract. +InterfacesShouldHaveSameMembers : Interface member 'public System.Boolean Avalonia.Controls.IMenuItem.StaysOpenOnClick.get()' is present in the implementation but not in the contract. +InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Controls.IMenuItem.StaysOpenOnClick.set(System.Boolean)' is present in the implementation but not in the contract. InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Controls.INativeMenuExporterEventsImplBridge.RaiseClosed()' is present in the implementation but not in the contract. InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Controls.INativeMenuExporterEventsImplBridge.RaiseOpening()' is present in the implementation but not in the contract. MembersMustExist : Member 'public void Avalonia.Controls.Embedding.Offscreen.OffscreenTopLevelImplBase.SetCursor(Avalonia.Platform.IPlatformHandle)' does not exist in the implementation but it does exist in the contract. @@ -7,4 +10,4 @@ EnumValuesMustMatch : Enum value 'Avalonia.Platform.ExtendClientAreaChromeHints InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Platform.ITopLevelImpl.SetCursor(Avalonia.Platform.ICursorImpl)' is present in the implementation but not in the contract. InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Platform.ITopLevelImpl.SetCursor(Avalonia.Platform.IPlatformHandle)' is present in the contract but not in the implementation. MembersMustExist : Member 'public void Avalonia.Platform.ITopLevelImpl.SetCursor(Avalonia.Platform.IPlatformHandle)' does not exist in the implementation but it does exist in the contract. -Total Issues: 7 +Total Issues: 11 From 04ae8ee43fb6016baef9ca8615025d08d5d97fb1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 30 Apr 2021 12:10:30 +0200 Subject: [PATCH 4/5] Added failing property batch update tests. --- .../AvaloniaObjectTests_BatchUpdate.cs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs index 53ad87421e..01d5752ead 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs @@ -5,6 +5,7 @@ using System.Reactive.Disposables; using System.Reactive.Linq; using System.Text; using Avalonia.Data; +using Avalonia.Layout; using Xunit; namespace Avalonia.Base.UnitTests @@ -104,6 +105,25 @@ namespace Avalonia.Base.UnitTests Assert.Equal("baz", target.Foo); } + [Fact] + public void SetValue_Change_Should_Be_Raised_After_Batch_Update_3() + { + var target = new TestClass(); + var raised = new List(); + + target.PropertyChanged += (s, e) => raised.Add(e); + + target.BeginBatchUpdate(); + target.SetValue(TestClass.BazProperty, Orientation.Horizontal, BindingPriority.LocalValue); + target.EndBatchUpdate(); + + Assert.Equal(1, raised.Count); + Assert.Equal(TestClass.BazProperty, raised[0].Property); + Assert.Equal(Orientation.Vertical, raised[0].OldValue); + Assert.Equal(Orientation.Horizontal, raised[0].NewValue); + Assert.Equal(Orientation.Horizontal, target.Baz); + } + [Fact] public void SetValue_Changes_Should_Be_Raised_In_Correct_Order_After_Batch_Update() { @@ -234,6 +254,26 @@ namespace Avalonia.Base.UnitTests Assert.Equal("baz", raised[0].NewValue); } + [Fact] + public void Binding_Change_Should_Be_Raised_After_Batch_Update_3() + { + var target = new TestClass(); + var observable = new TestObservable(Orientation.Horizontal); + var raised = new List(); + + target.PropertyChanged += (s, e) => raised.Add(e); + + target.BeginBatchUpdate(); + target.Bind(TestClass.BazProperty, observable, BindingPriority.LocalValue); + target.EndBatchUpdate(); + + Assert.Equal(1, raised.Count); + Assert.Equal(TestClass.BazProperty, raised[0].Property); + Assert.Equal(Orientation.Vertical, raised[0].OldValue); + Assert.Equal(Orientation.Horizontal, raised[0].NewValue); + Assert.Equal(Orientation.Horizontal, target.Baz); + } + [Fact] public void Binding_Completion_Should_Be_Raised_After_Batch_Update() { @@ -579,6 +619,9 @@ namespace Avalonia.Base.UnitTests public static readonly StyledProperty BarProperty = AvaloniaProperty.Register(nameof(Bar)); + public static readonly StyledProperty BazProperty = + AvaloniaProperty.Register(nameof(Bar), Orientation.Vertical); + public string Foo { get => GetValue(FooProperty); @@ -590,6 +633,12 @@ namespace Avalonia.Base.UnitTests get => GetValue(BarProperty); set => SetValue(BarProperty, value); } + + public Orientation Baz + { + get => GetValue(BazProperty); + set => SetValue(BazProperty, value); + } } public class TestObservable : ObservableBase> From 8959d55418132248e2c74d11b7d2c37d8e0e2b20 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 30 Apr 2021 12:13:12 +0200 Subject: [PATCH 5/5] Correctly handle default values in batch update notifications. We were previously calling `Optional.GetValueOrDefault()` in order to convert an `Optional` to an `Optional` which meant that empty values were getting converted to default values, meaning that in certain cases change notifications weren't raised. Add an `Optional.Cast()` methods (like we have in `BindingValue`) and use that in order to preserve empty values when ending a batch update operation. --- src/Avalonia.Base/Data/Optional.cs | 14 ++++++++++++++ src/Avalonia.Base/PropertyStore/BindingEntry.cs | 4 ++-- .../PropertyStore/ConstantValueEntry.cs | 4 ++-- src/Avalonia.Base/PropertyStore/LocalValueEntry.cs | 4 ++-- src/Avalonia.Base/PropertyStore/PriorityValue.cs | 4 ++-- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Base/Data/Optional.cs b/src/Avalonia.Base/Data/Optional.cs index 8e044d7896..9dec399e35 100644 --- a/src/Avalonia.Base/Data/Optional.cs +++ b/src/Avalonia.Base/Data/Optional.cs @@ -153,4 +153,18 @@ namespace Avalonia.Data /// public static Optional Empty => default; } + + public static class OptionalExtensions + { + /// + /// Casts the type of an using only the C# cast operator. + /// + /// The target type. + /// The binding value. + /// The cast value. + public static Optional Cast(this Optional value) + { + return value.HasValue ? new Optional((T)value.Value) : Optional.Empty; + } + } } diff --git a/src/Avalonia.Base/PropertyStore/BindingEntry.cs b/src/Avalonia.Base/PropertyStore/BindingEntry.cs index 3e17a81dd8..1b29338f07 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntry.cs @@ -127,8 +127,8 @@ namespace Avalonia.PropertyStore sink.ValueChanged(new AvaloniaPropertyChangedEventArgs( owner, (AvaloniaProperty)property, - oldValue.GetValueOrDefault(), - newValue.GetValueOrDefault(), + oldValue.Cast(), + newValue.Cast(), Priority)); } diff --git a/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs b/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs index d39fc3bb1e..dc4a1d88c1 100644 --- a/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs @@ -65,8 +65,8 @@ namespace Avalonia.PropertyStore sink.ValueChanged(new AvaloniaPropertyChangedEventArgs( owner, (AvaloniaProperty)property, - oldValue.GetValueOrDefault(), - newValue.GetValueOrDefault(), + oldValue.Cast(), + newValue.Cast(), Priority)); } } diff --git a/src/Avalonia.Base/PropertyStore/LocalValueEntry.cs b/src/Avalonia.Base/PropertyStore/LocalValueEntry.cs index f49b74f4a8..8fe2ad7794 100644 --- a/src/Avalonia.Base/PropertyStore/LocalValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/LocalValueEntry.cs @@ -36,8 +36,8 @@ namespace Avalonia.PropertyStore sink.ValueChanged(new AvaloniaPropertyChangedEventArgs( owner, (AvaloniaProperty)property, - oldValue.GetValueOrDefault(), - newValue.GetValueOrDefault(), + oldValue.Cast(), + newValue.Cast(), BindingPriority.LocalValue)); } } diff --git a/src/Avalonia.Base/PropertyStore/PriorityValue.cs b/src/Avalonia.Base/PropertyStore/PriorityValue.cs index 80496fc045..556f1a6269 100644 --- a/src/Avalonia.Base/PropertyStore/PriorityValue.cs +++ b/src/Avalonia.Base/PropertyStore/PriorityValue.cs @@ -197,8 +197,8 @@ namespace Avalonia.PropertyStore sink.ValueChanged(new AvaloniaPropertyChangedEventArgs( owner, (AvaloniaProperty)property, - oldValue.GetValueOrDefault(), - newValue.GetValueOrDefault(), + oldValue.Cast(), + newValue.Cast(), Priority)); }