Correctly handle command.CanExecute state.

Added a new `IsEnabledCore` property to `InputElement` which is overridden in `Button` and `MenuItem` to override the `IsEffectivelyEnabled` state with the enabled state of the command.

Also add data validation of the `Command` property to `MenuItem` to make it behave the same as `Button` when `Command` is bound to a non-existent property.

Fixes #2501

src/Avalonia.Controls/Button.cs

@@ -33,8 +33,6 @@ namespace Avalonia.Controls
     /// </summary>
     public class Button : ContentControl
     {
-        private ICommand _command;
-
         /// <summary>
         /// Defines the <see cref="ClickMode"/> property.
         /// </summary>
@@ -75,6 +73,9 @@ namespace Avalonia.Controls
         public static readonly StyledProperty<bool> IsPressedProperty =
             AvaloniaProperty.Register<Button, bool>(nameof(IsPressed));
 
+        private ICommand _command;
+        private bool _commandCanExecute = true;
+
         /// <summary>
         /// Initializes static members of the <see cref="Button"/> class.
         /// </summary>
@@ -147,6 +148,8 @@ namespace Avalonia.Controls
             private set { SetValue(IsPressedProperty, value); }
         }
 
+        protected override bool IsEnabledCore => base.IsEnabledCore && _commandCanExecute; 
+
         /// <inheritdoc/>
         protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
         {
@@ -289,7 +292,11 @@ namespace Avalonia.Controls
             {
                 if (status?.ErrorType == BindingErrorType.Error)
                 {
-                    IsEnabled = false;
+                    if (_commandCanExecute)
+                    {
+                        _commandCanExecute = false;
+                        UpdateIsEffectivelyEnabled();
+                    }
                 }
             }
         }
@@ -348,9 +355,13 @@ namespace Avalonia.Controls
         /// <param name="e">The event args.</param>
         private void CanExecuteChanged(object sender, EventArgs e)
         {
-            // HACK: Just set the IsEnabled property for the moment. This needs to be changed to
+            var canExecute = Command == null || Command.CanExecute(CommandParameter);
-            // use IsEnabledCore etc. but it will do for now.
+
-            IsEnabled = Command == null || Command.CanExecute(CommandParameter);
+            if (canExecute != _commandCanExecute)
+            {
+                _commandCanExecute = canExecute;
+                UpdateIsEffectivelyEnabled();
+            }
         }
 
         /// <summary>

src/Avalonia.Controls/MenuItem.cs

@@ -9,6 +9,7 @@ using Avalonia.Controls.Generators;
 using Avalonia.Controls.Mixins;
 using Avalonia.Controls.Primitives;
 using Avalonia.Controls.Templates;
+using Avalonia.Data;
 using Avalonia.Input;
 using Avalonia.Interactivity;
 using Avalonia.LogicalTree;
@@ -20,8 +21,6 @@ namespace Avalonia.Controls
     /// </summary>
     public class MenuItem : HeaderedSelectingItemsControl, IMenuItem, ISelectable
     {
-        private ICommand _command;
-
         /// <summary>
         /// Defines the <see cref="Command"/> property.
         /// </summary>
@@ -91,9 +90,8 @@ namespace Avalonia.Controls
         private static readonly ITemplate<IPanel> DefaultPanel =
             new FuncTemplate<IPanel>(() => new StackPanel());
 
-        /// <summary>
+        private ICommand _command;
-        /// The submenu popup.
+        private bool _commandCanExecute = true;
-        /// </summary>
         private Popup _popup;
 
         /// <summary>
@@ -231,6 +229,8 @@ namespace Avalonia.Controls
         /// <inheritdoc/>
         IMenuElement IMenuItem.Parent => Parent as IMenuElement;
 
+        protected override bool IsEnabledCore => base.IsEnabledCore && _commandCanExecute;
+
         /// <inheritdoc/>
         bool IMenuElement.MoveSelection(NavigationDirection direction, bool wrap) => MoveSelection(direction, wrap);
 
@@ -400,6 +400,22 @@ namespace Avalonia.Controls
             }
         }
 
+        protected override void UpdateDataValidation(AvaloniaProperty property, BindingNotification status)
+        {
+            base.UpdateDataValidation(property, status);
+            if (property == CommandProperty)
+            {
+                if (status?.ErrorType == BindingErrorType.Error)
+                {
+                    if (_commandCanExecute)
+                    {
+                        _commandCanExecute = false;
+                        UpdateIsEffectivelyEnabled();
+                    }
+                }
+            }
+        }
+
         /// <summary>
         /// Closes all submenus of the menu item.
         /// </summary>
