Skip to content

Commit

Permalink
fix(Button): Command binding resetting on CanExecution exception
Browse files Browse the repository at this point in the history
  • Loading branch information
Xiaoy312 committed Oct 3, 2024
1 parent e1aa19f commit 82846ba
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.UI.Xaml.Markup;
using Microsoft.UI.Xaml.Controls.Primitives;
using Color = Windows.UI.Color;
using Microsoft.UI.Xaml.Data;

#if HAS_UNO_WINUI || WINAPPSDK || WINUI
using Colors = Microsoft.UI.Colors;
Expand Down Expand Up @@ -317,6 +318,26 @@ public async Task When_Button_Flyout_TemplateBinding()
}
#endif

[TestMethod]
public async Task When_Command_CanExecute_Throws()
{
// Here we are testing against a bug where
// a data-bound ICommand that throws in its CanExecute
// can cause the binding to reset to its FallbackValue.
var vm = new
{
UnstableCommand = new DelegateCommand(x => throw new Exception("fail...")),

Check warning on line 329 in src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Button.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Button.cs#L329

'System.Exception' should not be thrown by user code.
};

var sut = new Button();
sut.SetBinding(Button.CommandProperty, new Binding { Path = new(nameof(vm.UnstableCommand)), FallbackValue = new NoopCommand() });
sut.DataContext = vm;

await UITestHelper.Load(sut, x => x.IsLoaded);

Assert.AreEqual(vm.UnstableCommand, sut.Command, "Binding did not set the proper value.");
}

private async Task RunIsExecutingCommandCommon(IsExecutingCommand command)
{
void FocusManager_LosingFocus(object sender, LosingFocusEventArgs e)
Expand Down Expand Up @@ -421,5 +442,26 @@ public void Execute(object parameter)
CanExecuteChanged?.Invoke(this, EventArgs.Empty);
}
}

public class DelegateCommand : ICommand
{
private readonly Func<object, bool> canExecuteImpl;
private readonly Action<object> executeImpl;

public event EventHandler CanExecuteChanged;

public DelegateCommand(Func<object, bool> canExecute = null, Action<object> execute = null)
{
this.canExecuteImpl = canExecute;
this.executeImpl = execute;
}

public void RaiseCanExecuteChanged() => CanExecuteChanged?.Invoke(this, default);

public bool CanExecute(object parameter) => canExecuteImpl?.Invoke(parameter) ?? true;
public void Execute(object parameter) => executeImpl?.Invoke(parameter);
}

public class NoopCommand() : DelegateCommand(null, null) { }

Check notice on line 465 in src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Button.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_Button.cs#L465

Remove this default value assigned to parameter 'execute'.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System;
using System.Windows.Input;
using Uno.Disposables;
using Uno.Foundation.Logging;
using Uno.UI.Xaml.Core;
using Windows.Devices.Input;
using Windows.Foundation;
Expand Down Expand Up @@ -86,7 +87,7 @@ internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs arg
}
else if (args.Property == CommandParameterProperty)
{
UpdateCanExecute();
UpdateCanExecuteSafe();
}
else if (args.Property == VisibilityProperty)
{
Expand Down Expand Up @@ -273,7 +274,7 @@ void CanExecuteChangedHandler(object? sender, object args)
}

// Coerce the button enabled state with the CanExecute state of the command.
UpdateCanExecute();
UpdateCanExecuteSafe();
}

/// <summary>
Expand All @@ -297,6 +298,24 @@ private void UpdateCanExecute()
SuppressIsEnabled(suppress);
}

private void UpdateCanExecuteSafe()
{
// uno specific workaround:
// If Button::Command binding produces an ICommand value that throws Exception in its CanExecute,
// this value will be canceled and replaced by the Binding::FallbackValue.
try
{
UpdateCanExecute();
}
catch (Exception e)
{
if (this.Log().IsEnabled(LogLevel.Error))
{
this.Log().Error($"Failed to update CanExecute", e);
}
}
}

/// <summary>
/// Executes ButtonBase.Command.
/// </summary>
Expand Down

0 comments on commit 82846ba

Please sign in to comment.