CommunityToolkit / dotnet

.NET Community Toolkit is a collection of helpers and APIs that work for all .NET developers and are agnostic of any specific UI platform. The toolkit is maintained and published by Microsoft, and part of the .NET Foundation.
https://docs.microsoft.com/dotnet/communitytoolkit/?WT.mc_id=dotnet-0000-bramin
Other
2.99k stars 294 forks source link

Allow RelayCommand CanExecute To Use A Function #751

Closed Axemasta closed 1 year ago

Axemasta commented 1 year ago

Overview

Currently the RelayCommand CanExecute takes a property name to use.

Xamarin forms developers, especially those that have used Prism will be used to this sort of concept with Prism's DelegateCommand using a similar design. Something that was really useful was the ability to provide a Can Execute function instead of a property, allowing for cleaner code since less boilerplate needs to be written.

Consider the following scenario:

Command CanExecute 1 Property

Given the following viewmodel that represents a screen with toggle to enable / disable a command:

image image
public partial class OnePropertyViewModel : ObservableObject
{
    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(DoSomethingCommand))]
    private bool valueOne;

    [RelayCommand(CanExecute = nameof(ValueOne))]
    private void DoSomething()
    {
        Debug.WriteLine("Doing something...");
    }
}

The existing api works perfectly and the toggling works, is simple to understand and alot of boilerplate rubbish we used to deal with is gone!

Command CanExecute 2+ Properties

When we add more conditions to our can execute, the code can get a little boilerplatey...

image image image image

As far as I could manage, you need an extra property to handle the bool logic, you can't for instance have a property that auto calculates this, it must be an ObservableProperty:

public partial class TwoPropertyViewModel : ObservableObject
{
    [ObservableProperty]
    private bool valueOne;

    [ObservableProperty]
    private bool valueTwo;

    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(DoSomethingCommand))]
    public bool bothToggled;

    [RelayCommand(CanExecute = nameof(BothToggled))]
    private void DoSomething()
    {
        Debug.WriteLine("Doing something...");
    }

    partial void OnValueOneChanged(bool value)
    {
        Debug.WriteLine($"ValueOne Changed: {value}");

        BothToggled = EvaluateBothToggled();

        Debug.WriteLine($"Both Toggled: {BothToggled}");
    }

    partial void OnValueTwoChanged(bool value)
    {
        Debug.WriteLine($"ValueTwo Changed: {value}");

        BothToggled = EvaluateBothToggled();

        Debug.WriteLine($"Both Toggled: {BothToggled}");
    }

    private bool EvaluateBothToggled()
    {
        return ValueOne && ValueTwo;
    }
}

Working in Maui if we did not use the mvvm toolkit, your viewmodel might look something like:

public class VanillaTwoPropertyViewModel : ViewModelBase
{
    private bool valueOne;

    public bool ValueOne
    {
        get => valueOne;
        set
        {
            if (SetProperty(ref valueOne, value))
            {
                DoSomethingCommand.ChangeCanExecute();
            }
        }
    }

    private bool valueTwo;

    public bool ValueTwo
    {
        get => valueTwo;
        set
        {
            if (SetProperty(ref valueTwo, value))
            {
                DoSomethingCommand.ChangeCanExecute();
            }
        }
    }

    public Command DoSomethingCommand { get; }

    public VanillaTwoPropertyViewModel()
    {
        DoSomethingCommand = new Command(DoSomething, () => ValueOne && ValueTwo);
    }

    private void DoSomething()
    {
        Debug.WriteLine("Doing something...");
    }
}

We don't need our extra property since we can pass our command the CanExecute function and all our properties need to do is raise a can execute changed notification for the command to evaluate

Adding the ability to evaluate CanExecute using a function instead of a bool will be useful to developers since it reduces the amount of code they need to write in order to implement more compelx CanExecute statements.

API breakdown

I have no strong opinions on how this api looks but here is a possible example

public partial class ProposedTwoPropertyViewModel : ViewModelBase
{
    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(BothToggled))]
    private bool valueOne;

    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(BothToggled))]
    private bool valueTwo;

    [RelayCommand(CanExecute = nameof(BothToggled))]
    private void DoSomething()
    {
        Debug.WriteLine("Doing something...");
    }

    [CanExecuteEvaluator]
    private bool BothToggled()
    {
        return ValueOne && ValueTwo;
    }
}

An added bonus is that when adding more properties, less code will need to be added to ensure BothToggled gets updated correctly

Usage example

See above

Breaking change?

No

Alternatives

Correct me if there is a way to do this, but not using the source generators is a way to achieve this however we want the source gens to be the best they can be!

Additional context

No.. I would be happy to contribute towards this change however I would need some help and guidance for specifics. I have built source generators before but this repo's code is particularly scary, a point in the right direction would be greatly appreciated 😇

Help us help you

Yes, but only if others can assist

Axemasta commented 1 year ago

Attaching source code for the samples using 1, 2, 2 vanilla properties :) RelayCommandCanExecuteSample.zip

kmgallahan commented 1 year ago

What about just doing this:

    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(DoSomethingCommand))]
    private bool valueOne;

    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(DoSomethingCommand))]
    private bool valueTwo;

    private bool BothToggled => ValueOne && ValueTwo;

    [RelayCommand(CanExecute = nameof(BothToggled))]
    private void DoSomething()
    {
        Debug.WriteLine("Doing something...");
    }

The property identified by CanExecute doesn't need to be decorated with ObserableProperty or NotifyCanExecuteChangedFor. It gets reevaluated anytime the CanExecuteChanged event is raised, which the two dependent values take care of.

Axemasta commented 1 year ago

Ah this is a simpler solution, I didnt consider this approach. This works in my sample so I'm happy to accept this as a working solution 😄

I have lots of legacy code examples which use the Func<bool> approach to CanExecute so I just automatically reached for that as a solution for writing the same behaviour in Maui. Thanks for the suggestion!