Merge pull request #894 from jkoritzinsky/DisableButtonOnNullCommandBinding

Buttons are disabled when there is a null in the binding chain for Command

samples/BindingTest/BindingTest.csproj

@@ -66,6 +66,7 @@
     <Compile Include="ViewModels\IndeiErrorViewModel.cs" />
     <Compile Include="ViewModels\ExceptionErrorViewModel.cs" />
     <Compile Include="ViewModels\MainWindowViewModel.cs" />
+    <Compile Include="ViewModels\NestedCommandViewModel.cs" />
     <Compile Include="ViewModels\TestItem.cs" />
   </ItemGroup>
   <ItemGroup>

samples/BindingTest/MainWindow.xaml

@@ -97,8 +97,9 @@
         <Button Content="Button" Command="{Binding StringValueCommand}" CommandParameter="Button"/>
         <ToggleButton Content="ToggleButton" IsChecked="{Binding BooleanFlag, Mode=OneWay}" Command="{Binding StringValueCommand}" CommandParameter="ToggleButton"/>
         <CheckBox Content="CheckBox" IsChecked="{Binding !BooleanFlag, Mode=OneWay}" Command="{Binding StringValueCommand}" CommandParameter="CheckBox"/>
-        <RadioButton Content="RadionButton" IsChecked="{Binding !!BooleanFlag, Mode=OneWay}" Command="{Binding StringValueCommand}" CommandParameter="RadioButton"/>
+        <RadioButton Content="Radio Button" IsChecked="{Binding !!BooleanFlag, Mode=OneWay}" Command="{Binding StringValueCommand}" CommandParameter="RadioButton"/>
         <TextBox Text="{Binding Path=StringValue}"/>
+        <Button Content="Nested View Model Button" Name="NestedTest" Command="{Binding NestedModel.Command}" />
       </StackPanel>
     </TabItem>
   </TabControl>

samples/BindingTest/ViewModels/MainWindowViewModel.cs

@@ -15,6 +15,7 @@ namespace BindingTest.ViewModels
         private string _stringValue = "Simple Binding";
         private bool _booleanFlag = false;
         private string _currentTime;
+        private NestedCommandViewModel _nested;
 
         public MainWindowViewModel()
         {
@@ -39,6 +40,7 @@ namespace BindingTest.ViewModels
             {
                 BooleanFlag = !BooleanFlag;
                 StringValue = param.ToString();
+                NestedModel = _nested ?? new NestedCommandViewModel();
             });
 
             Task.Run(() =>
@@ -94,5 +96,11 @@ namespace BindingTest.ViewModels
         public DataAnnotationsErrorViewModel DataAnnotationsValidation { get; } = new DataAnnotationsErrorViewModel();
         public ExceptionErrorViewModel ExceptionDataValidation { get; } = new ExceptionErrorViewModel();
         public IndeiErrorViewModel IndeiDataValidation { get; } = new IndeiErrorViewModel();
+
+        public NestedCommandViewModel NestedModel
+        {
+            get { return _nested; }
+            private set { this.RaiseAndSetIfChanged(ref _nested, value); }
+        }
     }
 }

samples/BindingTest/ViewModels/NestedCommandViewModel.cs

@@ -0,0 +1,20 @@
+using ReactiveUI;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using System.Windows.Input;
+
+namespace BindingTest.ViewModels
+{
+    public class NestedCommandViewModel : ReactiveObject
+    {
+        public NestedCommandViewModel()
+        {
+            Command = ReactiveCommand.Create();
+        }
+
+        public ICommand Command { get; }
+    }
+}

src/Avalonia.Base/Avalonia.Base.csproj

@@ -2,6 +2,7 @@
   <PropertyGroup>
     <TargetFramework>netstandard1.3</TargetFramework>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+    <RootNamespace>Avalonia</RootNamespace>
   </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
     <DebugSymbols>true</DebugSymbols>

src/Avalonia.Base/AvaloniaObject.cs

@@ -664,24 +664,11 @@ namespace Avalonia
 
             if (notification != null)
             {
-                if (notification.ErrorType == BindingErrorType.Error)
-                {
-                    Logger.Error(
-                        LogArea.Binding,
-                        this,
-                        "Error in binding to {Target}.{Property}: {Message}",
-                        this,
-                        property,
-                        ExceptionUtilities.GetMessage(notification.Error));
-                }
-
-                if (notification.HasValue)
-                {
-                    value = notification.Value;
-                }
+                notification.LogIfError(this, property);
+                value = notification.Value;
             }
 
