diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 0bead04982..b661bcfe33 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -107,6 +107,7 @@ namespace Avalonia.Controls private ICommand? _command; private bool _commandCanExecute = true; + private bool _commandBindingError; private Popup? _popup; private KeyGesture? _hotkey; private bool _isEmbeddedInMenu; @@ -379,6 +380,8 @@ namespace Avalonia.Controls { Command.CanExecuteChanged += CanExecuteChanged; } + + TryUpdateCanExecute(); var parent = Parent; @@ -498,13 +501,11 @@ namespace Avalonia.Controls base.UpdateDataValidation(property, value); if (property == CommandProperty) { - if (value.Type == BindingValueType.BindingError) + _commandBindingError = value.Type == BindingValueType.BindingError; + if (_commandBindingError && _commandCanExecute) { - if (_commandCanExecute) - { - _commandCanExecute = false; - UpdateIsEffectivelyEnabled(); - } + _commandCanExecute = false; + UpdateIsEffectivelyEnabled(); } } } @@ -526,22 +527,20 @@ namespace Avalonia.Controls /// The event args. private static void CommandChanged(AvaloniaPropertyChangedEventArgs e) { - if (e.Sender is MenuItem menuItem) + if (e.Sender is MenuItem menuItem && + ((ILogical)menuItem).IsAttachedToLogicalTree) { - if (((ILogical)menuItem).IsAttachedToLogicalTree) + if (e.OldValue is ICommand oldCommand) { - if (e.OldValue is ICommand oldCommand) - { - oldCommand.CanExecuteChanged -= menuItem.CanExecuteChanged; - } + oldCommand.CanExecuteChanged -= menuItem.CanExecuteChanged; + } - if (e.NewValue is ICommand newCommand) - { - newCommand.CanExecuteChanged += menuItem.CanExecuteChanged; - } + if (e.NewValue is ICommand newCommand) + { + newCommand.CanExecuteChanged += menuItem.CanExecuteChanged; } - menuItem.CanExecuteChanged(menuItem, EventArgs.Empty); + menuItem.TryUpdateCanExecute(); } } @@ -553,7 +552,7 @@ namespace Avalonia.Controls { if (e.Sender is MenuItem menuItem) { - menuItem.CanExecuteChanged(menuItem, EventArgs.Empty); + menuItem.TryUpdateCanExecute(); } } @@ -564,8 +563,29 @@ namespace Avalonia.Controls /// The event args. private void CanExecuteChanged(object sender, EventArgs e) { - var canExecute = Command == null || Command.CanExecute(CommandParameter); + TryUpdateCanExecute(); + } + /// + /// Tries to evaluate CanExecute value of a Command if menu is opened + /// + private void TryUpdateCanExecute() + { + 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); if (canExecute != _commandCanExecute) { _commandCanExecute = canExecute; @@ -635,6 +655,11 @@ namespace Avalonia.Controls if (value) { + foreach (var item in Items.OfType()) + { + item.TryUpdateCanExecute(); + } + RaiseEvent(new RoutedEventArgs(SubmenuOpenedEvent)); IsSelected = true; PseudoClasses.Add(":open"); diff --git a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs index 5f82e28722..a2bdcd1ea8 100644 --- a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs +++ b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs @@ -116,7 +116,7 @@ namespace Avalonia.Controls.Platform protected IMenu? Menu { get; private set; } - public static TimeSpan MenuShowDelay { get; set; } = TimeSpan.FromMilliseconds(400); + protected static TimeSpan MenuShowDelay { get; } = TimeSpan.FromMilliseconds(400); protected internal virtual void GotFocus(object sender, GotFocusEventArgs e) { diff --git a/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs b/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs index ebe471f303..db31d22b4f 100644 --- a/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs +++ b/tests/Avalonia.Controls.UnitTests/MenuItemTests.cs @@ -2,15 +2,21 @@ using System.Collections.Generic; using System.Text; using System.Windows.Input; +using Avalonia.Collections; +using Avalonia.Controls.Primitives; using Avalonia.Data; using Avalonia.Input; +using Avalonia.Platform; using Avalonia.UnitTests; +using Moq; using Xunit; namespace Avalonia.Controls.UnitTests { public class MenuItemTests { + private Mock popupImpl; + [Fact] public void Header_Of_Minus_Should_Apply_Separator_Pseudoclass() { @@ -79,7 +85,7 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void MenuItem_Is_Enabled_When_Bound_Command_Is_Added() + public void MenuItem_Is_Enabled_When_Added_To_Logical_Tree_And_Bound_Command_Is_Added() { var viewModel = new { @@ -91,7 +97,8 @@ namespace Avalonia.Controls.UnitTests DataContext = new object(), [!MenuItem.CommandProperty] = new Binding("Command"), }; - + var root = new TestRoot { Child = target }; + Assert.True(target.IsEnabled); Assert.False(target.IsEffectivelyEnabled); @@ -158,10 +165,11 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void MenuItem_Invokes_CanExecute_When_CommandParameter_Changed() + public void MenuItem_Invokes_CanExecute_When_Added_To_Logical_Tree_And_CommandParameter_Changed() { var command = new TestCommand(p => p is bool value && value); var target = new MenuItem { Command = command }; + var root = new TestRoot { Child = target }; target.CommandParameter = true; Assert.True(target.IsEffectivelyEnabled); @@ -169,6 +177,151 @@ namespace Avalonia.Controls.UnitTests target.CommandParameter = false; Assert.False(target.IsEffectivelyEnabled); } + + [Fact] + public void MenuItem_Does_Not_Invoke_CanExecute_When_ContextMenu_Closed() + { + using (Application()) + { + var canExecuteCallCount = 0; + var command = new TestCommand(_ => + { + canExecuteCallCount++; + return true; + }); + var target = new MenuItem(); + var contextMenu = new ContextMenu { Items = new AvaloniaList { target } }; + var window = new Window { Content = new Panel { ContextMenu = contextMenu } }; + window.ApplyTemplate(); + window.Presenter.ApplyTemplate(); + + Assert.True(target.IsEffectivelyEnabled); + target.Command = command; + Assert.Equal(0, canExecuteCallCount); + + target.CommandParameter = false; + Assert.Equal(0, canExecuteCallCount); + + command.RaiseCanExecuteChanged(); + Assert.Equal(0, canExecuteCallCount); + + contextMenu.Open(); + Assert.Equal(2, canExecuteCallCount);//2 because popup is changing logical child + + command.RaiseCanExecuteChanged(); + Assert.Equal(3, canExecuteCallCount); + + target.CommandParameter = true; + Assert.Equal(4, canExecuteCallCount); + } + } + + [Fact] + public void MenuItem_Does_Not_Invoke_CanExecute_When_MenuFlyout_Closed() + { + using (Application()) + { + var canExecuteCallCount = 0; + var command = new TestCommand(_ => + { + canExecuteCallCount++; + return true; + }); + var target = new MenuItem(); + var flyout = new MenuFlyout { Items = new AvaloniaList { target } }; + var button = new Button { Flyout = flyout }; + var window = new Window { Content = button }; + window.ApplyTemplate(); + window.Presenter.ApplyTemplate(); + + Assert.True(target.IsEffectivelyEnabled); + target.Command = command; + Assert.Equal(0, canExecuteCallCount); + + target.CommandParameter = false; + Assert.Equal(0, canExecuteCallCount); + + command.RaiseCanExecuteChanged(); + Assert.Equal(0, canExecuteCallCount); + + flyout.ShowAt(button); + Assert.Equal(2, canExecuteCallCount); + + command.RaiseCanExecuteChanged(); + Assert.Equal(3, canExecuteCallCount); + + target.CommandParameter = true; + Assert.Equal(4, canExecuteCallCount); + } + } + + [Fact] + public void MenuItem_Does_Not_Invoke_CanExecute_When_Parent_MenuItem_Closed() + { + using (Application()) + { + var canExecuteCallCount = 0; + var command = new TestCommand(_ => + { + canExecuteCallCount++; + return true; + }); + var target = new MenuItem(); + var parentMenuItem = new MenuItem { Items = new AvaloniaList { target } }; + var contextMenu = new ContextMenu { Items = new AvaloniaList { parentMenuItem } }; + var window = new Window { Content = new Panel { ContextMenu = contextMenu } }; + window.ApplyTemplate(); + window.Presenter.ApplyTemplate(); + contextMenu.Open(); + + Assert.True(target.IsEffectivelyEnabled); + target.Command = command; + Assert.Equal(0, canExecuteCallCount); + + target.CommandParameter = false; + Assert.Equal(0, canExecuteCallCount); + + command.RaiseCanExecuteChanged(); + Assert.Equal(0, canExecuteCallCount); + + try + { + parentMenuItem.IsSubMenuOpen = true; + } + catch (InvalidOperationException) + { + //popup host creation failed exception + } + Assert.Equal(1, canExecuteCallCount); + + command.RaiseCanExecuteChanged(); + Assert.Equal(2, canExecuteCallCount); + + target.CommandParameter = true; + Assert.Equal(3, canExecuteCallCount); + } + } + private IDisposable Application() + { + var screen = new PixelRect(new PixelPoint(), new PixelSize(100, 100)); + var screenImpl = new Mock(); + screenImpl.Setup(x => x.ScreenCount).Returns(1); + screenImpl.Setup(X => X.AllScreens).Returns( new[] { new Screen(1, screen, screen, true) }); + + var windowImpl = MockWindowingPlatform.CreateWindowMock(); + popupImpl = MockWindowingPlatform.CreatePopupMock(windowImpl.Object); + popupImpl.SetupGet(x => x.RenderScaling).Returns(1); + windowImpl.Setup(x => x.CreatePopup()).Returns(popupImpl.Object); + + windowImpl.Setup(x => x.Screen).Returns(screenImpl.Object); + + var services = TestServices.StyledWindow.With( + inputManager: new InputManager(), + windowImpl: windowImpl.Object, + windowingPlatform: new MockWindowingPlatform(() => windowImpl.Object, x => popupImpl.Object)); + + return UnitTestApplication.Start(services); + } private class TestCommand : ICommand { @@ -198,6 +351,8 @@ namespace Avalonia.Controls.UnitTests public bool CanExecute(object parameter) => _canExecute(parameter); public void Execute(object parameter) => _execute(parameter); + + public void RaiseCanExecuteChanged() => _canExecuteChanged?.Invoke(this, EventArgs.Empty); } } }