diff --git a/src/Avalonia.Controls/Button.cs b/src/Avalonia.Controls/Button.cs index d8c31fc4779..821aef77588 100644 --- a/src/Avalonia.Controls/Button.cs +++ b/src/Avalonia.Controls/Button.cs @@ -34,8 +34,10 @@ public enum ClickMode [PseudoClasses(pcFlyoutOpen, pcPressed)] public class Button : ContentControl, ICommandSource, IClickableControl { - private const string pcPressed = ":pressed"; + private const string pcPressed = ":pressed"; private const string pcFlyoutOpen = ":flyout-open"; + private EventHandler? _canExecuteChangeHandler = default; + private EventHandler CanExecuteChangedHandler => _canExecuteChangeHandler ??= new(CanExecuteChanged); /// /// Defines the property. @@ -250,10 +252,11 @@ protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e base.OnAttachedToLogicalTree(e); - if (Command != null) + (var command, var parameter) = (Command, CommandParameter); + if (command is not null) { - Command.CanExecuteChanged += CanExecuteChanged; - CanExecuteChanged(this, EventArgs.Empty); + command.CanExecuteChanged += CanExecuteChangedHandler; + CanExecuteChanged(command, parameter); } } @@ -269,9 +272,9 @@ protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs base.OnDetachedFromLogicalTree(e); - if (Command != null) + if (Command is { } command) { - Command.CanExecuteChanged -= CanExecuteChanged; + command.CanExecuteChanged -= CanExecuteChangedHandler; } } @@ -343,9 +346,10 @@ protected virtual void OnClick() var e = new RoutedEventArgs(ClickEvent); RaiseEvent(e); - if (!e.Handled && Command?.CanExecute(CommandParameter) == true) + (var command, var parameter) = (Command, CommandParameter); + if (!e.Handled && command is not null && command.CanExecute(parameter)) { - Command.Execute(CommandParameter); + command.Execute(parameter); e.Handled = true; } } @@ -451,25 +455,24 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang if (change.Property == CommandProperty) { + var (oldValue, newValue) = change.GetOldAndNewValue(); if (((ILogical)this).IsAttachedToLogicalTree) { - var (oldValue, newValue) = change.GetOldAndNewValue(); if (oldValue is ICommand oldCommand) { - oldCommand.CanExecuteChanged -= CanExecuteChanged; + oldCommand.CanExecuteChanged -= CanExecuteChangedHandler; } if (newValue is ICommand newCommand) { - newCommand.CanExecuteChanged += CanExecuteChanged; + newCommand.CanExecuteChanged += CanExecuteChangedHandler; } } - - CanExecuteChanged(this, EventArgs.Empty); + CanExecuteChanged(newValue, CommandParameter); } else if (change.Property == CommandParameterProperty) { - CanExecuteChanged(this, EventArgs.Empty); + CanExecuteChanged(Command, change.NewValue); } else if (change.Property == IsCancelProperty) { @@ -557,7 +560,18 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang /// The event args. private void CanExecuteChanged(object? sender, EventArgs e) { - var canExecute = Command == null || Command.CanExecute(CommandParameter); + CanExecuteChanged(Command, CommandParameter); + } + + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] + private void CanExecuteChanged(ICommand? command, object? parameter) + { + if (!((ILogical)this).IsAttachedToLogicalTree) + { + return; + } + + var canExecute = command == null || command.CanExecute(parameter); if (canExecute != _commandCanExecute) { diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index a81ca13e811..a2a54d5550f 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using Avalonia.Reactive; using System.Windows.Input; using Avalonia.Automation.Peers; using Avalonia.Controls.Metadata; @@ -13,7 +12,7 @@ using Avalonia.Input; using Avalonia.Interactivity; using Avalonia.LogicalTree; -using Avalonia.Layout; +using Avalonia.Reactive; namespace Avalonia.Controls { @@ -24,6 +23,9 @@ namespace Avalonia.Controls [PseudoClasses(":separator", ":radio", ":toggle", ":checked", ":icon", ":open", ":pressed", ":selected")] public class MenuItem : HeaderedSelectingItemsControl, IMenuItem, ISelectable, ICommandSource, IClickableControl, IRadioButton { + private EventHandler? _canExecuteChangeHandler = default; + private EventHandler CanExecuteChangedHandler => _canExecuteChangeHandler ??= new(CanExecuteChanged); + /// /// Defines the property. /// @@ -83,7 +85,7 @@ public class MenuItem : HeaderedSelectingItemsControl, IMenuItem, ISelectable, I /// public static readonly StyledProperty GroupNameProperty = RadioButton.GroupNameProperty.AddOwner(); - + /// /// Defines the event. /// @@ -292,7 +294,7 @@ public bool StaysOpenOnClick get => GetValue(StaysOpenOnClickProperty); set => SetValue(StaysOpenOnClickProperty, value); } - + /// public MenuItemToggleType ToggleType { @@ -306,7 +308,7 @@ public bool IsChecked get => GetValue(IsCheckedProperty); set => SetValue(IsCheckedProperty, value); } - + bool IRadioButton.IsChecked { get => IsChecked; @@ -319,7 +321,7 @@ bool IRadioButton.IsChecked get => GetValue(GroupNameProperty); set => SetValue(GroupNameProperty, value); } - + /// /// Gets or sets a value that indicates whether the has a submenu. /// @@ -413,15 +415,16 @@ protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e { SetCurrentValue(HotKeyProperty, _hotkey); } - + base.OnAttachedToLogicalTree(e); - if (Command != null) + (var command, var parameter) = (Command, CommandParameter); + if (command is not null) { - Command.CanExecuteChanged += CanExecuteChanged; + command.CanExecuteChanged += CanExecuteChangedHandler; } - - TryUpdateCanExecute(); + + TryUpdateCanExecute(command, parameter); var parent = Parent; @@ -437,7 +440,7 @@ protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) { base.OnAttachedToVisualTree(e); - + TryUpdateCanExecute(); } @@ -454,7 +457,7 @@ protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs if (Command != null) { - Command.CanExecuteChanged -= CanExecuteChanged; + Command.CanExecuteChanged -= CanExecuteChangedHandler; } } @@ -464,9 +467,10 @@ protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs /// The click event args. protected virtual void OnClick(RoutedEventArgs e) { - if (!e.Handled && Command?.CanExecute(CommandParameter) == true) + (var command, var parameter) = (Command, CommandParameter); + if (!e.Handled && command is not null && command.CanExecute(parameter) == true) { - Command.Execute(CommandParameter); + command.Execute(parameter); e.Handled = true; } } @@ -577,21 +581,25 @@ private void CloseSubmenus() /// The event args. private static void CommandChanged(AvaloniaPropertyChangedEventArgs e) { - if (e.Sender is MenuItem menuItem && - ((ILogical)menuItem).IsAttachedToLogicalTree) + var newCommand = e.NewValue as ICommand; + if (e.Sender is MenuItem menuItem) + { - if (e.OldValue is ICommand oldCommand) + if (((ILogical)menuItem).IsAttachedToLogicalTree) { - oldCommand.CanExecuteChanged -= menuItem.CanExecuteChanged; - } + if (e.OldValue is ICommand oldCommand) + { + oldCommand.CanExecuteChanged -= menuItem.CanExecuteChangedHandler; + } - if (e.NewValue is ICommand newCommand) - { - newCommand.CanExecuteChanged += menuItem.CanExecuteChanged; + if (newCommand is not null) + { + newCommand.CanExecuteChanged += menuItem.CanExecuteChangedHandler; + } } - - menuItem.TryUpdateCanExecute(); + menuItem.TryUpdateCanExecute(newCommand, menuItem.CommandParameter); } + } /// @@ -602,7 +610,8 @@ private static void CommandParameterChanged(AvaloniaPropertyChangedEventArgs e) { if (e.Sender is MenuItem menuItem) { - menuItem.TryUpdateCanExecute(); + (var command, var parameter) = (menuItem.Command, e.NewValue); + menuItem.TryUpdateCanExecute(command, parameter); } } @@ -621,21 +630,27 @@ private void CanExecuteChanged(object? sender, EventArgs e) /// private void TryUpdateCanExecute() { - if (Command == null) + TryUpdateCanExecute(Command, CommandParameter); + } + + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] + private void TryUpdateCanExecute(ICommand? command, object? parameter) + { + if (command == null) { _commandCanExecute = !_commandBindingError; UpdateIsEffectivelyEnabled(); return; } - + //Perf optimization - only raise CanExecute event if the menu is open if (!((ILogical)this).IsAttachedToLogicalTree || Parent is MenuItem { IsSubMenuOpen: false }) { return; } - - var canExecute = Command.CanExecute(CommandParameter); + + var canExecute = command.CanExecute(parameter); if (canExecute != _commandCanExecute) { _commandCanExecute = canExecute; @@ -720,7 +735,7 @@ private void IsCheckedChanged(AvaloniaPropertyChangedEventArgs e) (MenuInteractionHandler as DefaultMenuInteractionHandler)?.OnCheckedChanged(this); } } - + /// /// Called when the property changes. /// @@ -834,7 +849,7 @@ private void PopupClosed(object? sender, EventArgs e) SelectedItem = null; } - void ICommandSource.CanExecuteChanged(object sender, EventArgs e) => this.CanExecuteChanged(sender, e); + void ICommandSource.CanExecuteChanged(object sender, EventArgs e) => CanExecuteChangedHandler(sender, e); void IClickableControl.RaiseClick() { diff --git a/src/Avalonia.Controls/SplitButton/SplitButton.cs b/src/Avalonia.Controls/SplitButton/SplitButton.cs index 3a5bc26b438..dc8dc56fcf4 100644 --- a/src/Avalonia.Controls/SplitButton/SplitButton.cs +++ b/src/Avalonia.Controls/SplitButton/SplitButton.cs @@ -136,7 +136,18 @@ public SplitButton() /// private void CanExecuteChanged(object? sender, EventArgs e) { - var canExecute = Command == null || Command.CanExecute(CommandParameter); + (var command, var parameter) = (Command, CommandParameter); + CanExecuteChanged(command, parameter); + } + + private void CanExecuteChanged(ICommand? command, object? parameter) + { + if (!((ILogical)this).IsAttachedToLogicalTree) + { + return; + } + + var canExecute = command is null || command.CanExecute(parameter); if (canExecute != _commandCanExecute) { @@ -282,10 +293,11 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e) { if (e.Property == CommandProperty) { + // Must unregister events here while a reference to the old command still exists + var (oldValue, newValue) = e.GetOldAndNewValue(); + if (_isAttachedToLogicalTree) { - // Must unregister events here while a reference to the old command still exists - var (oldValue, newValue) = e.GetOldAndNewValue(); if (oldValue is ICommand oldCommand) { @@ -298,11 +310,11 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e) } } - CanExecuteChanged(this, EventArgs.Empty); + CanExecuteChanged(newValue, CommandParameter); } - else if (e.Property == CommandParameterProperty) + else if (e.Property == CommandParameterProperty && IsLoaded) { - CanExecuteChanged(this, EventArgs.Empty); + CanExecuteChanged(Command, e.NewValue); } else if (e.Property == FlyoutProperty) { @@ -386,15 +398,16 @@ protected override void OnKeyUp(KeyEventArgs e) /// The event args from the internal Click event. protected virtual void OnClickPrimary(RoutedEventArgs? e) { + (var command, var parameter) = (Command, CommandParameter); // Note: It is not currently required to check enabled status; however, this is a failsafe if (IsEffectivelyEnabled) { var eventArgs = new RoutedEventArgs(ClickEvent); RaiseEvent(eventArgs); - if (!eventArgs.Handled && Command?.CanExecute(CommandParameter) == true) + if (!eventArgs.Handled && command?.CanExecute(parameter) == true) { - Command.Execute(CommandParameter); + command.Execute(parameter); eventArgs.Handled = true; } } diff --git a/tests/Avalonia.Base.UnitTests/Input/KeyboardNavigationTests_Tab.cs b/tests/Avalonia.Base.UnitTests/Input/KeyboardNavigationTests_Tab.cs index 5d8ffd1e13d..1cdbd9f0281 100644 --- a/tests/Avalonia.Base.UnitTests/Input/KeyboardNavigationTests_Tab.cs +++ b/tests/Avalonia.Base.UnitTests/Input/KeyboardNavigationTests_Tab.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using Avalonia.Controls; using Avalonia.Input; +using Avalonia.Threading; using Avalonia.UnitTests; using Xunit; @@ -1282,6 +1283,8 @@ public void Next_Skip_Button_When_Command_CanExecute_Is_False() Button expected; bool executed = false; + using var app = UnitTestApplication.Start(TestServices.StyledWindow); + var top = new StackPanel { [KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.Cycle, @@ -1304,6 +1307,12 @@ public void Next_Skip_Button_When_Command_CanExecute_Is_False() } }; + var testRoot = new TestRoot(top); + + top.ApplyTemplate(); + + Dispatcher.UIThread.RunJobs(DispatcherPriority.Loaded); + var result = KeyboardNavigationHandler.GetNext(current, NavigationDirection.Next) as Button; Assert.Equal(expected.Name, result?.Name); diff --git a/tests/Avalonia.Controls.UnitTests/ButtonTests.cs b/tests/Avalonia.Controls.UnitTests/ButtonTests.cs index 0ddc247be64..876adcc28fe 100644 --- a/tests/Avalonia.Controls.UnitTests/ButtonTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ButtonTests.cs @@ -1,5 +1,7 @@ using System; -using System.Windows.Input; +using Avalonia.Controls.Presenters; +using Avalonia.Controls.Templates; +using Avalonia.Controls.UnitTests.Utils; using Avalonia.Data; using Avalonia.Input; using Avalonia.Interactivity; @@ -9,7 +11,6 @@ using Avalonia.Rendering; using Avalonia.Threading; using Avalonia.UnitTests; -using Avalonia.VisualTree; using Moq; using Xunit; using MouseButton = Avalonia.Input.MouseButton; @@ -19,7 +20,7 @@ namespace Avalonia.Controls.UnitTests public class ButtonTests : ScopedTestBase { private MouseTestHelper _helper = new MouseTestHelper(); - + [Fact] public void Button_Is_Disabled_When_Command_Is_Disabled() { @@ -100,6 +101,9 @@ public void Button_Is_Enabled_When_Bound_Command_Is_Added() DataContext = new object(), [!Button.CommandProperty] = new Binding("Command"), }; + var root = new TestRoot { Child = target }; + + Dispatcher.UIThread.RunJobs(DispatcherPriority.Loaded); Assert.True(target.IsEnabled); Assert.False(target.IsEffectivelyEnabled); @@ -141,9 +145,9 @@ public void Button_Raises_Click() renderer.Setup(r => r.HitTest(It.IsAny(), It.IsAny(), It.IsAny>())) .Returns>((p, r, f) => r.Bounds.Contains(p) ? new Visual[] { r } : new Visual[0]); - + using var _ = UnitTestApplication.Start(TestServices.StyledWindow); - + var root = new Window() { HitTesterOverride = renderer.Object }; var target = new Button() { @@ -188,7 +192,7 @@ public void Button_Does_Not_Raise_Click_When_PointerReleased_Outside() target.Click += (s, e) => clicked = true; RaisePointerEntered(target); - RaisePointerMove(target, new Point(50,50)); + RaisePointerMove(target, new Point(50, 50)); RaisePointerPressed(target, 1, MouseButton.Left, new Point(50, 50)); RaisePointerExited(target); @@ -210,9 +214,9 @@ public void Button_With_RenderTransform_Raises_Click() .Returns>((p, r, f) => r.Bounds.Contains(p.Transform(r.RenderTransform.Value.Invert())) ? new Visual[] { r } : new Visual[0]); - + using var _ = UnitTestApplication.Start(TestServices.StyledWindow); - + var root = new Window() { HitTesterOverride = renderer.Object }; var target = new Button() { @@ -298,16 +302,104 @@ public void Button_Invokes_CanExecute_When_CommandParameter_Changed() [Fact] public void Raises_Click_When_AccessKey_Raised() { - var command = new TestCommand(p => p is bool value && value); - var target = new Button { Command = command }; + var raised = 0; + var ah = new AccessKeyHandler(); + using var app = UnitTestApplication.Start(TestServices.StyledWindow.With(accessKeyHandler: ah)); + + var impl = CreateMockTopLevelImpl(); + var command = new TestCommand(p => p is bool value && value, _ => raised++); + Button target; + var root = new TestTopLevel(impl.Object) + { + Template = CreateTemplate(), + Content = target = new Button + { + Content = "_A", + Command = command, + Template = new FuncControlTemplate