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

ViewModel closing when navigate to new page #1238

Closed
okunevdm opened this issue Nov 27, 2018 · 59 comments
Closed

ViewModel closing when navigate to new page #1238

okunevdm opened this issue Nov 27, 2018 · 59 comments
Assignees
Milestone

Comments

@okunevdm
Copy link

Catel.MVVM.Xamarin.Forms
After navigate to new page previous ViewModel is closing. And when press back button and return to first page viewmodel for this page create new instance.
is catel:ContentPage contains property like CloseViewModelOnUnloaded?

Expected behaviour

when NavigateForward ViewModel continues to work on background.
when NavigateBack ViewModel closing.

Actual behaviour

ViewModel closing when NavigateForward

@GeertvanHorrik GeertvanHorrik self-assigned this Nov 28, 2018
@GeertvanHorrik
Copy link
Member

@alexfdezsauco care to join the conversation? You wrote this part.

@stale
Copy link

stale bot commented Jan 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 28, 2019
@stale stale bot closed this as completed Feb 4, 2019
@GreenKn1ght
Copy link
Contributor

GreenKn1ght commented Feb 6, 2019

This is a critical bug in Xamarin.Android due to the fact that Page.OnAppearing is called every time the page appears on the phone screen: this may be caused by navigation or by the OS when the app/phone comes out of sleep.
CloseViewModelOnUnloaded can't help because then you will have to handle the ViewModel lifecycle on your own.

@GeertvanHorrik
Copy link
Member

We definitely want to fix this, but might need a bit of help here, interested?

@GeertvanHorrik GeertvanHorrik added this to the 5.10.0 milestone Feb 21, 2019
@okunevdm
Copy link
Author

I fix this bug in my project by create my own ContentPage with needed behaviour of _userControlLogic

@GreenKn1ght
Copy link
Contributor

@okunevdm did you use the UserControlLogic.CloseViewModelOnUnloaded property or came up with another solution?

@GreenKn1ght
Copy link
Contributor

I solved this by overriding the OnParentSet method and firing Loaded and Unloaded events depending on whether the Parent is null.
But before creating PR I need to test it at least on UWP and iOS.

@GreenKn1ght
Copy link
Contributor

By the way, we still want to know when the view appears or disappears in associated view model. Any thoughts? Can we extend ViewModelBase with OnAppearing and OnDisappearing methods?

@GeertvanHorrik
Copy link
Member

Why would you want Appearing / Disappearing methods, aren't they equal to Loaded/ Unloaded? In that case, you can (should) implement this in the iView implementation so Catel can deal with this.

@GreenKn1ght
Copy link
Contributor

I solved this by overriding the OnParentSet method and firing Loaded and Unloaded events depending on whether the Parent is null.
But before creating PR I need to test it at least on UWP and iOS.

With this they won't longer be equal. Unloaded will be fired when our page leaves the navigation stack.

@GreenKn1ght
Copy link
Contributor

I'll prepare sample to show this behavior.

@okunevdm
Copy link
Author

@okunevdm did you use the UserControlLogic.CloseViewModelOnUnloaded property or came up with another solution?