@@ -443,9 +459,13 @@ namespace Avalonia.Controls
         /// <param name="e">The event args.</param>
         private void CanExecuteChanged(object sender, EventArgs e)
         {
-            // HACK: Just set the IsEnabled property for the moment. This needs to be changed to
+            var canExecute = Command == null || Command.CanExecute(CommandParameter);
-            // use IsEnabledCore etc. but it will do for now.
+
-            IsEnabled = Command == null || Command.CanExecute(CommandParameter);
+            if (canExecute != _commandCanExecute)
+            {
+                _commandCanExecute = canExecute;
+                UpdateIsEffectivelyEnabled();
+            }
         }
 
         /// <summary>

src/Avalonia.Input/InputElement.cs

@@ -29,8 +29,10 @@ namespace Avalonia.Input
         /// <summary>
         /// Defines the <see cref="IsEffectivelyEnabled"/> property.
         /// </summary>
-        public static readonly StyledProperty<bool> IsEffectivelyEnabledProperty =
+        public static readonly DirectProperty<InputElement, bool> IsEffectivelyEnabledProperty =
-            AvaloniaProperty.Register<InputElement, bool>(nameof(IsEffectivelyEnabled), true);
+            AvaloniaProperty.RegisterDirect<InputElement, bool>(
+                nameof(IsEffectivelyEnabled),
+                o => o.IsEffectivelyEnabled);
 
         /// <summary>
         /// Gets or sets associated mouse cursor.
@@ -146,6 +148,7 @@ namespace Avalonia.Input
         /// </summary>
         public static readonly RoutedEvent<RoutedEventArgs> DoubleTappedEvent = Gestures.DoubleTappedEvent;
 
+        private bool _isEffectivelyEnabled = true;
         private bool _isFocused;
         private bool _isPointerOver;
 
@@ -344,31 +347,25 @@ namespace Avalonia.Input
             internal set { SetAndRaise(IsPointerOverProperty, ref _isPointerOver, value); }
         }
 
-        /// <summary>
+        /// <inheritdoc/>
-        /// Gets a value indicating whether the control is effectively enabled for user interaction.
+        public bool IsEffectivelyEnabled
-        /// </summary>
+        {
-        /// <remarks>
+            get => _isEffectivelyEnabled;
-        /// The <see cref="IsEnabled"/> property is used to toggle the enabled state for individual
+            private set => SetAndRaise(IsEffectivelyEnabledProperty, ref _isEffectivelyEnabled, value);
-        /// controls. The <see cref="IsEffectivelyEnabled"/> property takes into account the
+        }
-        /// <see cref="IsEnabled"/> value of this control and its parent controls.
+
-        /// </remarks>
+        public List<KeyBinding> KeyBindings { get; } = new List<KeyBinding>();
-        bool IInputElement.IsEffectivelyEnabled => IsEffectivelyEnabled;
 
         /// <summary>
-        /// Gets a value indicating whether the control is effectively enabled for user interaction.
+        /// Allows a derived class to override the enabled state of the control.
         /// </summary>
         /// <remarks>
-        /// The <see cref="IsEnabled"/> property is used to toggle the enabled state for individual
+        /// Derived controls may wish to disable the enabled state of the control without overwriting the
-        /// controls. The <see cref="IsEffectivelyEnabled"/> property takes into account the
+        /// user-supplied <see cref="IsEnabled"/> setting. This can be done by overriding this property
-        /// <see cref="IsEnabled"/> value of this control and its parent controls.
+        /// to return the overridden enabled state. If the value returned from <see cref="IsEnabledCore"/>
+        /// should change, then the derived control should call <see cref="UpdateIsEffectivelyEnabled()"/>.
         /// </remarks>
-        protected bool IsEffectivelyEnabled
+        protected virtual bool IsEnabledCore => IsEnabled;
-        {
-            get { return GetValue(IsEffectivelyEnabledProperty); }
-            set { SetValue(IsEffectivelyEnabledProperty, value); }
-        }
-
-        public List<KeyBinding> KeyBindings { get; } = new List<KeyBinding>();
 
         /// <summary>
         /// Focuses the control.
@@ -486,6 +483,15 @@ namespace Avalonia.Input
         {
         }
 
