Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RaiseCanExecuteChanged on MvxCommand status is not checked again #2064

Closed
Sebastian1989101 opened this issue Jul 24, 2017 · 17 comments
Closed
Milestone

Comments

@Sebastian1989101
Copy link

I have a simple string.IsNullOrEmpty check for my Property to evaluate if the command can execute or not. But even if I call the RaiseCanExecuteChanged method, the CanExecute method is not called again and the button stays disabled. I'm using Xamarin.Forms with MvvmCross 5.1.

@mvanbeusekom
Copy link
Contributor

Have a look at issue #1877, I think you have a similar misunderstanding.

First of all the RaiseCanExecuteChanged event / method is called when the CanExecute delegate is changed. So it is not meant to trigger the CanExecute method but to inform you that the implementation changed.

Probably you want to call the CanExecute method directly (or use it in a binding to set the Enabled property). If you provide a bit more details on where you are trying to use it (code) I might be able to help you more.

@Sebastian1989101
Copy link
Author

But this makes no sense. Why should a void method without any parameter and with a "Raise" as prefix not invoke that there might be a change?! Also there is the CanExecuteChanged event too for event registration. The RaiseCanExecuteChanged method should invoke a "PropertyChanged" like event (here called CanExecuteChanged) so that the UI is requesting the CanExecute again but that is not happening. Otherwise this would be completely different from WPF and Xamarin.Forms. Also the Naming and Parameter of the Raise-Methode would make no sense at all. The event for Invoke a property changed event in the view model is also called RaisePropertyChanged and works as expected.

@Sebastian1989101
Copy link
Author

About my code:

I have this for the command property definition:
public MvxCommand AcceptCommand { get; }

This in the constructor for the init:
AcceptCommand = new MvxCommand(OnAcceptCommandExecute, OnAcceptCommandCanExecute);

And here are the two methods implemented:

private void OnAcceptCommandExecute()
{
    if (!string.IsNullOrEmpty(Feed.Title) && !string.IsNullOrEmpty(Feed.Url))
    {
        rssFeedDatabaseRepository.Insert(Feed);
        MessagingCenter.Send(Application.Current, "RssFeedsChanged");
    }

    Close(this);
}

private bool OnAcceptCommandCanExecute()
{
    return !string.IsNullOrEmpty(Feed.Title) && !string.IsNullOrEmpty(Feed.Url);
}

And I now want to invoke that the CanExecute might change if the Feed is changing:

Feed.Url = value;
AcceptCommand.RaiseCanExecuteChanged();

@mvanbeusekom
Copy link
Contributor

My initial explanation indeed makes no sense and is incorrect. The concept should indeed be that you call the RaiseCanExecuteChanged method to inform the binding system to re-evaluated the condition defined by the CanExecute implementation (in your case because the Feed.Url property was modified).

Your code looks correct and I will have to look into why the CanExecute method is not re-evaluated.

@mvanbeusekom
Copy link
Contributor

mvanbeusekom commented Jul 24, 2017

@Sebastian1989101 while reading up on this issue I found the following explanation in the MvvmCross documentation (https://www.mvvmcross.com/documentation/fundamentals/data-binding):

Note: One important feature sometimes used in Windows Xaml binding but poorly supported by MvvmCross is the CanExecute/CanExecuteChanged functionality on ICommand. In Xaml binding this property and event pair can be used to enable/disable UI controls such as buttons.

However, in MvvmCross, this auto-enable/disable binding isn’t currently widely supported - with support instead being given to secondary binding properties - e.g. to pairs of bindings like:

Click GoHome; IsEnabled CanGoHome

In other words CanExecute is not supported, use a specific property to enable/disable you button. If you have time and feel like adding support for CanExecute we would be happy to accept a pull-request.

@Sebastian1989101
Copy link
Author

Sebastian1989101 commented Jul 24, 2017

Thanks for the reply. I will change this in my code but maybe it should marked as Obsolete or at least threw a NotSupportedException while it is not working / implemented. So nobody get confused at this again.

Is there any full overview about what is currently not working or not finished for Xamarin.Forms? I find it hard to find a full documentation about MvvmCross with Xamarin.Forms - I'm searching since 2 hours now how Modal dialogs work with Forms for example.. I used MvvmCross a few times with native implementations for iOS only projects and had never so many problems as with this simple project now..

@martijn00 martijn00 added this to the 5.1.0 milestone Oct 31, 2017
@Sebastian1989101
Copy link
Author

@martijn00 This was added to the milestone for 5.1.0. Today I needed such a function again and tested it with 6.0.0 beta7 and again, RaiseCanExecuteChange does not trigger a reevaluation of the can execute. Even worse, the can execute is never reevaluated so that if the relevant property changes, nothing will happen.

@nickrandolph
Copy link
Contributor

Any chance you can reproduce this in playground and submit a PR showing the issue?

@Sebastian1989101
Copy link
Author

@nickrandolph I'm currently trying to get the Playground compile and execute (also for the other open issue). As soon as this works I can create Repos with the issue(s).

@nickrandolph
Copy link
Contributor

Are you trying to run it on Mac or PC? The latter is easier as it works in VS. Make sure you do a clean all across solution and then force rebuild individually on mvvmcross, Android appcompat, and the mvvmcross.forms. Then you should be good to run the samples

@Sebastian1989101
Copy link
Author

@nickrandolph Just deleted everything and checked it out completely fresh. Now I'm trying to restore NuGet packages but it looks like it is failing all over the place. I'm even not able to compile the MvvmCross core project. It always ends with Error: NuGet packages need to be restored before building. NuGet MSBuild targets are missing and are needed for building. The NuGet MSBuild targets are generated when the NuGet packages are restored. (MvvmCross).

Actual I'm trying with VS for Mac because I prefer to work with my Mac.

@nickrandolph
Copy link
Contributor

I think the multi-targeting isn’t working well on vs for Mac. If you have VS (PC) handy I would try that as I think you’ll be able to get it running much easier

@Sebastian1989101
Copy link
Author

@sieroslaw
Copy link

I've this problem too. CanExecutedChanged and CanExecute (MvxCommand) is not always called after invoke RaiseCanExecuteChanged (checked on MVVMCross 6.0.0.beta7 and 6.0.1)

@mgochmuradov
Copy link
Contributor

mgochmuradov commented Jun 14, 2018

On WPF I had a helper class that solved the issue

    public class MvxWpfCommandHelper : IMvxCommandHelper
    {
        public event EventHandler CanExecuteChanged
        {
            add => CommandManager.RequerySuggested += value;
            remove => CommandManager.RequerySuggested -= value;
        }

        public void RaiseCanExecuteChanged(object sender)
        {
            CommandManager.InvalidateRequerySuggested();
        }
    }

Where CommandManager is System.Windows.Input.CommandManager, which is not available on other platforms.
Could there be any solution like that for Android (Forms)?

@MartinZikmund
Copy link
Contributor

MartinZikmund commented Jun 20, 2018

Same situation for me, RaiseExecuteChanged does not re-evaluate CanExecute on MvvmCross 6.1 on Forms Android for me. I had to use to classic Xamarin.Forms Command

@ivanmeling
Copy link

ivanmeling commented Aug 14, 2020

If you're using MvxAsyncCommand you can initialize it with the allowConcurrentExecutions constructor parameter set to true. That way the CanExecute is always reevaluated. This is more of a hack than a real solution, as the concurrent code in MvxAsyncCommand looks really shady.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants