dotnet icon indicating copy to clipboard operation
dotnet copied to clipboard

[Bug] RelayCommand does not update CanExecute status

Open JavierMarcuzzi opened this issue 2 years ago • 13 comments

Description

I am performing it on Net Maui, Mac and iOS, they fail, on Android it usually works. On windows I did not test it.

I will copy the code so you can reproduce it.

I enter the first name, then the last name, but I add one letter less to the first name so that it does not meet the validation rule, then I add the correct, ie, one more letter to name, so the validation is correct, but the button no longer works. And if I use the version with CanShow, it does not update its status on the screen, it does not react to changes in the number of letters, but internally it is detected and writes the message according to the result. It is easier to reproduce it than to explain it. You can see that the button with the text try Save Valida Command does not work the second time, when everything is valid, if I close and open the program it works but only the first time.

Reproduction Sample file UI

public class MainPageCSharpMarkupValidationfluentDI : ContentPage { int count = 0; readonly vmPersona _vm;

    public MainPageCSharpMarkupValidationfluentDI(vmPersona vm)
    {
        BindingContext = _vm = vm;

        var mi_scrollView = new ScrollView();
        var mi_verticalStacklayout = new VerticalStackLayout
        {
            Spacing = 25,
            Children =
            {
                new Label
                {
                    Text="Validacion C# Markpu DI!",
                }
                .Font(size: 32).CenterHorizontal(),

                 new Label().Bind(Label.TextProperty, nameof(vm.Mensaje)),
                 new Label().Bind(Label.TextProperty, (nameof(vm.CanShow)).ToString()),
                  new Button()
                             .Text("ver si esta forma anda")
                             .BindCommand(nameof(vm.ShowCommand))
                             .CenterHorizontal(),

                 new Entry()
                {
                    Keyboard = Keyboard.Chat,
                    BackgroundColor = Colors.AliceBlue,
                }.FontSize(15)
                .Placeholder("Enter name")
                .TextColor(Colors.Black)
                .Height(44)
                .Margin(5, 5)
                .Bind(Entry.TextProperty, nameof(vm.Nombre)),

                 new Label().Bind(Label.TextProperty, nameof(vm.Nombre)),

                 new HorizontalStackLayout
                 {
                     Children=
                     {
                        new VerticalStackLayout
                         {
                             Children=
                            {
                                new Label {Text="First Name"},
                                new Entry().FontSize(15)
                                .Placeholder("First Name")
                                .TextColor(Colors.Black)
                                .Height(44)
                                .Margin(5, 5)
                                .Width(300)
                                .Bind(Entry.TextProperty, nameof(vm.FirstName)),

                                new Label().Bind(Label.TextProperty, nameof(vm.FirstName)),

                            }
                         },
                        new VerticalStackLayout
                         {
                             Children=
                            {
                                new Label {Text="Last Name"},
                                new Entry()
                                .FontSize(15)
                                .Placeholder("last Name")
                                .TextColor(Colors.Black)
                                .Height(44)
                                .Margin(5, 5)
                                .Width(300)
                                .Bind(Entry.TextProperty, nameof(vm.LastName)),

                                new Label().Bind(Label.TextProperty, nameof(vm.LastName)),

                              
                            }
                         }

                     }
                 }.CenterHorizontal(),
                     new VerticalStackLayout
                         {
                             Children=
                            {
                             new Button()
                             .Text("try Save Valida Command")
                             .BindCommand(nameof(vm.SaveValidaCommand))
                             .CenterHorizontal()
                            }
                         },

                new Label().Bind(Label.TextProperty, nameof(vm.FullName)),
                 new Label().Bind(Label.TextProperty, nameof(vm.FullNameValida)),


                new Label{Text="Ver respuesta de Validación Fluent ....."}
                .Font(size: 24, bold:true).CenterHorizontal(),

                new Label{Text="ErrorMensajePasa", TextColor=Colors.BlueViolet},
                new Label().Bind(Label.TextProperty, nameof(vm.ErrorMensajePasa)),
                 new Label{Text="ErrorMensajeGeneral", TextColor=Colors.BlueViolet},
                new Label().Bind(Label.TextProperty, nameof(vm.ErrorMensajeGeneral)),
                 new Label{Text="FirtNameError", TextColor=Colors.BlueViolet},
                new Label().Bind(Label.TextProperty, nameof(vm.FirtNameError)),
                 new Label{Text="LastNameError", TextColor=Colors.BlueViolet},
                new Label().Bind(Label.TextProperty, nameof(vm.LastNameError)),


                new Label{Text="Fin respuesta Validación, inicio para ver si anda el mvvm ..."}
                .Font(size: 24, bold:true).CenterHorizontal(),

                new Button()
                .Text("Valida Excecute SingUpCommand")
                .Font(bold: true)
                .BindCommand(nameof(vm.ExecuteSignUpCommand))
                .CenterHorizontal(),

            }
        }.Paddings(30, 30, 30, 30);

        mi_scrollView.Content = mi_verticalStacklayout;
        Content = mi_scrollView;
    }

}