-            if (notification == null || notification.HasValue)
+            if (notification == null || notification.ErrorType == BindingErrorType.Error || notification.HasValue)
             {
                 var metadata = (IDirectPropertyMetadata)property.GetMetadata(GetType());
                 var accessor = (IDirectPropertyAccessor)GetRegistered(property);

src/Avalonia.Base/Data/BindingNotification.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
 using System;
+using Avalonia.Logging;
 
 namespace Avalonia.Data
 {

src/Avalonia.Base/Logging/LoggerExtensions.cs

@@ -0,0 +1,53 @@
+using System;
+using Avalonia.Data;
+
+namespace Avalonia.Logging
+{
+    internal static class LoggerExtensions
+    {
+        public static void LogIfError(
+            this BindingNotification notification,
+            object source,
+            AvaloniaProperty property)
+        {
+            if (notification.ErrorType == BindingErrorType.Error)
+            {
+                if (notification.Error is AggregateException aggregate)
+                {
+                    foreach (var inner in aggregate.InnerExceptions)
+                    {
+                        LogError(source, property, inner);
+                    }
+                }
+                else
+                {
+                    LogError(source, property, notification.Error);
+                }
+            }
+        }
+
+        private static void LogError(object source, AvaloniaProperty property, Exception e)
+        {
+            var level = LogEventLevel.Warning;
+
+            if (e is BindingChainException b &&
+                !string.IsNullOrEmpty(b.Expression) &&
+                string.IsNullOrEmpty(b.ExpressionErrorPoint))
+            {
+                // The error occurred at the root of the binding chain: it's possible that the
+                // DataContext isn't set up yet, so log at Information level instead of Warning
+                // to prevent spewing hundreds of errors.
+                level = LogEventLevel.Information;
+            }
+
+            Logger.Log(
+                level,
+                LogArea.Binding,
+                source,
+                "Error in binding to {Target}.{Property}: {Message}",
+                source,
+                property,
+                e.Message);
+        }
+    }
+}

src/Avalonia.Base/PriorityBindingEntry.cs

@@ -98,7 +98,7 @@ namespace Avalonia
 
             if (notification != null)
             {
-                if (notification.HasValue)
+                if (notification.HasValue || notification.ErrorType == BindingErrorType.Error)
                 {
                     Value = notification.Value;
                     _owner.Changed(this);

src/Avalonia.Base/PriorityValue.cs

@@ -189,14 +189,7 @@ namespace Avalonia
         /// <param name="error">The binding error.</param>
         public void LevelError(PriorityLevel level, BindingNotification error)
         {
-            Logger.Log(
-                LogEventLevel.Error,
-                LogArea.Binding,
-                Owner,
-                "Error in binding to {Target}.{Property}: {Message}",
-                Owner,
-                Property,
-                error.Error.Message);
+            error.LogIfError(Owner, Property);
         }
 
         /// <summary>

src/Avalonia.Base/Utilities/ExceptionUtilities.cs

@@ -1,23 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-
-namespace Avalonia.Utilities
-{
-    internal static class ExceptionUtilities
-    {
-        public static string GetMessage(Exception e)
-        {
-            var aggregate = e as AggregateException;
-
-            if (aggregate != null)
-            {
-                return string.Join(" | ", aggregate.InnerExceptions.Select(x => x.Message));
-            }
-
-            return e.Message;
-        }
-    }
-}

src/Avalonia.Controls/Button.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Linq;
 using System.Windows.Input;
+using Avalonia.Data;
 using Avalonia.Input;
 using Avalonia.Interactivity;
 using Avalonia.Rendering;
@@ -41,8 +42,9 @@ namespace Avalonia.Controls
         /// <summary>
         /// Defines the <see cref="Command"/> property.
         /// </summary>
-        public static readonly StyledProperty<ICommand> CommandProperty =
-            AvaloniaProperty.Register<Button, ICommand>(nameof(Command));
+        public static readonly DirectProperty<Button, ICommand> CommandProperty =
+            AvaloniaProperty.RegisterDirect<Button, ICommand>(nameof(Command),
+                button => button.Command, (button, command) => button.Command = command, enableDataValidation: true);
 
         /// <summary>
         /// Defines the <see cref="HotKey"/> property.
@@ -68,6 +70,8 @@ namespace Avalonia.Controls
         public static readonly RoutedEvent<RoutedEventArgs> ClickEvent =
             RoutedEvent.Register<Button, RoutedEventArgs>("Click", RoutingStrategies.Bubble);
 
+        private ICommand _command;
+
         /// <summary>
         /// Initializes static members of the <see cref="Button"/> class.
         /// </summary>
@@ -102,8 +106,8 @@ namespace Avalonia.Controls
         /// </summary>
         public ICommand Command
         {
-            get { return GetValue(CommandProperty); }
-            set { SetValue(CommandProperty, value); }
+            get { return _command; }
+            set { SetAndRaise(CommandProperty, ref _command, value); }
         }
 
         /// <summary>
@@ -250,6 +254,18 @@ namespace Avalonia.Controls
             }
         }
 
+        protected override void UpdateDataValidation(AvaloniaProperty property, BindingNotification status)
+        {
+            base.UpdateDataValidation(property, status);
+            if(property == CommandProperty)
+            {
+                if(status?.ErrorType == BindingErrorType.Error)
+                {
+                    IsEnabled = false;
+                }
+            }
+        }
+
         /// <summary>
         /// Called when the <see cref="Command"/> property changes.
         /// </summary>

src/Avalonia.Controls/MenuItem.cs

@@ -24,7 +24,7 @@ namespace Avalonia.Controls
         /// Defines the <see cref="Command"/> property.
         /// </summary>
         public static readonly StyledProperty<ICommand> CommandProperty =
-            Button.CommandProperty.AddOwner<MenuItem>();
+            AvaloniaProperty.Register<MenuItem, ICommand>(nameof(Command));
 
         /// <summary>
         /// Defines the <see cref="HotKey"/> property.

src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs

@@ -239,17 +239,7 @@ namespace Avalonia.Markup.Data
 
                 if (broken != null)
                 {
-                    // We've received notification of a broken expression due to a null value
-                    // somewhere in the chain. If this null value occurs at the first node then we
-                    // ignore it, as its likely that e.g. the DataContext has not yet been set up.
-                    if (broken.HasNodes)
-                    {
-                        broken.Commit(Description);
-                    }
-                    else
-                    {
-                        o = AvaloniaProperty.UnsetValue;
-                    }
+                    broken.Commit(Description);
                 }
                 return o;
             }

src/Markup/Avalonia.Markup/Data/MarkupBindingChainException.cs

@@ -32,10 +32,12 @@ namespace Avalonia.Markup.Data
         public void Commit(string expression)
         {
             Expression = expression;
-            ExpressionErrorPoint = string.Join(".", _nodes.Reverse())
-                .Replace(".!", "!")
-                .Replace(".[", "[")
-                .Replace(".^", "^");
+            ExpressionErrorPoint = _nodes != null ?
+                string.Join(".", _nodes.Reverse())
+                    .Replace(".!", "!")
+                    .Replace(".[", "[")
+                    .Replace(".^", "^") :
+                string.Empty;
             _nodes = null;
         }
     }

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -314,7 +314,7 @@ namespace Avalonia.Base.UnitTests
                 new InvalidOperationException("Foo"),
                 BindingErrorType.Error));
 
