unoplatform / uno

Open-source platform for building cross-platform native Mobile, Web, Desktop and Embedded apps quickly. Create rich, C#/XAML, single-codebase apps from any IDE. Hot Reload included! 90m+ NuGet Downloads!!
https://platform.uno
Apache License 2.0
8.89k stars 720 forks source link

`Loading`, `Loaded` & `Unloaded` events are called too early #2895

Open MartinZikmund opened 4 years ago

MartinZikmund commented 4 years ago

Current behavior

The Loading and Loaded events are called immediately after the control is added in the visual tree.

Expected behavior

The controls start Loading and get Loaded only once the UI thread is freed up.

How to reproduce it (as minimally and precisely as possible)

I have two examples that demonstrate the differences.

Frame navigation

public sealed partial class MainPage : Page
{
    public MainPage()
    {
        this.InitializeComponent();
        Debug.WriteLine("Page constructor");
        Loading += MainPage_Loading;
        Loaded += MainPage_Loaded;
    }

    private void MainPage_Loading(FrameworkElement sender, object args)
    {            
        Debug.WriteLine("Page loading");
    }

    protected override void OnNavigatingFrom(NavigatingCancelEventArgs e)
    {
        base.OnNavigatingFrom(e);
        Debug.WriteLine("Page navigating from");
    }

    protected override void OnNavigatedTo(NavigationEventArgs e)
    {
        base.OnNavigatedTo(e);
        Debug.WriteLine("Page navigated to");
    }

    private void MainPage_Loaded(object sender, RoutedEventArgs e)
    {
        Debug.WriteLine("Page loaded");
    }
}

// navigating to the page
frame.Navigating += RootFrame_Navigating;
frame.Navigated += RootFrame_Navigated;
frame.NavigationStopped += RootFrame_NavigationStopped;
frame.Navigate(typeof(MainPage));
Debug.WriteLine("After Navigated");

The output is:

Frame navigating Page constructor Frame navigated Page navigated to After navigated Page loading Page loaded

Whereas in case of Uno it is:

Frame navigating Page constructor Page loading Page loaded Frame navigated Page navigated to After navigated

Creating & throwing away many controls

public partial class CustomControl : Control
{
    public CustomControl()
    {
        this.Loading += CustomControl_Loading;
        this.Loaded += CustomControl_Loaded;
    }

    private void CustomControl_Loading(
        Windows.UI.Xaml.FrameworkElement sender,
        object args)
    {
        Debug.WriteLine("Loading");
    }

    private void CustomControl_Loaded(
        object sender, 
        Windows.UI.Xaml.RoutedEventArgs e)
    {
        Debug.WriteLine("Loaded");
    }

    private void CustomControl_Unloaded(object sender, Windows.UI.Xaml.RoutedEventArgs e)
    {
        Debug.WriteLine("UnLoaded");
    }

    protected override void OnApplyTemplate()
    {
        base.OnApplyTemplate();
    }
}

// in button click handler
private void Button_Click(object sender, RoutedEventArgs e)
{
    for (int i = 0; i < 1000; i++)
    {
        var btn = new CustomControl();
        Content = btn;
    }
    Debug.WriteLine("After for loop");
}

Output on UWP:

After for loop Loading Loaded 999x Unloaded

Output on Uno:

Loading Loaded Unloaded Loading Loaded Unloaded ... 997x the same Unloaded Loading Loaded After for loop

Environment

Nuget Package: 2.0

Package Version(s): 2.0

Affected platform(s):

MartinZikmund commented 4 years ago

I have updated the issue with Unloaded event as well, as it is affected too.

MartinZikmund commented 4 years ago

Out of curiosity I tried to implement this "delayed" Loading and Loaded reporting for iOS only in this branch. This implementation is surely not ideal solution, handles FrameworkElement only and does not fix Unloaded, but the Samples app runs fine even after this change.

Essentially instead of raising OnLoading and OnLoaded immediately, they are enqueued and then batch executed when UI thread is available.

nor0x commented 4 years ago

not sure if this is related, but with the current version the splash screen is not disappearing once the app is already loaded, i have tested this on WASM

The following block doesn't get removed or hidden for me.

<div id="uno-loading">
    <img src="Assets/SplashScreen.scale-200.png" id="uno-loading-splash" class="uno-splash">
</div>
MartinZikmund commented 4 years ago

@jeromelaban Is it realistic to resolve this without significant breaking changes? It is probably the main blocker for MvvmCross support. If it cannot be solved easily, I will check if MvvmCross.Uno code can be modified to circumvent this somehow.

jeromelaban commented 4 years ago

It is realistic (and required) to fix this without breaking changes, but it would most likely be fixed properly by porting the original Frame code. I would suggest that, for now, modifying the MvvmCross code to adjust for the behavior is the safest course of action.