this is my ContentPage class
` public class ContentPage : global::Xamarin.Forms.ContentPage, IView
{
private readonly UserControlLogic _userControlLogic;
private object _oldbindingContext;

    public ContentPage()
        : this(null)
    {
        BindingContextChanged += OnBindingContextChanged;
    }

    private void OnBindingContextChanged(object o, EventArgs eventArgs)
    {
        DataContextChanged?.Invoke(this, new DataContextChangedEventArgs(_oldbindingContext, BindingContext));
        _oldbindingContext = BindingContext;
        RaiseViewModelChanged();
    }

    public ContentPage(IViewModel viewModel)
    {
        _userControlLogic = new UserControlLogic(this, null, viewModel);
        _userControlLogic.TargetViewPropertyChanged += (sender, e) =>
        {
            OnPropertyChanged(e.PropertyName);
        };

        _userControlLogic.ViewModelClosedAsync += OnViewModelClosedAsync;

        _userControlLogic.ViewModelChanged += (sender, args) =>
        {
            if (!ObjectHelper.AreEqual(BindingContext, _userControlLogic.ViewModel))
            {
                BindingContext = _userControlLogic.ViewModel;
            }
        };

        _userControlLogic.ViewModelPropertyChanged += (sender, e) =>
        {
            OnViewModelPropertyChanged(e);
            ViewModelPropertyChanged?.Invoke(this, e);
        };

        Loaded += (sender, e) =>
        {
        };

        Unloaded += (sender, e) =>
        {
        };

        DataContextChanged += OnDataContextChanged;

        this.CustomBackButtonAction = OnBackButtonActionAsync;
    }

    public IViewModel ViewModel => DataContext as IViewModel;

    public event EventHandler<EventArgs> ViewModelChanged;

    public event EventHandler<PropertyChangedEventArgs> ViewModelPropertyChanged;

    public object DataContext
    {
        get => BindingContext;
        set => BindingContext = value;
    }

    public object Tag { get; set; }

    public event EventHandler<EventArgs> Loaded;

    public event EventHandler<EventArgs> Unloaded;

    public event EventHandler<DataContextChangedEventArgs> DataContextChanged;


    private void OnViewModelPropertyChanged(PropertyChangedEventArgs e)
    {
    }

    private void RaiseViewModelChanged()
    {
        OnViewModelChanged();
        ViewModelChanged?.Invoke(this, EventArgs.Empty);
    }

    protected virtual void OnViewModelChanged()
    {
    }

    protected virtual Task OnViewModelClosedAsync(object sender, ViewModelClosedEventArgs e)
    {
        return TaskHelper.Completed;
    }

    public event EventHandler<EventArgs> BackButtonPressed;

    protected override void OnAppearing()
    {
        Loaded?.Invoke(this, EventArgs.Empty);

        base.OnAppearing();
    }

    protected override void OnDisappearing()
    {
        base.OnDisappearing();

        Unloaded?.Invoke(this, EventArgs.Empty);
    }

    protected sealed override bool OnBackButtonPressed()
    {
        /*
        BackButtonPressed.SafeInvoke(this);
        var popupLayout = Content as PopupLayout;
        //// TODO: Lookup for top most popup layout.
        return popupLayout != null && popupLayout.IsPopupActive || base.OnBackButtonPressed();
        */

        BackButtonPressed?.Invoke(this, EventArgs.Empty);

        return base.OnBackButtonPressed();
    }

    private void OnDataContextChanged(object sender, DataContextChangedEventArgs eventArgs)
    {
        ViewModelChanged?.Invoke(this, EventArgs.Empty);
    }

    public Action CustomBackButtonAction { get; set; }
    protected async void OnBackButtonActionAsync()
    {
        if (await _userControlLogic.CancelAndCloseViewModelAsync())
        {
            await Navigation.PopAsync(true);
        }
    }
}

`

@okunevdm
Copy link
Author

okunevdm commented Feb 21, 2019

for hardware back button in android i override in MainActivity
public override void OnBackPressed() { // retrieve the current xamarin forms page instance var currentPage = Xamarin.Forms.Application.Current.MainPage.Navigation.NavigationStack.LastOrDefault(); // check if the page has subscribed to the custom back button event if (currentPage != null && currentPage.GetType().IsSubclassOf(typeof(DmMobile.Controls.ContentPage)) && ((DmMobile.Controls.ContentPage)currentPage).CustomBackButtonAction != null) { // invoke the Custom back button action ((DmMobile.Controls.ContentPage)currentPage).CustomBackButtonAction.Invoke(); // and disable the default back button action } else { base.OnBackPressed(); } }

@GreenKn1ght
Copy link
Contributor

Probably I'm blind, but there is only one difference: CustomBackButtonAction. I do not see how this changes the current behavior.

@GreenKn1ght
Copy link
Contributor

Please take a look at my sample.

@okunevdm
Copy link
Author

Probably I'm blind, but there is only one difference: CustomBackButtonAction. I do not see how this changes the current behavior.

CustomBackButtonAction uses in concrete instance of contentPage:
`
[XamlCompilation(XamlCompilationOptions.Compile)]
public partial class SettingsView
{
public SettingsView()
{
InitializeComponent ();

        CustomBackButtonAction = async () =>
        {
            var result = await this.DisplayAlert(Properties.Resources.Warning, Properties.Resources.ConfirmWithoutSaving, Properties.Resources.Yes, Properties.Resources.No);

            if (result)
            {
                OnBackButtonActionAsync();
            }
        };
    }
}

`

@okunevdm
Copy link
Author

okunevdm commented Feb 22, 2019

now i can not use ViewModel for control of Navigation behavior because of issue https://github.com/Catel/Catel/issues/1252 - i can not show message with buttons yes/no, but DisplayAlert works fine.

@GreenKn1ght
Copy link
Contributor

GreenKn1ght commented Feb 22, 2019

Thus you prevent navigation. We should make it work out of the box, but initial issue is "After navigate to new page previous ViewModel is closing". How did you fix it?

@okunevdm
Copy link
Author

default CustomBackButtonAction is protected async void OnBackButtonActionAsync()
in constructor: this.CustomBackButtonAction = OnBackButtonActionAsync;
when back button pressed (hardware or software) it call CustomBackButtonAction. and then:

    protected async void OnBackButtonActionAsync()
    {
        if (await _userControlLogic.CancelAndCloseViewModelAsync())
        {
            await Navigation.PopAsync(true);
        }
    }

@GreenKn1ght
Copy link
Contributor

But if you navigate forward, your view model will still be closed and then recreated.

@okunevdm
Copy link
Author

oh, yes of course.. )) I forgot that I really used CloseViewModelOnUnloaded and when copy paste code of ContentPage i cut it... sorry.
I add this property to ContentPage and set it to false:

    public class ContentPage : global::Xamarin.Forms.ContentPage, IView
    {
     ...
        public ContentPage(IViewModel viewModel)
        {
            ....
            _userControlLogic = new UserControlLogic(this, null, viewModel);
            CloseViewModelOnUnloaded = false;
           ....
        }
        public bool CloseViewModelOnUnloaded
        {
            get { return (bool)GetValue(CloseViewModelOnUnloadedProperty); }
            set { SetValue(CloseViewModelOnUnloadedProperty, value); _userControlLogic.CloseViewModelOnUnloaded = CloseViewModelOnUnloaded; }
        }
        public static readonly BindableProperty CloseViewModelOnUnloadedProperty = BindableProperty.Create(nameof(CloseViewModelOnUnloaded), typeof(bool), typeof(ContentPage), false);
    }