-            Assert.Equal(6.7, target.GetValue(Class1.QuxProperty));
+            Assert.Equal(5.6, target.GetValue(Class1.QuxProperty));
         }
 
         [Fact]
@@ -343,7 +343,7 @@ namespace Avalonia.Base.UnitTests
 
             LogCallback checkLogMessage = (level, area, src, mt, pv) =>
             {
-                if (level == LogEventLevel.Error &&
+                if (level == LogEventLevel.Warning &&
                     area == LogArea.Binding &&
                     mt == expectedMessageTemplate)
                 {
@@ -359,7 +359,7 @@ namespace Avalonia.Base.UnitTests
                     new InvalidOperationException("Foo"),
                     BindingErrorType.Error));
 
-                Assert.Equal(6.7, target.GetValue(Class1.QuxProperty));
+                Assert.Equal(5.6, target.GetValue(Class1.QuxProperty));
                 Assert.True(called);
             }
         }

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs

@@ -352,14 +352,14 @@ namespace Avalonia.Base.UnitTests
         }
 
         [Fact]
-        public void BindingError_Does_Not_Cause_Target_Update()
+        public void DataValidationError_Does_Not_Cause_Target_Update()
         {
             var target = new Class1();
             var source = new Subject<object>();
 
             target.Bind(Class1.FooProperty, source);
             source.OnNext("initial");
-            source.OnNext(new BindingNotification(new InvalidOperationException("Foo"), BindingErrorType.Error));
+            source.OnNext(new BindingNotification(new InvalidOperationException("Foo"), BindingErrorType.DataValidationError));
 
             Assert.Equal("initial", target.GetValue(Class1.FooProperty));
         }