+        /// <summary>
+        /// Updates the <see cref="IsEffectivelyEnabled"/> property value according to the parent
+        /// control's enabled state and <see cref="IsEnabledCore"/>.
+        /// </summary>
+        protected void UpdateIsEffectivelyEnabled()
+        {
+            UpdateIsEffectivelyEnabled(this.GetVisualParent<InputElement>());
+        }
+
         private static void IsEnabledChanged(AvaloniaPropertyChangedEventArgs e)
         {
             ((InputElement)e.Sender).UpdateIsEffectivelyEnabled();
@@ -511,14 +517,6 @@ namespace Avalonia.Input
             OnPointerLeave(e);
         }
 
-        /// <summary>
-        /// Updates the <see cref="IsEffectivelyEnabled"/> property value.
-        /// </summary>
-        private void UpdateIsEffectivelyEnabled()
-        {
-            UpdateIsEffectivelyEnabled(this.GetVisualParent<InputElement>());
-        }
-
         /// <summary>
         /// Updates the <see cref="IsEffectivelyEnabled"/> property based on the parent's
         /// <see cref="IsEffectivelyEnabled"/>.
@@ -526,14 +524,7 @@ namespace Avalonia.Input
         /// <param name="parent">The parent control.</param>
         private void UpdateIsEffectivelyEnabled(InputElement parent)
         {
-            if (parent != null)
+            IsEffectivelyEnabled = IsEnabledCore && (parent?.IsEffectivelyEnabled ?? true);
-            {
-                IsEffectivelyEnabled = IsEnabled && parent.IsEffectivelyEnabled;
-            }
-            else
-            {
-                IsEffectivelyEnabled = IsEnabled;
-            }
 
             foreach (var child in this.GetVisualChildren().OfType<InputElement>())
             {

tests/Avalonia.Controls.UnitTests/ButtonTests.cs

@@ -24,11 +24,11 @@ namespace Avalonia.Controls.UnitTests
             };
             var root = new TestRoot { Child = target };
 
-            Assert.False(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
             command.IsEnabled = true;
-            Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
             command.IsEnabled = false;
-            Assert.False(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
         }
 
         [Fact]
@@ -54,7 +54,8 @@ namespace Avalonia.Controls.UnitTests
                 [!Button.CommandProperty] = new Binding("Command"),
             };
 
-            Assert.False(target.IsEnabled);
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
         }
 
         [Fact]
@@ -72,8 +73,12 @@ namespace Avalonia.Controls.UnitTests
             };
 
             Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
+
             target.DataContext = null;
-            Assert.False(target.IsEnabled);
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
         }
 
         [Fact]
@@ -90,9 +95,13 @@ namespace Avalonia.Controls.UnitTests
                 [!Button.CommandProperty] = new Binding("Command"),
             };
 
-            Assert.False(target.IsEnabled);
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+
             target.DataContext = viewModel;
+
             Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
         }
 
         [Fact]
@@ -109,9 +118,13 @@ namespace Avalonia.Controls.UnitTests
                 [!Button.CommandProperty] = new Binding("Command"),
             };
 
-            Assert.False(target.IsEnabled);
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+
             target.DataContext = viewModel;
-            Assert.False(target.IsEnabled);
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
         }
 
         [Fact]

tests/Avalonia.Controls.UnitTests/MenuItemTests.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Text;
 using System.Windows.Input;
+using Avalonia.Data;
 using Avalonia.Input;
 using Avalonia.UnitTests;
 using Xunit;
@@ -26,6 +27,103 @@ namespace Avalonia.Controls.UnitTests
             Assert.False(target.Focusable);
         }
 