Note that FrameworkElement.IsLoaded is available in WinUI and can be used to compensate for this behavior in a backward compatible way, once this is fixed in Uno.

MartinZikmund commented 4 years ago

It is realistic (and required) to fix this without breaking changes, but it would most likely be fixed properly by porting the original Frame code. I would suggest that, for now, modifying the MvvmCross code to adjust for the behavior is the safest course of action.

Note that FrameworkElement.IsLoaded is available in WinUI and can be used to compensate for this behavior in a backward compatible way, once this is fixed in Uno.

I think the problem is not only in Frame, but in other controls too - the Loaded event is in general called immediately after the control is attached to tree, whereas in UWP it is executed after the UI thread is free - see the CustomControl example code in the issue description.

We were looking into it with @nickrandolph , the problem for MvvmCross can be worked around for now by using INotifyPropertyChanged on the page and notifying after the VM was set to refresh all x:Binds (but it also means all x:Bind must be at least OneWay), or use classic Binding. This workaround could be mentioned as a temporary limitation for the first release.

dr1rrb commented 4 years ago

🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨 🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨 🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

Note, I discovered that on UWP if we add a child to a parent while in the Loaded event handler of the parent, then the child will receive the Loaded BEFORE the Loading !!!

🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨 🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨 🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

Note2 : The tests disabled by https://github.com/unoplatform/uno/pull/3728/commits/2d10b14dbefd039365319a3bce30d40cb3c276d7 should be updated and multi-targeted

DierkDroth commented 4 years ago

Curious: is there any ETA on when the underlying issue would be addressed?

jeromelaban commented 4 years ago

@DierkDroth there is no ETA for this issue yet, it is quite complex to address as it changes many underlying sequences across all platforms. We're working on it, this thread will get updated as we go.

DierkDroth commented 4 years ago

Thanks for the update, @jeromelaban

Youssef1313 commented 10 months ago

We'll need to revisit this workaround:

https://github.com/unoplatform/uno/blob/69b64c91fb67c693bcbe08c2175afd1836a4751c/src/Uno.UI/Microsoft/UI/Xaml/Controls/TabView/TabView.cs#L600-L601

Youssef1313 commented 10 months ago

ContentControl specifically seems to be a bit special.

image

It looks like when Content is set, it's not immediately added as a child (the comment in WinUI code says this is done "later".

Then, for some reason, fIsLive is set to false which may have an effect on Loaded event.

In the case of Uno, setting ContentControl.Content will immediately add the child.

image

Youssef1313 commented 10 months ago

Also note that UserControl in WinUI is NOT ContentControl, while in Uno it's a ContentControl.

If we aligned ContentControl behavior with WinUI, we will have to verify UserControls behavior as well on WinUI and exclude them from that special behavior until the inheritance is fixed.

Youssef1313 commented 10 months ago
    public partial class MyStackPanel : StackPanel
    {
        public MyStackPanel()
        {
            var btn = new Button();
            btn.Loading += Btn_Loading;
            btn.Loaded += Btn_Loaded;
            this.Children.Add(btn);
        }

        private void Btn_Loaded(object sender, RoutedEventArgs e)
        {
            Debug.WriteLine("Loaded");
        }

        private void Btn_Loading(FrameworkElement sender, object args)
        {
            Debug.WriteLine("Loading");
        }

        protected override Size MeasureOverride(Size availableSize)
        {
            return new Size(500, 500);
        }

        protected override Size ArrangeOverride(Size finalSize)
        {
            return new Size(500, 500);
        }
    }

Using the above code, the button will get "Loaded" but not "Loading".

Note that if ArrangeOverride is removed, it will get both. This is because Arrange will attempt to measure if it detects the element wasn't measured.

Youssef1313 commented 10 months ago

Another interesting case:

// sp is an already loaded stack panel.
var btn = new Button();
btn.Loading += Btn_Loading;
btn.Loaded += Btn_Loaded;
btn.Unloaded += Btn_Unloaded;
sp.Children.Add(btn);
sp.Children.Clear();
sp.Children.Add(btn);

The order of events is:

  1. Loaded
  2. Loading
  3. Unloaded

But the end state of btn is IsLoaded = true.

KWodarczyk commented 1 month ago

I have also observed that all If I have multiple pages in my application their "Loaded" event if fired without event going to the page. Not sure if this is related but I need to figure out how to make "Loaded" event fire only when page is accessed any ideas ?

MartinZikmund commented 1 month ago

I have also observed that all If I have multiple pages in my application their "Loaded" event if fired without event going to the page. Not sure if this is related but I need to figure out how to make "Loaded" event fire only when page is accessed any ideas ?

That is certainly odd, which platform are you observing this on? Can you create a repro and post as a separate issue please?