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.93k stars 724 forks source link

`x:Bind` is evaluated too early causing changes in `Page.OnNavigatedTo` to be ignored #2872

Open MartinZikmund opened 4 years ago

MartinZikmund commented 4 years ago

Current behavior

When await is used before first navigation in App.xaml.cs, x:Bind suddenly behaves "weirdly" - it binds data which are available during page constructor, but does not consider any updates during OnNavigatedTo (which is when MVVM frameworks like MvvmCross set the DataContext).

Expected behavior

x:Bind behavior should not change if user awaits in OnLaunched.

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

  1. Create a new Uno Platform app
  2. Set MainPage.xaml as follows:
<Page
    x:Class="BindBreak.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:BindBreak"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d">

    <Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
        <TextBlock Text="{x:Bind Text}" Margin="20" FontSize="30" />
    </Grid>
</Page>
  1. Set code-behind as follows:
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.WindowsRuntime;
using Windows.Foundation;
using Windows.Foundation.Collections;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Controls.Primitives;
using Windows.UI.Xaml.Data;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Media;
using Windows.UI.Xaml.Navigation;

// The Blank Page item template is documented at http://go.microsoft.com/fwlink/?LinkId=402352&clcid=0x409

namespace BindBreak
{
    /// <summary>
    /// An empty page that can be used on its own or navigated to within a Frame.
    /// </summary>
    public sealed partial class MainPage : Page
    {
        public MainPage()
        {
            this.InitializeComponent();
            this.DataContextChanged += MainPage_DataContextChanged;
        }

        private void MainPage_DataContextChanged(DependencyObject sender, DataContextChangedEventArgs args)
        {
            Text = "In data context changed";
        }

        protected override void OnNavigatedTo(NavigationEventArgs e)
        {
            base.OnNavigatedTo(e);
            DataContext = new object();
        }

        public string Text { get; set; } = "Should not display";
    }
}
  1. Run the app on UWP, Android. Notice in both cases app says "In data context changed"
  2. In App.xaml.cs change OnLaunched to be async:
    protected override async void OnLaunched(LaunchActivatedEventArgs e)
  3. Add delay before navigating in OnLaunched:
    await Task.Delay(1000);
    rootFrame.Navigate(typeof(MainPage), e.Arguments);
  4. Launch app on UWP. Observe there is no change in behavior.
  5. Launch app on Android. Observe it now shows "Should not display".

Workaround

Add the following statement at the end of the OnNavigatedTo method:

Bindings.Update();

Environment

Nuget Package: latest stable + latest dev

Package Version(s):

Affected platform(s):

Visual Studio:

Relevant plugins:

Anything else we need to know?

MartinZikmund commented 4 years ago

After a bit further digging, the async is not the main issue - when we add another page with the same logic and add a button to MainPage to navigate to it, the second page shows "Should not display" even if OnLaunched is not async. So it seems for any page that not loaded immediately after start of the application x:Bind behaves "incorrectly"

MartinZikmund commented 4 years ago

Added higher priority, as this is important for MvvmCross apps to work seamlessly when Uno support is added.

jeromelaban commented 4 years ago

It seems one of the problems is the ordering of page Loaded and navigation events and overrides, which may be related to https://github.com/unoplatform/uno/pull/2008.

MartinZikmund commented 4 years ago

@jeromelaban Oh, I must ping him. I have also noticed the reason is probably the event ordering - for subsequent navigations, Loaded event runs before OnNavigatedTo, which means view model/DataContext is not set yet and hence things like ViewAppearing/ViewAppeared in MvvmCross stop working.

MartinZikmund commented 4 years ago

Leaving here for reference - https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.page.onnavigatedto#remarks - OnNavigatedTo should always be called before Loaded

Page.OnNavigatedTo(NavigationEventArgs) Method (Windows.UI.Xaml.Controls) - Windows UWP applications
MartinZikmund commented 4 years ago

This is caused by #2895.

jeromelaban commented 2 years ago

A workaround for this particular issue is to use:

Bindings.Update();

At the end of the OnNavigatedTo method. This will force the bindings to re-evaluate to their newer value (Since OnNavigatedTo is evaluated too late because of https://github.com/unoplatform/uno/issues/2895.

hawkerm commented 2 years ago

Not sure if related or slightly different, but I'm using the common behavior/load pattern to do async initialization of my ViewModel:

    <i:Interaction.Behaviors>
        <act:EventTriggerBehavior EventName="Loaded">
            <act:InvokeCommandAction Command="{x:Bind ViewModel.InitializeCommand, Mode=OneWay}" />
        </act:EventTriggerBehavior>
    </i:Interaction.Behaviors>

So while Bindings.Update() appears to work the first load, this trigger for the Loaded is being missed on subsequent page navigations in WPF/WASM, where it works fine in UWP.

I may have to see if I can just move this behavior to the OnPageNavigated override instead though, but is another quirk of the loading order being out of order. Will have to test that out later.