Reproduction Sample file FluentValidation

public class vPersona : AbstractValidator<mPersona> { public vPersona() { RuleFor(x => x.Name).NotNull().Length(5, 20).NotEqual("Javier"); RuleFor(x => x.Surname).NotNull().Length(4, 20); } } public class mPersona { public string Name { get; set; } public string Surname { get; set; } }

Reproduction Sample file mvvm

public partial class vmPersona : ObservableObject { public mPersona PersonaObj { get; set; } private readonly vPersona _validator;

    public vmPersona()
    {
        _validator = new vPersona();
    }

    [ObservableProperty]
   // [AlsoNotifyChangeFor(nameof(ExecuteSignUpCommand))]
    [NotifyCanExecuteChangedFor(nameof(ExecuteSignUpCommand))]
    //  [AlsoNotifyChangeFor(nameof(MensajeValidaNombre))]
    string nombre;


    [ObservableProperty]
    [NotifyPropertyChangedFor(nameof(FullName))]
    // [AlsoNotifyChangeFor(nameof(FullName))]
    //  [NotifyCanExecuteChangedFor(nameof(validaNuevoJavier))]
    [NotifyCanExecuteChangedFor(nameof(ShowCommand))]
    private string firstName = string.Empty;

    [ObservableProperty]
    [NotifyPropertyChangedFor(nameof(FullName))]
    //[AlsoNotifyChangeFor(nameof(FullName))]
    //      [NotifyPropertyChangedFor(nameof(validaNuevoJavier))]
    [NotifyCanExecuteChangedFor(nameof(ShowCommand))]
    private string lastName = string.Empty;

    public string FullName => $"{FirstName} {LastName}";

    [ObservableProperty]
    [NotifyPropertyChangedFor(nameof(FullNameValida))]
    //[AlsoNotifyChangeFor(nameof(FullNameValida))]
    [NotifyCanExecuteChangedFor(nameof(SaveValidaCommand))]
    //  [NotifyCanExecuteChangedFor(nameof(validaNuevoJavier))]
    private string firstNameValida = string.Empty;

    [ObservableProperty]
    [NotifyPropertyChangedFor(nameof(FullNameValida))]
   // [AlsoNotifyChangeFor(nameof(FullNameValida))]
    [NotifyCanExecuteChangedFor(nameof(SaveValidaCommand))]
    //      [NotifyPropertyChangedFor(nameof(validaNuevoJavier))]
    private string lastNameValida = string.Empty;

    public string FullNameValida => $"{FirstNameValida} {LastNameValida}";


    [RelayCommand(CanExecute = nameof(CanSaveExecute), IncludeCancelCommand = true)]
   // [ICommand(CanExecute = nameof(CanSaveExecute), IncludeCancelCommand = true)]
    private async Task SaveValidaAsync(CancellationToken cancelToken)
    {
        // Code to save the user details  ....  CanSaveExecute
        await Task.FromResult(0);
    }

    private bool CanSaveExecute()
  => !string.IsNullOrWhiteSpace(FirstName) && !string.IsNullOrWhiteSpace(LastName);

    [ObservableProperty]
    string errorMensajePasa = string.Empty;

    [ObservableProperty]
    string errorMensajeGeneral = string.Empty;


    [ObservableProperty]
    string firtNameError = string.Empty;

    [ObservableProperty]
    string lastNameError = string.Empty;

   
    // [ICommand]
    [RelayCommand]
    async Task ExecuteSignUpAsync()
    {
        await Shell.Current.DisplayAlert("Probar botón!",
               $"Parece que anda", "OK");

        PersonaObj = new mPersona
        {
            Name = FirstName,
            Surname = LastName
        };
        var validationResult = _validator.Validate(PersonaObj);
        if (validationResult.IsValid)
        {
            ErrorMensajePasa = "Validation Success..!!";
            ErrorMensajeGeneral = string.Empty;
        }
        else
        {
            ErrorMensajePasa = "Validation Failes..!!";

            ErrorMensajeGeneral = validationResult.ToString();

          //  var nError = validationResult.Errors.Where(e => e.PropertyName.Contains("Name"));
            var nError = validationResult.Errors.Where(e => e.PropertyName.Contains(nameof(PersonaObj.Name)));
            var sError = validationResult.Errors.Where(e => e.PropertyName.Contains("Surname"));

            foreach (var item in nError)
            {
                FirtNameError = item.ErrorMessage + Environment.NewLine;
            }

            foreach (var item in sError)
            {
                LastNameError = item.ErrorMessage + Environment.NewLine;
            }
        }
    }

    public bool CanShow { get => firstName.Length > 2 && lastName.Length > 2; }

    [ObservableProperty]
    string mensaje = string.Empty;

    //  [ICommand(CanExecute = nameof(CanShow))]
    [RelayCommand(CanExecute = nameof(CanShow))]
    private void Show()
    {
        //  MessageBox.Show(FullName);
        Mensaje = FullName;
    }

    partial void OnFirstNameChanged(string value)
    {
        if (CanShow)
        {
            //                MessageBox.Show($"Execute Custom code on {value}");
            Mensaje = $"Execute Custom code on {value}";
        }
    }

}

Steps to Reproduce

  1. In field Name, write Javier, this is not valid name.
  2. Click
  3. In field Name, write JavierX, this is valid name.
  4. Click don't work6. 6 The first button, change status, but, status word don't change.

Reproduction imagery

JavierMarcuzzi avatar Jun 15 '22 14:06 JavierMarcuzzi

Transferring this Issue to CommunityToolkit/dotnet

brminnick avatar Jun 15 '22 15:06 brminnick

That's... A lot of code 😅 @JavierMarcuzzi Can you share a minimal repro, possibly with no MAUI code involved?

Sergio0694 avatar Jun 15 '22 15:06 Sergio0694

Dear Sergio Pedri

Yes, I share the code on GitHub, but I don't know why, it doesn't allow me to upload it automatically, so I did it manually.

Yes, in https://github.com/JavierMarcuzzi/netMauiMvvmProblem https://github.com/JavierMarcuzzi/netMauiMvvmProblem

I did not copy the obj, bin, platform folder.

I reverted the files to a previous version, anyway, it is commented, just remove the "//" and you can try it. The last thing, that is, the last option in shell, where it is pure c#, is not finished, it occurred to me to write without marking and return to the stable version, to test the problem.

If you play a little with the validations, you will see that the button does not allow a second chance. But for this I think you should put everything in community.mvvm beta. For example:
[ICommand] // [RelayCommand] async Task ExecuteSignUpAsync() { await valida();

    } 

Translated with www.DeepL.com/Translator (free version)

El 15 jun. 2022, a las 12:55, Sergio Pedri @.***> escribió:

That's... A lot of code 😅 @JavierMarcuzzi https://github.com/JavierMarcuzzi Can you share a minimal repro, possibly with no MAUI code involved?

— Reply to this email directly, view it on GitHub https://github.com/CommunityToolkit/dotnet/issues/316#issuecomment-1156648703, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHUKHDWVD7ZREIJQVT4SPLVPH4HLANCNFSM5Y3WNZ5A. You are receiving this because you were mentioned.

JavierMarcuzzi avatar Jun 15 '22 21:06 JavierMarcuzzi

Hey @JavierMarcuzzi, that is a full blown project, not a minimal repro. I don't even have the MAUI SDK installed 😅 Can you please share a minimal, self-contained repro, possibly with no MAUI references?

If you need a reference, this is an example of a good minimal repro that can help us investigate: https://github.com/CommunityToolkit/dotnet/issues/283. Essentially, a small, self-contained snippet we can use to reproduce the issue.

Thank you! 🙂

Sergio0694 avatar Jun 16 '22 11:06 Sergio0694

Dear Sergio Pedri

I wouldn't know how to make a minimal example, the solution basically uses mvvm and markup, both from Community Toolkit.

To put it somehow, if the validation is the first time, everything works correctly, but if the validation is incorrect because the word entered in the form does not meet the validation rules, when correcting the word entered the validation would be correct, but the button does not work, and as RelayCommand generates CanExecute, I suspect that the problem is generated by this way, but I do not know how to separate the problem of the form that asks me, at this moment I am writing a new form removing community markup and then I will remove community mvvm, this way I can isolate the part that gives problems. This is what I can do according to my knowledge, I am a veterinarian, not a software engineer, I use computer science for what today they would say data scientist.

Thank you for your answer, not all important people answer the mails.

Regards Javier Marcuzzi

Translated with www.DeepL.com/Translator (free version)

El 16 jun. 2022, a las 08:38, Sergio Pedri @.***> escribió:

Hey @JavierMarcuzzi https://github.com/JavierMarcuzzi, that is a full blown project, not a minimal repro. I don't even have the MAUI SDK installed 😅 Can you please share a minimal, self-contained repro, possibly with no MAUI references?

If you need a reference, this is an example of a good minimal repro that can help us investigate: #283 https://github.com/CommunityToolkit/dotnet/issues/283. Essentially, a small, self-contained snippet we can use to reproduce the issue.

Thank you! 🙂

— Reply to this email directly, view it on GitHub https://github.com/CommunityToolkit/dotnet/issues/316#issuecomment-1157559473, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHUKHBRM3IHGZ7VNSQBJWLVPMG3XANCNFSM5Y3WNZ5A. You are receiving this because you were mentioned.

JavierMarcuzzi avatar Jun 16 '22 14:06 JavierMarcuzzi

To clarify why I'm asking for a minimal repro - the thing is that having to test a full project requires time, and often it's not even an actual problem with the MVVM Toolkit, but just either something caused by another framework (eg. MAUI, etc.), or just the user doing something wrong. Not having a minimal repro makes triaging issues particularly expensive for us, and our time is just limited (also because many of us juggle multiple projects at the same time). Let me know how that experiment goes 🙂

@brminnick since you're more familiar with MAUI, maybe you could help take a quick look? I'm happy to help triage this once we have a more restricted repro I can use with just .NET stuff 👍

"Thank you for your answer, not all important people answer the mails."

I wouldn't say that, we're all just trying to help over here, no worries 😊

Sergio0694 avatar Jun 16 '22 14:06 Sergio0694

https://github.com/wesleyscaldwell/CommToolKitRelayCommandCanExecute

Trial & Step 1:

  • If you click button 1 first you'll see button 2 is disabled but doesn't grey out. Trial & Step 2:
  • Close App and start again : Click button 2 first - Button 2 is disabled and greyed out * Issue: the same binding event results in two different outputs

I have the issue in Maui - which was excluded per the above request. BUT - i do seem to get different results in the maui app. In maui, after clicking Button 1 >>> I am still able to click button two.

wesleyscaldwell avatar Aug 07 '22 13:08 wesleyscaldwell

@wesleyscaldwell Are you able to write a unit test for this? Like, a standalone method, with no UI framework dependencies, that just clearly expresses exactly what logic you expect and what assert is failing according to how you think this should work. That would help a lot 🙂

For reference, something exactly like this:

https://github.com/CommunityToolkit/dotnet/blob/96517eace88d95ec52a881e3e1ed74ea51436b40/tests/CommunityToolkit.Mvvm.UnitTests/Test_RelayCommand.cs#L15-L40

Sergio0694 avatar Aug 07 '22 13:08 Sergio0694

I am able to verify that in the above scenario done in Maui. When clicking button 1 for a second time, the Button two "CanExecute" value is false yet it is clickable.

Hope this helps, i will use the ICommand as a workaround and manually set the ChangeCanExecute method.

wesleyscaldwell avatar Aug 07 '22 14:08 wesleyscaldwell

@wesleyscaldwell Are you able to write a unit test for this? Like, a standalone method, with no UI framework dependencies, that just clearly expresses exactly what logic you expect and what assert is failing according to how you think this should work. That would help a lot 🙂

For reference, something exactly like this:

https://github.com/CommunityToolkit/dotnet/blob/96517eace88d95ec52a881e3e1ed74ea51436b40/tests/CommunityToolkit.Mvvm.UnitTests/Test_RelayCommand.cs#L15-L40

I could look into it, but it is related to testing the UI. And honestly, I haven't written UI unit tests to make this reasonable.

The important thing to note is that the property values are correct. But the UI doesn't reflect the property values.

wesleyscaldwell avatar Aug 07 '22 14:08 wesleyscaldwell

Right, but the thing is, I need to be able to understand whether this issue is on the MVVM Toolkit or on the MAUI side. If it's the former, it means we can write a standalone test like this that reproduces the issue, and then we can fix it. If it's the latter, then this is a bug in MAUI, and this is the wrong repo 😄

Sergio0694 avatar Aug 07 '22 14:08 Sergio0694

I'll see what i can do when i'm not at work. I'm not an expert on this, and i'm sure that is coming across. haha But if i had to guess based on what i've look at so far. Something behind the RelayCommand CanExecute property when changed on a command OTHER than itself the UI isn't notified correctly of the CanExecute property change.

Recap: It DOES work when - RelayCommand2 changes the status of RelayCommad2 It DOES NOT work when - RelayCommand1 changes the status of RelayCommand2

And that is shown in the repo I provided By simply clicking the buttons.

