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

Pop unsubscribing #26

Open
petarkekez opened this issue Mar 1, 2021 · 2 comments
Open

Pop unsubscribing #26

petarkekez opened this issue Mar 1, 2021 · 2 comments

Comments

@petarkekez
Copy link

petarkekez commented Mar 1, 2021

Hi, to start thanks for the framework.

I had an issue with stuff not unsubscribing with Shell.

This happened because the unsubscription automaticly happend only on the navigation page, code:

if (sender is Application && e.PropertyName == "MainPage")
{
    var page = (sender as Application).MainPage;

    if (page is NavigationPage)
    {
        BindEvents(page as NavigationPage);
    }
}

because of this i needed to update this method to this:

static void App_PropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
{
    if (sender is Application && e.PropertyName == "MainPage")
    {
        var page = (sender as Application).MainPage;

        if (page is NavigationPage)
        {
            BindEvents(page as NavigationPage);
        }
        else if (page is Shell s)
        {
            s.Navigating += Shell_Navigating;
        }
    }
}
private static void Shell_Navigating(object sender, ShellNavigatingEventArgs e)
{
    if (e.Source == ShellNavigationSource.Pop
        || e.Source == ShellNavigationSource.Remove)
    {
        if (sender is Shell s)
        {
            UnsubscribePage(s.CurrentPage);
        }
    }

    if (e.Source == ShellNavigationSource.PopToRoot)
    {
        if (sender is Shell s)
        {

            UnsubscribePage(s.CurrentPage);
        }
    }

    if (e.Source == ShellNavigationSource.Unknown)
    {
        if(sender is Shell s)
        {
            if( !(s.CurrentPage is MainView) ) // On initial shell setup this is triggered..
            {
                UnsubscribePage(s.CurrentPage);
            }
        }
    }
}

I can add this to a PR if you would like to review/merge it?

@johankson
Copy link
Collaborator

Hi! Thanks for using TinyPubSub!

Add it to a PR and we'll take a look at it and merge it if there are no issues with it. We do need to do some testing to make sure it doesn't break anything, but we'll try to be as quick as possible.

And reach out if there are any other questions.

@johankson
Copy link
Collaborator

Is it OK if I add these changes myself?

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

2 participants