Mvvmcross: After navigating with back button Command can't be called

Created on 10 Aug 2018  路  22Comments  路  Source: MvvmCross/MvvmCross

馃悰 Bug Report

I'm having IMvxAsyncCommand that calls Navigation to view with result, like
var result = await NavigationService.Navigate(parameter);
And when I'm navigating back from SecondViewModel with hardware button it is no longer possible to call this command.

Expected behavior

Reproduction steps

Configuration

Version: 6.x

Platform:

  • [ ] :iphone: iOS
  • [ ] :robot: Android
  • [ ] :checkered_flag: WPF
  • [ ] :earth_americas: UWP
  • [ ] :apple: MacOS
  • [ ] :tv: tvOS
  • [x] :monkey: Xamarin.Forms
needs-investigation up-for-grabs

Most helpful comment

@Cheesebaron @martijn00 is there an issue with how we complete async navigation methods when the user presses the back button v closing the view model?

Just FYI, I've encountered this issue when closing the view model as well (albeit less frequently), so I don't think it's limited to the back button.

All 22 comments

BTW, if I'm declaring my command as allowConcurrentExecutions = true all is working fine.

I noticed this as well, easy to reproduce.

As a workaround you can add this line to your SecondViewModel's OnDissapearing:

ViewModel.CloseCompletionSource?.TrySetResult(false);

Same case in my application. When we can expect a fix for this issue?

@piotrekbigu you can init your command with allowConcurrentExecutions = true. This is kind of work around.

@xyashapyx
Yeah I know it but I do not want to give my commands a chance to execute concurrently :)

We've been struggling with this issue for the last 6 months. We ended up removing the async from navigation all together and we also have to register the commands on every page load for some reason. If you set a command in the constructor to navigate, then navigate, then go back the command is no longer wired up. To get around this we call a private method called SetNavigationCommands in the ViewAppearing method to get it to work correctly. GIANT pain and ugly code because of this issue. We're not passing parameters like they are in the example. We were just trying to navigate normally using MvxAsyncCommand(() => _navigationService.Navigate());

Does not work:
public IMvxAsyncCommand SettingsButtonClickCommand => new MvxAsyncCommand(() => _navigationService.Navigate<SettingsTabbedViewModel>());

Works:

public IMvxCommand SettingsButtonClickCommand { get; set; }

private void SetNavigationCommands()
{
    SettingsButtonClickCommand = new MvxCommand(() => _navigationService.Navigate<SettingsTabbedViewModel>());
}

public override void ViewAppearing()
{
    SetNavigationCommands();
}

I believe this is related to the poor support for CanExecute/CanExecuteChanged functionality on ICommand (as called out in the Data binding documentation for MvvmCross). To work around it, I added a property to my ViewModel called CanNavigate and bound the IsEnabled property of my button to it.

public bool CanNavigate { get; } = true;

public IMvxCommand NavigateAsync => new MvxAsyncCommand(async () =>
{
    await navigationService.Navigate<NextPageViewModel>();

    await RaisePropertyChanged(() => CanNavigate);
});

And in my XAML:
<Button mvx:Bi.nd="Command NavigateAsync; IsEnabled CanNavigate" Text="Navigate"/>

By virtue of it being a work around it's obviously not ideal, but in all of my testing it has proved successful.

@Cheesebaron @martijn00 is there an issue with how we complete async navigation methods when the user presses the back button v closing the view model?

@Cheesebaron @martijn00 is there an issue with how we complete async navigation methods when the user presses the back button v closing the view model?

Just FYI, I've encountered this issue when closing the view model as well (albeit less frequently), so I don't think it's limited to the back button.

I confirm that the issue exists as of 6.4.0 version.
In fact, it is actual for a long time.
I found this issue (closed by some reason), which looks quite similar (2.5 years(!) ago).

And the issue is quite serious, because there always no guarantee the IMvxAsyncCommand will definitely work, so it just eliminates a sense of using of it at all (even though the idea of using it for controls disabling until the processing (async) method finished is pretty cool).

Also, to be more clear, I stuck with it quite often lately. And all the projects I had the issue, were Xamarin.Forms, and the issue seemed to appear on Android only.

Yes this issue is here for a while and it's not just for navigation it happens for other async calls too.
And yes so far what I've seen over the years it's only android issue.

Well if the task that runs inside a MvxAsyncCommand doesn't finish and you haven't allowed multiple executions, then CanExecute will return false until the task has finished or you have cancelled the command by calling Cancel() on it.

@Cheesebaron We are talking about tasks that finished.
It is reproducible on my phone xiaomi MI A1, android 9. On the emulator this example is working fine.

Create simple app that has two tab pages: Page1 and Page2. Place on Page1 Button with MvxAsyncCommand(SomeTask)

private async Task SomeTask() { await Task.Delay(5000); }

On the android click on the button and it become disabled. Move to second tab Page2. Leave the task to finish and then move back to Page1.

Button remains disabled even though the task has finished. If you test some other bindings like change the text of the button after await Task.Delay(3000) while you are on the tab Page2 these are working.

On iOS no issues with this, but the button is not disabled at all. It is still clickable just the command task is not invoked.

here is the recording:
recoridng.zip

It seems this issue still exists and is not limited to just Android as others have suggested. I am experiencing this same issue on an Xamarin.Forms app with MvvmCross 6.4.2 running on iOS 12. However, for some reason, iOS 13 doesn't have that same issue.

I had the same issue, and as per the documentation https://www.mvvmcross.com/documentation/fundamentals/navigation
in the override for ViewDestroy, I set the CloseCompletionSource state, which then correctly cancelled the navigation task, thus making my control active again:

public override void ViewDestroy(bool viewFinishing = true)
{

            if (CloseCompletionSource != null && !CloseCompletionSource.Task.IsCompleted && !CloseCompletionSource.Task.IsFaulted)
                CloseCompletionSource?.TrySetCanceled();
            base.ViewDestroy(viewFinishing);
}

Any news on this issue ? Still happens in 6.4.2 on android.
There is plenty work around to make things works (like with cancellation token), but did they intent to fix the issue at it's core ?

Still happens with mvvmcross 7.0.0 on Android. Any news concerning this issue ?

I had the same issue, and as per the documentation https://www.mvvmcross.com/documentation/fundamentals/navigation
in the override for ViewDestroy, I set the CloseCompletionSource state, which then correctly cancelled the navigation task, thus making my control active again:

public override void ViewDestroy(bool viewFinishing = true)
{

            if (CloseCompletionSource != null && !CloseCompletionSource.Task.IsCompleted && !CloseCompletionSource.Task.IsFaulted)
                CloseCompletionSource?.TrySetCanceled();
            base.ViewDestroy(viewFinishing);
}

This work around is actually quite simple and works well.

I had the same issue, and as per the documentation https://www.mvvmcross.com/documentation/fundamentals/navigation
in the override for ViewDestroy, I set the CloseCompletionSource state, which then correctly cancelled the navigation task, thus making my control active again:

public override void ViewDestroy(bool viewFinishing = true)
{

            if (CloseCompletionSource != null && !CloseCompletionSource.Task.IsCompleted && !CloseCompletionSource.Task.IsFaulted)
                CloseCompletionSource?.TrySetCanceled();
            base.ViewDestroy(viewFinishing);
}

This work around is actually quite simple and works well.

@clem13480 How does it work? CloseCompletionSource is always null.

I had the same issue, and as per the documentation https://www.mvvmcross.com/documentation/fundamentals/navigation
in the override for ViewDestroy, I set the CloseCompletionSource state, which then correctly cancelled the navigation task, thus making my control active again:

public override void ViewDestroy(bool viewFinishing = true)
{

            if (CloseCompletionSource != null && !CloseCompletionSource.Task.IsCompleted && !CloseCompletionSource.Task.IsFaulted)
                CloseCompletionSource?.TrySetCanceled();
            base.ViewDestroy(viewFinishing);
}

This work around is actually quite simple and works well.

@clem13480 How does it work? CloseCompletionSource is always null.

You have to be inheriting IMvxViewModel.
Here is my full base class. And the CloseCompletionSource is not null.

public abstract class BaseViewModel : BaseViewModel, IMvxViewModel, IBaseViewModel
{
public TaskCompletionSource CloseCompletionSource { get; set; }
public BaseViewModel(IMvxLogProvider log, IMvxNavigationService navigationService) : base(log, navigationService) { }

    public override void ViewDestroy(bool viewFinishing = true)
    {
        if (CloseCompletionSource != null && !CloseCompletionSource.Task.IsCompleted && !CloseCompletionSource.Task.IsFaulted)
            CloseCompletionSource?.TrySetCanceled();

        base.ViewDestroy(viewFinishing);
    }
}

@clem13480 I can reproduce the issue, even with CloseCompletionSource implemented. Have you implemented any cancellation logic inside your MvxAsyncCommand?

@LITTOMA CloseCompletionSource will not be null if you navigate to it with the return object overloading.

I also noticed that this effect only occurs once for one command. After one command is disabled, the others work fine.

If I set allowConcurrentExecutions in MvxAsyncCommand to true, then it works fine. But this is not the solution. It will just not disable it in the first place.

The ViewModel navigation Task finishes properly. So maybe the MvxAsyncCommand noticing it is not implemented properly.

Edit:
I believe I found the cause.
The MvxAsyncCommand implementations are all fine.
But in the Method ExecuteConcurrentAsync the second RaiseCanExecuteChanged() in the finally block will not be recognized by the UI. Thats why the Button will be still disabled.

This behavior is caused in the File MvxCommand.cs
In the Method SafeCopyEventHandlerList the EventHandler for recognizing the reactivated CanExecute is !thing.IsAlive (not alive). This will mark the EventHandler as dead and it will not be called.

Even if I call MvxAsyncCommand.RaiseCanExecuteChanged(); in the ViewAppeared() will not help, because the connection between the Command and the UI CommandManager is lost.

Edit2:
The Problem with the CanExecute not notifying the UI, because the WeakReference is lost is even occurring with MvxCommand if you are clicking some Commands successively.
For Example if I have two Buttons A and B. And if I clicking A, B, A, B, A, B, then the CanExecute of B will not work anymore. And here is no navigation involved, it is all on the same view/page.

I had the same issue, and as per the documentation https://www.mvvmcross.com/documentation/fundamentals/navigation
in the override for ViewDestroy, I set the CloseCompletionSource state, which then correctly cancelled the navigation task, thus making my control active again:

public override void ViewDestroy(bool viewFinishing = true)
{

            if (CloseCompletionSource != null && !CloseCompletionSource.Task.IsCompleted && !CloseCompletionSource.Task.IsFaulted)
                CloseCompletionSource?.TrySetCanceled();
            base.ViewDestroy(viewFinishing);
}

@purusartha 's solution is a good workaround, but it has a catch. If you are only navigating from A->B, then you are golden. Hitting "back" on B causes the navigation call to return a null result which you can manage accordingly in your code. After that, the IMvxAsyncCommand works as expected.

BUT. If you do A->B and then B->C, when the B->C navigation occurs, the ViewDestroy method is invoked, thus causing the await for navigation from A->B to return a null result, even though you should techincally be still waiting for the result of A->B navigation. I had that in my code, rendering the suggested workaround unusable.

Was this page helpful?
0 / 5 - 0 ratings