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

[Android] Unloaded event is incorrectly raised during navigation for a view in a custom layout #16697

Closed
marchev-prgs opened this issue Aug 11, 2023 · 14 comments
Assignees
Labels
area-core-lifecycle XPlat and Native UIApplicationDelegate/Activity/Window lifecycle events p/1 Work that is critical for the release, but we could probably ship without partner Issue or Request from a partner team platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@marchev-prgs
Copy link

marchev-prgs commented Aug 11, 2023

Description

I have a custom Layout. During the MeasureOverride() of the custom layout I create some inner element and add it to Children. Later, when navigation to another page happens - the Unloaded event of the inner element is raised (the Unloaded of the custom layout is not). When I navigate back - the Loaded event is not raised. This leaves custom logic in Loaded/Unloaded event handlers in a inconsistent state.

Steps to Reproduce

  1. Download the repo from the link (https://github.com/telerik/ms-samples/tree/main)
  2. Go to folder Maui\UnloadedRaisedInScenarioWithCustomLayout
  3. Open the solution
  4. Run the App in Android
  5. See in the collection view log that Loaded was raised for both the custom layout and the innerGrid
  6. Click the "navigate" button (it navigates to an empty page)
  7. Navigate back to the previous page
  8. See that Unloaded event was raised for innerGrid

Actual: See the log history in the collection view - the innerGrid received Unloaded when we navigated to the empty page, but Loaded was never raised after that.

Expected: The Unloaded for innerGrid should have not been raised, as the Unloaded for the custom layout was not raised.

unloaded

Link to public reproduction project repository

https://github.com/telerik/ms-samples/tree/main

Version with bug

8.0.0-preview.6.8686

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

N/A

Relevant log output

No response

@marchev-prgs marchev-prgs added the t/bug Something isn't working label Aug 11, 2023
@PureWeen PureWeen added the partner Issue or Request from a partner team label Aug 11, 2023
@PureWeen PureWeen self-assigned this Aug 11, 2023
@PureWeen PureWeen added the area-core-lifecycle XPlat and Native UIApplicationDelegate/Activity/Window lifecycle events label Aug 11, 2023
@PureWeen
Copy link
Member

Related #15833

@ghost
Copy link

ghost commented Aug 11, 2023

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@AnnYang01
Copy link
Collaborator

Verified this on Visual Studio Enterprise 17.8.0 Preview 1.0 using below Project (.NET 8.0), This issue repro on Android 13.0-API33.
MauiApp11.zip
CustomLayout

@AnnYang01 AnnYang01 added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Aug 23, 2023
@MichaelRumpler
Copy link
Contributor

I saw the same problem on iOS and Windows. The Unloaded event is raised when a new page is pushed, but the VisualElement is still on the navigation stack. I saw that in .NET8 RC1. My customers tell me that this worked before in the preview versions.

@mjo151
Copy link

mjo151 commented Sep 29, 2023

I am seeing this problem as well with .NET 8 RC1. It occurs on iOS and Android and I haven't tried Windows yet. When a page is pushed onto the navigation stack, the unloaded event is raised on that page. This is a change in behavior from .NET 7 and has major consequences. Is this expected behavior or a bug? It seems like a bug since the page has not been unloaded since it's still on the navigation stack. I would only expect OnDisappearing to be called in that case.

@Uridel
Copy link

Uridel commented Nov 24, 2023

I'm seeing the same issue in my project as well. Are there any plans to fix this, or what the correct handling should be?

@ibrahimmd90
Copy link

This issue has messed up resources management. Unload event is a vital event for releasing resources. I wish it gets higher attention to fix it. This issue started when I switched to .NET 8 production release from .NET 7.

@ibrahimmd90
Copy link

ibrahimmd90 commented Nov 25, 2023

There could be different workarounds depending on your application. In my application, I use shell tabs and navigation. It makes it little tricky to fix it. Here are what I have done which seems acceptable until now:
1- Create an abstract BasePage that overrides OnNavigatedFrom and OnNavigatedTo. Here it is.
C# only:

public abstract class BasePage : ContentPage
{
    public BasePage() : base()
    {
        
    }
    protected override void OnNavigatedFrom(NavigatedFromEventArgs args)
    {
        var shellSection = Shell.Current.CurrentItem.CurrentItem;
        if(shellSection != this.shellSection)
        {
            return;
        }

        var navigation = Application.Current.MainPage?.Navigation;
        var stack = navigation?.NavigationStack;
        var cnt = stack?.Count;
        if(cnt == pageCnt - 1)
        {
            if(BindingContext is BaseViewModel vm)
            {
                vm.NavigatedAway();
            }
        }
        base.OnNavigatedFrom(args);
    }

    private int? pageCnt;

    private ShellSection shellSection;

    protected override void OnNavigatedTo(NavigatedToEventArgs args)
    {
        if(pageCnt.HasValue)
        {
            return;
        }
        this.shellSection = Shell.Current.CurrentItem.CurrentItem;
        var navigation = Application.Current.MainPage?.Navigation;
        var stack = navigation?.NavigationStack;
        pageCnt = stack?.Count;
        base.OnNavigatedTo(args);
    }
}

2- As you could see, I am using an abstract BaseViewModel that has NavigatedAway method. Here is the implementation of this particular method:

/// <summary>
/// This flag is set to true when the ViewModel is created and cleared its view calls <see cref="NavigatedAway"/> 
/// to let whatever resource managment code dispose it.
/// </summary>
public bool ShouldRemainAlive { get; private set; } = true;

/// <summary>
/// Called by the view of this ViewModel when the view detects events that mean the resources associated with the view
/// should be disposed.
/// </summary>
public void NavigatedAway()
{
    if (disposedValue) return;
    ShouldRemainAlive = false;
}

3- In the place where you handle pages unload event, you check for this flag and see if it is time to release the ViewModel of this page. In my case, I use an extension method:

internal static class ServiceCollectionExtensions
{
    public static void AddPage<T>(this IServiceCollection serviceCollection) where T : VisualElement
    {
        serviceCollection.AddTransient(PageWithDisposableContext<T>);
    }
    private static T PageWithDisposableContext<T>(IServiceProvider serviceProvider) where T : VisualElement
    {
        var page = ActivatorUtilities.CreateInstance<T>(serviceProvider);
        page.Unloaded += (sender, e) =>
        {
            if(sender is VisualElement view)
            {
                if(view.BindingContext is BaseViewModel viewModel)
                {
                    if(!viewModel.ShouldRemainAlive)
                    {
                        viewModel.Dispose();
                    }
                }
            }
        };

        return page;
    }
}

I use this extension when I add pages during app building:
builder.Services.AddPage<YouPageThatImplementsBasePage>();

@samhouts samhouts added the p/1 Work that is critical for the release, but we could probably ship without label Dec 7, 2023
@shobanasuresh
Copy link

We're also hitting this issue in our projects after upgrading to .Net 8. The loaded event is raised when returning to a page that's already on the navigation stack. This causes problems with the initialization logic. Is this an intended change introduced in .Net 8 or a bug? If it's an intentional change, what's the correct event to handle one time initialization?

@MichaelRumpler
Copy link
Contributor

I used HandlerChanging instead (my issue). That event is even documented.

@dotMorten
Copy link
Contributor

dotMorten commented Dec 13, 2023

I'm confused about this bug report. When a UI component disappears from the view, I'd expect Unload to fire, and when it reappears I'd expect it to Load again. I don't get the argument that if it's in the navigation stack it shouldn't raise unload. If you're interested in navigation events, listen for navigation events.

The loaded event is raised when returning to a page that's already on the navigation stack. This causes problems with the initialization logic.

It sounds to me like you were relying on a bug in the load/unload lifecycle. They finally fixed this bug, and that is now causing problems for you. I think the more correct thing to do is move your initialization logic to the right place.

@marchev-prgs
Copy link
Author

I used HandlerChanging instead (my issue). That event is even documented.

I modified the original test project (from the description) and the HandlerChanging event is not raised when navigating to another page, nor when navigating back from that page. My point is that the HandlerChanging event cannot always be used as a work-around.

@PureWeen
Copy link
Member

PureWeen commented Jan 18, 2024

I'm confused about this bug report. When a UI component disappears from the view, I'd expect Unload to fire, and when it reappears I'd expect it to Load again. I don't get the argument that if it's in the navigation stack it shouldn't raise unload. If you're interested in navigation events, listen for navigation events.

The loaded event is raised when returning to a page that's already on the navigation stack. This causes problems with the initialization logic.

It sounds to me like you were relying on a bug in the load/unload lifecycle. They finally fixed this bug, and that is now causing problems for you. I think the more correct thing to do is move your initialization logic to the right place.

Yea, unloaded/loaded are tied to platform events indicating that the view is part of the visual tree or has been removed from the visual tree. It's definitely unfortunate this was broken in NET7.

I used HandlerChanging instead (my issue). That event is even documented.

@MichaelRumpler

I am curious why unloaded/loaded still didn't work for you? It makes me think there's some nuance to this issue I'm missing. You can still connect gestures inside loaded and then disconnect them inside unloaded. Is there a reason you need gestures to still be present on a control that's no longer part of the visual tree?

AFAICT the original issue was somewhat a bug.

Actual: See the log history in the collection view - the innerGrid received Unloaded when we navigated to the empty page, but Loaded was never raised after that.

Now with the original sample, the unloaded and loaded events correctly fire for the innerGrid and the custom layout when returning back to the initial page.

I realize the fix doesn't match the "Expected" but it's at least now not completely unexpected :-)

@PureWeen
Copy link
Member

I've checked with the original reporter of this issue, and it is now working in .NET 8 to their satisfaction.

If you have a variation to the original issue that you'd like to report, please create a new issue.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-core-lifecycle XPlat and Native UIApplicationDelegate/Activity/Window lifecycle events p/1 Work that is critical for the release, but we could probably ship without partner Issue or Request from a partner team platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests