dotnet icon indicating copy to clipboard operation
dotnet copied to clipboard

[NotifyCanExecuteChangedFor] not work when the property changed in Async environment

Open lqy16 opened this issue 1 year ago • 3 comments

Describe the bug

If a property is labeled with [ObservableProperty] and [NotifyCanExecuteChangedFor], e.g. [NotifyCanExecuteChangedFor(nameof(ClickCommand))] [ObservableProperty] public bool indicator; it should notify the property (e.g., bool CanExecute) which decides whether ClickCommand is executable when it changes. When Indicator is changed in Sync environment, all goes well. However, when Indicator is changed in Aync environment, e.g., Task.Run(() => Indicator = false);, it doesn't notify that CanExecute should change, and the executable status of ClickCommand doesn't change.

Regression

No response

Steps to reproduce

It can be easily reproduced with a simple demo. Create a new default WPF project in Visual Studio 2022 with .NET 6.0. Then

1. Modify the main content of MainWindow.xaml file into:
<Window
x:Class="NotifyCanExecuteChangedForBugTest.MainWindow"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:local="clr-namespace:NotifyCanExecuteChangedForBugTest"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
Title="MainWindow"
Width="400"
Height="100"
d:DataContext="{d:DesignInstance local:MainWindowViewModel}"
mc:Ignorable="d">
<Grid>
<StackPanel HorizontalAlignment="Center" Orientation="Horizontal">
<Button Width="100" Height="30" Margin="10,0" Command="{Binding Path=ChangeIndicatorSyncCommand}" Content="Sync" />
<Button Width="100" Height="30" Margin="10,0" Command="{Binding Path=ChangeIndicatorAsyncCommand}" Content="Async" />
<Button Width="100" Height="30" Margin="10,0" Command="{Binding Path=ClickCommand}" Content="Test" />
</StackPanel>
</Grid>
</Window>```"xaml"

2. Add File MainWindowViewModel.cs:
 System.Threading.Tasks;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
namespace NotifyCanExecuteChangedForBugTest
{
partial class MainWindowViewModel : ObservableRecipient
{
public MainWindowViewModel()
{
Indicator = true;
}
[NotifyCanExecuteChangedFor(nameof(ClickCommand))]
[ObservableProperty]
public bool indicator;
private bool CanClick => Indicator;
[RelayCommand(CanExecute = nameof(CanClick))]
private void Click() { }
[RelayCommand]
private void ChangeIndicatorSync()
{
Indicator = false;
}
[RelayCommand]
private void ChangeIndicatorAsync()
{
Task.Run(() => Indicator = false);
}
}
}```"csharp"

3. Add one line in MainWindow.xaml.cs
```DataContext = new MainWindowViewModel();```"csharp"

4. Run the project. (See Screenshots) Initially, the Button "Test" is executable. When "Sync" is clicked, the property Indicator is changed to False, and "Test" is not executable any more. However, when "Async" is clicked, the property Indicator is changed to False in async environment, but "Test" is still executable.

Expected behavior

when "Async" is clicked, "Test" should also become inexecutable.

Screenshots

demo

IDE and version

VS 2022

IDE version

17.5.5

Nuget packages

  • [ ] CommunityToolkit.Common
  • [ ] CommunityToolkit.Diagnostics
  • [ ] CommunityToolkit.HighPerformance
  • [X] CommunityToolkit.Mvvm (aka MVVM Toolkit)

Nuget package version(s)

8.2.1

Additional context

No response

Help us help you

Yes, I'd like to be assigned to work on this item

lqy16 avatar Oct 24 '23 07:10 lqy16

Currently, I use IValueConverter and IsEnabled to directly bind the excutable status of the Button to the property. That works.

lqy16 avatar Oct 24 '23 07:10 lqy16

Hi there

In my opinion this is not a bug.

According to RelayCommand attribute - Asynchronous commands the command method should return a Task type for this.

[RelayCommand]
private async Task ChangeIndicatorAsync() { ... }

The command is then accessible in xaml as this:

Command="{Binding Path=ChangeIndicatorCommand}"

I also think you should use Dispatcher.BeginInvoke, to update the bounded Properties, when you do your work within another Task.

Here is my working example:

public partial class MainWindowViewModel : ObservableRecipient
{
    public MainWindowViewModel()
    {
        Counter = 1;
        Indicator = true;
    }

    [NotifyCanExecuteChangedFor(nameof(ClickCommand))]
    [ObservableProperty]
    public int counter;

    [NotifyCanExecuteChangedFor(nameof(ClickCommand))]
    [ObservableProperty]
    public bool indicator;

    [RelayCommand(CanExecute = nameof(Indicator))]
    private void Click() => Counter++;

    [RelayCommand]
    private void ChangeIndicatorSync()
    {
        Counter = 0;
        Indicator = !Indicator;
    }

    [RelayCommand]
    private async Task ChangeIndicatorAsync()
    {
        var newIndicatorValue = false;
        await Task.Run(() => newIndicatorValue = !Indicator);
        await Task.Delay(3000);
        await Dispatcher.CurrentDispatcher.BeginInvoke(() =>
        {
            Counter = 0;
            Indicator = newIndicatorValue;
        });
    }
}
<Grid>
    <StackPanel HorizontalAlignment="Center" Orientation="Horizontal">
        <Button Width="100" Height="30" Margin="10,0" Command="{Binding Path=ChangeIndicatorSyncCommand}" Content="Sync" />
        <Button Width="100" Height="30" Margin="10,0" Command="{Binding Path=ChangeIndicatorCommand}" Content="Async" />
        <Button Width="100" Height="30" Margin="10,0" Command="{Binding Path=ClickCommand}" Content="{Binding Counter}" />
    </StackPanel>
</Grid>

best regards

JochenMader avatar Oct 24 '23 13:10 JochenMader

It is redundant to update an observable property in Dispatcher as WPF would automatically marshal the change to UI thread no matter where the PropertyChanged event occurs. On the other hand, the CanExecuteChanged does not have this nice feature and I think it would be great to wrap it for parity.

wfjsw avatar Dec 30 '23 08:12 wfjsw