@GreenKn1ght
Copy link
Contributor

Now it makes sense!
But you should be wary of navigating backwards using the NavigationPage and NavigationService.

@okunevdm
Copy link
Author

okunevdm commented Feb 22, 2019

why? i use NavigationPage/NavigationService and it work fine.

@GeertvanHorrik
Copy link
Member

Maybe we can do type forwarding in https://docs.microsoft.com/en-us/dotnet/framework/app-domains/type-forwarding-in-the-common-language-runtime, but I don't want to lose Windows Phone (windows 10 mobile) support on UAP (use this myself) so updating to the min version of UAP that implements .net standard is not an option.

@GeertvanHorrik
Copy link
Member

Now I think of it, this might be caused because Catel.Core doesn't explicitly use the .net standard 2.0 implementation, but the platform-specific one (that is also included there). Maybe we can force it to use the .net standard one instead.

@GreenKn1ght
Copy link
Contributor

Can we support two versions of UAP? Since the Xamarin.Forms common library implements .NETStandard, we can't use the old version of UAP here anyway.

@GreenKn1ght
Copy link
Contributor

AFAIK, XF 3+ doesn't support WM10. Thus, we can only support it as a non-XF library

@GeertvanHorrik
Copy link
Member

The issue here is Catel.Core, that supports any platform (and the UWP version picks the UAP, not the .net standard implementation).

@GreenKn1ght
Copy link
Contributor

I understood that. I propose to target Catel.Core to uap10.0 (will use local types) and uap10.0.16299 (will use .NETStandard2.0 types). That should solve the problem, isn't it?

@GeertvanHorrik
Copy link
Member

Might work, let me try that.

@GeertvanHorrik
Copy link
Member

image

@GeertvanHorrik
Copy link
Member

Just pushed the first bits, need to leave now, but will try to build it on the build server.

@GeertvanHorrik
Copy link
Member

Build - 5.10.0-beta.15

image

@GreenKn1ght
Copy link
Contributor

It works!

@GeertvanHorrik
Copy link
Member

Awesome, thanks for pointing me in the right direction, it was a brilliant idea. You got sufficient stuff working to complete this ticket?

@GreenKn1ght
Copy link
Contributor

At first glance, yes. I still need to set up the environment for iOS. I will try to finish ticket at this weekend.

@GeertvanHorrik
Copy link
Member

Do you mind if we put this ticket for 5.11? Then we can close the beta and release 5.10 early next week.

@GreenKn1ght
Copy link
Contributor

Yes, let's put it for 5.11.

@GeertvanHorrik
Copy link
Member

@GreenKn1ght any updates?

@GeertvanHorrik GeertvanHorrik modified the milestones: 5.11.0, 5.12.0 Jul 17, 2019
@GeertvanHorrik GeertvanHorrik modified the milestones: 5.12.0, 5.13.0 Oct 9, 2019
@GeertvanHorrik GeertvanHorrik modified the milestones: 5.13.0, 6.0.0 Nov 25, 2019
@GeertvanHorrik GeertvanHorrik modified the milestones: 6.0.0, Up for grabs Sep 25, 2020
@GeertvanHorrik
Copy link
Member

Since we removed support for all frameworks exception .NET 6 (WPF) for 6.0, I am closing this issue.

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

No branches or pull requests

3 participants