@@ -389,7 +389,7 @@ namespace Avalonia.Base.UnitTests
 
             LogCallback checkLogMessage = (level, area, src, mt, pv) =>
             {
-                if (level == LogEventLevel.Error &&
+                if (level == LogEventLevel.Warning &&
                     area == LogArea.Binding &&
                     mt == "Error in binding to {Target}.{Property}: {Message}" &&
                     pv.Length == 3 &&

tests/Avalonia.Controls.UnitTests/ButtonTests.cs

@@ -0,0 +1,125 @@
+using System;
+using System.Windows.Input;
+using Avalonia.Markup.Xaml.Data;
+using Xunit;
+
+namespace Avalonia.Controls.UnitTests
+{
+    public class ButtonTests
+    {
+        [Fact]
+        public void Button_Is_Disabled_When_Command_Is_Disabled()
+        {
+            var command = new TestCommand(false);
+            var target = new Button
+            {
+                Command = command,
+            };
+
+            Assert.False(target.IsEnabled);
+            command.IsEnabled = true;
+            Assert.True(target.IsEnabled);
+            command.IsEnabled = false;
+            Assert.False(target.IsEnabled);
+        }
+
+        [Fact]
+        public void Button_Is_Disabled_When_Bound_Command_Doesnt_Exist()
+        {
+            var target = new Button
+            {
+                [!Button.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.False(target.IsEnabled);
+        }
+
+        [Fact]
+        public void Button_Is_Disabled_When_Bound_Command_Is_Removed()
+        {
+            var viewModel = new
+            {
+                Command = new TestCommand(true),
+            };
+
+            var target = new Button
+            {
+                DataContext = viewModel,
+                [!Button.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.True(target.IsEnabled);
+            target.DataContext = null;
+            Assert.False(target.IsEnabled);
+        }
+
+        [Fact]
+        public void Button_Is_Enabled_When_Bound_Command_Is_Added()
+        {
+            var viewModel = new
+            {
+                Command = new TestCommand(true),
+            };
+
+            var target = new Button
+            {
+                DataContext = new object(),
+                [!Button.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.False(target.IsEnabled);
+            target.DataContext = viewModel;
+            Assert.True(target.IsEnabled);
+        }
+
+        [Fact]
+        public void Button_Is_Disabled_When_Disabled_Bound_Command_Is_Added()
+        {
+            var viewModel = new
+            {
+                Command = new TestCommand(false),
+            };
+
+            var target = new Button
+            {
+                DataContext = new object(),
+                [!Button.CommandProperty] = new Binding("Command"),
+            };
+
+            Assert.False(target.IsEnabled);
+            target.DataContext = viewModel;
+            Assert.False(target.IsEnabled);
+        }
+
+        private class TestCommand : ICommand
+        {
+            private bool _enabled;
+
+            public TestCommand(bool enabled)
+            {
+                _enabled = enabled;
+            }
+
+            public bool IsEnabled
+            {
+                get { return _enabled; }
+                set
+                {
+                    if (_enabled != value)
+                    {
+                        _enabled = value;
+                        CanExecuteChanged?.Invoke(this, EventArgs.Empty);
+                    }
+                }
+            }
+
+            public event EventHandler CanExecuteChanged;
+
+            public bool CanExecute(object parameter) => _enabled;
+
+            public void Execute(object parameter)
+            {
+            }
+        }
+    }
+}

tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs

@@ -67,49 +67,69 @@ namespace Avalonia.Markup.UnitTests.Data
         }
 
         [Fact]
-        public async Task Should_Return_UnsetValue_For_Root_Null()
+        public async Task Should_Return_BindingNotification_Error_For_Root_Null()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(default(object), "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(AvaloniaProperty.UnsetValue, result);
+            Assert.Equal(
+                new BindingNotification(
+                        new MarkupBindingChainException("Null value", "Foo", string.Empty),
+                        BindingErrorType.Error,
+                        AvaloniaProperty.UnsetValue),
+                result);
 
             GC.KeepAlive(data);
         }
 
         [Fact]
-        public async Task Should_Return_UnsetValue_For_Root_UnsetValue()
+        public async Task Should_Return_BindingNotification_Error_For_Root_UnsetValue()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(AvaloniaProperty.UnsetValue, "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(AvaloniaProperty.UnsetValue, result);
+            Assert.Equal(
+                new BindingNotification(
+                        new MarkupBindingChainException("Null value", "Foo", string.Empty),
+                        BindingErrorType.Error,
+                        AvaloniaProperty.UnsetValue),
+                result);
 
             GC.KeepAlive(data);
         }
 
         [Fact]
-        public async Task Should_Return_UnsetValue_For_Observable_Root_Null()
+        public async Task Should_Return_BindingNotification_Error_For_Observable_Root_Null()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(Observable.Return(default(object)), "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(AvaloniaProperty.UnsetValue, result);
+            Assert.Equal(
+                new BindingNotification(
+                        new MarkupBindingChainException("Null value", "Foo", string.Empty),
+                        BindingErrorType.Error,
+                        AvaloniaProperty.UnsetValue),
+                result);
 
             GC.KeepAlive(data);
         }
 
         [Fact]
-        public async Task Should_Return_UnsetValue_For_Observable_Root_UnsetValue()
+        public async void Should_Return_BindingNotification_Error_For_Observable_Root_UnsetValue()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(Observable.Return(AvaloniaProperty.UnsetValue), "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(AvaloniaProperty.UnsetValue, result);
+            Assert.Equal(
+                new BindingNotification(
+                        new MarkupBindingChainException("Null value", "Foo", string.Empty),
+                        BindingErrorType.Error,
+                        AvaloniaProperty.UnsetValue),
+                result);
 
             GC.KeepAlive(data);
         }
@@ -117,7 +137,7 @@ namespace Avalonia.Markup.UnitTests.Data
         [Fact]
         public async Task Should_Get_Simple_Property_Chain()
         {
-            var data = new { Foo = new { Bar = new { Baz = "baz" } }  };
+            var data = new { Foo = new { Bar = new { Baz = "baz" } } };
             var target = new ExpressionObserver(data, "Foo.Bar.Baz");
             var result = await target.Take(1);
 
@@ -215,7 +235,7 @@ namespace Avalonia.Markup.UnitTests.Data
             var target = new ExpressionObserver(data, "Bar");
             var result = new List<object>();
 
-            var sub = target.Subscribe(x => result.Add(x));            
+            var sub = target.Subscribe(x => result.Add(x));
 
             Assert.Equal(new[] { "foo" }, result);
 
@@ -305,7 +325,7 @@ namespace Avalonia.Markup.UnitTests.Data
             data.Next = old;
 
             Assert.Equal(
-                new object[] 
+                new object[]
                 {
                     "bar",
                     new BindingNotification(
@@ -313,7 +333,7 @@ namespace Avalonia.Markup.UnitTests.Data
                         BindingErrorType.Error,
                         AvaloniaProperty.UnsetValue),
                     "bar"
-                }, 
+                },
                 result);
 
             sub.Dispose();
@@ -473,7 +493,7 @@ namespace Avalonia.Markup.UnitTests.Data
         [Fact]
         public void SetValue_Should_Return_False_For_Missing_Property()
         {
-            var data = new Class1 { Next = new WithoutBar()};
+            var data = new Class1 { Next = new WithoutBar() };
             var target = new ExpressionObserver(data, "Next.Bar");
 
             using (target.Subscribe(_ => { }))
@@ -545,12 +565,15 @@ namespace Avalonia.Markup.UnitTests.Data
             update.OnNext(Unit.Default);
 
             Assert.Equal(
-                new object[] 
+                new object[]
                 {
                     "foo",
                     "bar",
-                    AvaloniaProperty.UnsetValue,
-                }, 
+                    new BindingNotification(
+                        new MarkupBindingChainException("Null value", "Foo", string.Empty),
+                        BindingErrorType.Error,
+                        AvaloniaProperty.UnsetValue)
+                },
                 result);
 
             Assert.Equal(0, first.PropertyChangedSubscriptionCount);

tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs

@@ -302,12 +302,12 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data
 
             // Bind Foo and Bar to the VM.
             target.Bind(OldDataContextTest.FooProperty, fooBinding);
-            //target.Bind(OldDataContextTest.BarProperty, barBinding);
+            target.Bind(OldDataContextTest.BarProperty, barBinding);
             target.DataContext = vm;
 
             // Make sure the control's Foo and Bar properties are read from the VM
             Assert.Equal(1, target.GetValue(OldDataContextTest.FooProperty));
-            //Assert.Equal(2, target.GetValue(OldDataContextTest.BarProperty));
+            Assert.Equal(2, target.GetValue(OldDataContextTest.BarProperty));
 
             // Set DataContext to null.
             target.DataContext = null;

tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ControlBindingTests.cs

@@ -37,7 +37,7 @@ namespace Avalonia.Markup.Xaml.UnitTests.Xaml
 
             LogCallback checkLogMessage = (level, area, src, mt, pv) =>
             {
-                if (level == LogEventLevel.Error &&
+                if (level == LogEventLevel.Warning &&
                     area == LogArea.Binding &&
                     mt == "Error in binding to {Target}.{Property}: {Message}" &&
                     pv.Length == 3 &&