+
+        [Fact]
+        public void MenuItem_Is_Disabled_When_Command_Is_Enabled_But_IsEnabled_Is_False()
+        {
+            var command = new TestCommand(true);
+            var target = new MenuItem
+            {
+                IsEnabled = false,
+                Command = command,
+            };
+
+            var root = new TestRoot { Child = target };
+
+            Assert.False(((IInputElement)target).IsEffectivelyEnabled);
+        }
+
+        [Fact]
+        public void MenuItem_Is_Disabled_When_Bound_Command_Doesnt_Exist()
+        {
+            var target = new MenuItem
+            {
+                [!MenuItem.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+        }
+
+        [Fact]
+        public void MenuItem_Is_Disabled_When_Bound_Command_Is_Removed()
+        {
+            var viewModel = new
+            {
+                Command = new TestCommand(true),
+            };
+
+            var target = new MenuItem
+            {
+                DataContext = viewModel,
+                [!MenuItem.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
+
+            target.DataContext = null;
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+        }
+
+        [Fact]
+        public void MenuItem_Is_Enabled_When_Bound_Command_Is_Added()
+        {
+            var viewModel = new
+            {
+                Command = new TestCommand(true),
+            };
+
+            var target = new MenuItem
+            {
+                DataContext = new object(),
+                [!MenuItem.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+
+            target.DataContext = viewModel;
+
+            Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
+        }
+
+        [Fact]
+        public void MenuItem_Is_Disabled_When_Disabled_Bound_Command_Is_Added()
+        {
+            var viewModel = new
+            {
+                Command = new TestCommand(false),
+            };
+
+            var target = new MenuItem
+            {
+                DataContext = new object(),
+                [!MenuItem.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+
+            target.DataContext = viewModel;
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+        }
+
         [Fact]
         public void MenuItem_Does_Not_Subscribe_To_Command_CanExecuteChanged_Until_Added_To_Logical_Tree()
         {
@@ -59,21 +157,6 @@ namespace Avalonia.Controls.UnitTests
             Assert.Equal(0, command.SubscriptionCount);
         }
 
-        [Fact]
-        public void MenuItem_Is_Disabled_When_Command_Is_Enabled_But_IsEnabled_Is_False()
-        {
-            var command = new TestCommand(true);
-            var target = new MenuItem
-            {
-                IsEnabled = false,
-                Command = command,
-            };
-
-            var root = new TestRoot { Child = target };
-
-            Assert.False(((IInputElement)target).IsEffectivelyEnabled);
-        }
-
         private class TestCommand : ICommand
         {
             private bool _enabled;

tests/Avalonia.Input.UnitTests/InputElement_Enabled.cs

@@ -0,0 +1,101 @@
+// Copyright (c) The Avalonia Project. All rights reserved.
+// Licensed under the MIT license. See licence.md file in the project root for full license information.
+
+using Avalonia.Controls;
+using Xunit;
+
+namespace Avalonia.Input.UnitTests
+{
+    public class InputElement_Enabled
+    {
+        [Fact]
+        public void IsEffectivelyEnabled_Follows_IsEnabled()
+        {
+            var target = new Decorator();
+
+            Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
+
+            target.IsEnabled = false;
+
+            Assert.False(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+        }
+
+        [Fact]
+        public void IsEffectivelyEnabled_Follows_Ancestor_IsEnabled()
+        {
+            Decorator child;
+            Decorator grandchild;
+            var target = new Decorator
+            {
+                Child = child = new Decorator
+                {
+                    Child = grandchild = new Decorator(),
+                }
+            };
+
+            Assert.True(target.IsEnabled);
+            Assert.True(target.IsEffectivelyEnabled);
+            Assert.True(child.IsEnabled);
+            Assert.True(child.IsEffectivelyEnabled);
+            Assert.True(grandchild.IsEnabled);
+            Assert.True(grandchild.IsEffectivelyEnabled);
+
+            target.IsEnabled = false;
+
+            Assert.False(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+            Assert.True(child.IsEnabled);
+            Assert.False(child.IsEffectivelyEnabled);
+            Assert.True(grandchild.IsEnabled);
+            Assert.False(grandchild.IsEffectivelyEnabled);
+        }
+
+        [Fact]
+        public void Disabled_Pseudoclass_Follows_IsEffectivelyEnabled()
+        {
+            Decorator child;
+            var target = new Decorator
+            {
+                Child = child = new Decorator()
+            };
+
+            Assert.DoesNotContain(":disabled", child.Classes);
+
+            target.IsEnabled = false;
+
+            Assert.Contains(":disabled", child.Classes);
+        }
+
+        [Fact]
+        public void IsEffectivelyEnabled_Respects_IsEnabledCore()
+        {
+            Decorator child;
+            var target = new TestControl
+            {
+                Child = child = new Decorator()
+            };
+
+            target.ShouldEnable = false;
+
+            Assert.True(target.IsEnabled);
+            Assert.False(target.IsEffectivelyEnabled);
+            Assert.True(child.IsEnabled);
+            Assert.False(child.IsEffectivelyEnabled);
+        }
+
+        private class TestControl : Decorator
+        {
+            private bool _shouldEnable;
+
+            public bool ShouldEnable
+            {
+                get => _shouldEnable;
+                set { _shouldEnable = value; UpdateIsEffectivelyEnabled(); }
+            }
+
+            protected override bool IsEnabledCore => IsEnabled && _shouldEnable; 
+        }
+    }
+}