Attribute Example Used: [RelayCommand(CanExecute = "Bar")]

wesleyscaldwell avatar Aug 07 '22 14:08 wesleyscaldwell

Dear, there is a problem with Community Toolkit, using mvvm (code generator) and community toolkit markup.

Create four buttons, in a row, one on top of the other. First button, execute command in mvvm for the first one. Second button, execute a different command for the first one. Everything works. Third button, same command as the first one, but it doesn't work. Fourth button, I modify only the name of the first command, the code is exactly copy and paste. This one works.

In short, according to the order of the buttons, it executes or does not execute the command.

Now I am rewriting my projects in xaml, I think that MAUI lacks development, even more in Mac, surely there must appear thousands of problems that will be solved. If I remember correctly in xaml I also had a similar problem when using the buttons, for some reason the commands run when they want.

Translated with www.DeepL.com/Translator (free version)

JavierMarcuzzi avatar Aug 08 '22 08:08 JavierMarcuzzi

Here is a simply example from WPF. <TextBox Name="TxtInput" Grid.Row="0" /> <Button Grid.Row="1" Content="Click me" Command="{Binding DoSomethingCommand}" CommandParameter="{Binding Text, ElementName=TxtInput}" /> and code from vm [RelayCommand(CanExecute = nameof(CanDoSomething))] private void DoSomething(string te) { } private bool CanDoSomething(string te) => !string.IsNullOrWhiteSpace(te); CanExecute fire once when I start debug. If return true Button can be presed, If false button is enabled and could not be triggered again.

Wep4ik avatar Oct 23 '22 20:10 Wep4ik

Looks like this is still a bug

acrigney avatar Nov 16 '22 07:11 acrigney

This is external and just a MAUI issue (see https://github.com/dotnet/maui/issues/9753), so closing this 🙂

Sergio0694 avatar Nov 16 '22 11:11 Sergio0694

n MAUI I have the RelayCommands working now if I add a parameter to the command which is enabling the CanExecute to be refreshed. i.e I needed to have a commandParameter even though its not actually used in my command. <Button x:Name="btnDelete" Content="Delete" Command="{Binding AddBatchCommand}" CommandParameter="{Binding SelectedProduct}"

acrigney avatar Nov 16 '22 12:11 acrigney

really frustrating. I'm faced with the same problem, I'm already in an evaluating phase of mvvm toolkit and discover bugs like this requires to spent a lot of time to understand if I'm doing something wrong or if it's simply another bug of Maui or of the library.

After declaring a fake parameter now I can see my command to be evaluated when I change a property value with the NotifyCanExecuteChangedFor decoration, but I can't see the command attached to the xaml to be executed. All is working if I attach the command to a GestureRecognizer

fgiacomelli avatar Nov 16 '22 16:11 fgiacomelli

I can appreciate how it's frustrating, but this is simply unrelated from the MVVM Toolkit 🙂 I'd recommend following https://github.com/dotnet/maui/issues/9753 to be updated on progress there.

Sergio0694 avatar Nov 16 '22 17:11 Sergio0694

I don't think it's a MAUI issue. I've run into the same thing with WPF/Windows Desktop. I know, I know, provide a minimal project to repro the issue. :) I'll see what I can do.

mabrown avatar Apr 03 '23 23:04 mabrown

Here's a very minimal .NET 6 WFP Windows Desktop project that demonstrates the issue. Two buttons on a window. The first is always enabled. The second starts disabled but should enable after clicking button 1 the first time, then disable after clicking button 1 the second time and so on. Button 2, if enabled, should display a message box when clicked. However, it never enables. WpfApp1.zip

mabrown avatar Apr 04 '23 00:04 mabrown

I meet the same issue

var canEditRemove = SelectedProfile != null;
EditServerCmd = new RelayCommand(() => EditServer(null), () => canEditRemove);
public ProfileItem SelectedProfile
{
    get => _selectedProfile;
    set
    {
        if (Equals(value, _selectedProfile)) return;
        _selectedProfile = value;
        OnPropertyChanged();
    }
}

On application first run, SelectedProfile is null, so EditServerCmd can not be executed, which is grey out on UI.

However, when SelectedProfile has value, EditServerCmd is still grey out, because it's CanExecute status is not updated to true

EdiWang avatar Apr 18 '23 05:04 EdiWang

Any update on this. Its definitely not a MAUI issue as I am seeing this on Windows using Avalonia.

ajmcateer avatar Jul 19 '23 20:07 ajmcateer

@ajmcateer are you able to share a minimal repro, even better a standalone unit test we could use? 🙂

Sergio0694 avatar Jul 19 '23 21:07 Sergio0694