Fix for MAUI RoutedViewHost #3303

ili · 2022-07-02T08:00:28Z

Fix for #3035

What is the current behavior?

Direct Router.NavigationStack manipulations are not affected on MAUI's NavigationPage.Navigation.NavigationStack
MAUI's NavigationPage.PoppedToRoot event is not supported
Double navigation to the same VM occures on application start

What is the new behavior?

Bugs are fixed
Sample app: https://github.com/ili/ReactiveUI-Navigation-Issue/tree/fix-maui-navigation

Other information:

Direct NavigationPage.Navigation manipulations are used to avoid page activations.

codecov · 2022-07-02T09:24:26Z

Codecov Report

Merging #3303 (6a2394f) into main (196b8a6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3303   +/-   ##
=======================================
  Coverage   63.83%   63.83%           
=======================================
  Files         157      157           
  Lines        5726     5726           
=======================================
  Hits         3655     3655           
  Misses       2071     2071

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 196b8a6...6a2394f. Read the comment docs.

glennawatson · 2022-07-02T09:35:16Z

Let me test and check out the code for you tomorrow. If it looks good I'll do a release for you then.

Thanks

src/ReactiveUI.Maui/RoutedViewHost.cs

ChrisPulman · 2022-07-05T08:20:43Z

Thanks for this PR.
I have tested the Android version and this seems to work as expected.

When I tested the same demo on Windows unfortunately the demo displays a blank screen due to a slightly different start-up process. I refactored some of the code to get it working however then I was then faced with a multitude of cross threading issues.
I will look into this in more detail to see if it's something to do with the manipulation of the core Navigation stack or just a scheduler issue elsewhere.

ChrisPulman · 2022-07-05T10:26:04Z

I can verify that both Windows and Android Targets now operate with the latest commit.
Waiting for @RLittlesII to verify Mac/iOS and to accept the PR

ili · 2022-07-05T15:01:30Z

src/ReactiveUI.Maui/RoutedViewHost.cs

+        MainThread.BeginInvokeOnMainThread(() =>
+        {
+            pg.Title = vm.UrlPathSegment;
+        });


@ChrisPulman there is the problem with setting title - setting it directly will break later binding.

Also if there are cross-thread issues may be it would be better to do smth like this:

Router? .Navigate .ObserveOn(RxApp.MainThreadScheduler) // <-- here .SelectMany(_ => PagesForViewModel(Router.GetCurrentViewModel())) .SelectMany(async page =>

?

I believe that it will only affect the ability to bind the Title, which with other platforms such as Xamarin we set the Title property too.
However there could be some argument that we shouldn't be setting this property on any platform to allow the implementor to choose what and how the properties are set or bound.
There should be commonality across the platforms in my opinion.
I'll ask @glennawatson to add his opinion on this.

@ChrisPulman i can't say about Xamarin, may be there are some difference between Xamarin bindings & MAUI compiled bindings, but the fact is - setting Title on navigating breaks bindings. I'v commented this code in my local RoutedViewHost implementation, and this saves :)

glennawatson · 2022-07-06T07:41:38Z

src/ReactiveUI.Maui/RoutedViewHost.cs

@@ -205,7 +209,13 @@ protected virtual Page PageForViewModel(IRoutableViewModel vm)

        ret.ViewModel = vm;

-        return (Page)ret;
+        var pg = (Page)ret;
+        MainThread.BeginInvokeOnMainThread(() =>


This isn't using the inbuilt RxUI way of doing this, so might be better to use RxApp.MainThreadScheduler

glennawatson · 2022-07-06T08:57:23Z

Sorry @ili we think we've identified an issue with Maui and the windows target unrelated to your code in regards to the schedulers.

I think this is why ObserveOn is failing for @ChrisPulman and a lot of the sample.

Will get @RLittlesII to do a test of your sample to make sure it works fine on iOS and then we can do a release soon, and we will work on solving the scheduler issue for Maui.

In terms of the auto title setting. I'm thinking having a property setting if that functionality is desirable is the way to go. This will allow an easier migration path for existing Xamarin users but more flexibility to users like yourself who don't want that functionality.

ili · 2022-07-06T10:43:02Z

@glennawatson @ChrisPulman really sorry if my comment would be stupid and obvious, but i'v faced with Windows cross thread exceptions until added reference to ReactiveUI.WinUI to my project:

	<ItemGroup Condition="'$(TargetFramework)' == 'net6.0-windows10.0.19041.0'">
	  <PackageReference Include="ReactiveUI.WinUI">
	    <Version>18.2.5</Version>
	  </PackageReference>
	</ItemGroup>

Use ObserveOn(RxApp.MainThreadScheduler) and RxApp.MainThreadScheduler.Schedule to handle cross threading.

ChrisPulman

Waiting for review of @RLittlesII before approval

ChrisPulman · 2022-07-06T20:01:24Z

src/ReactiveUI.Maui/RoutedViewHost.cs

+    /// <summary>
+    /// The Set Title on Navigate property.
+    /// </summary>
+    public static readonly BindableProperty SetTitleOnNavigateProperty = BindableProperty.Create(


Added Set Title On Navigate Property

ChrisPulman · 2022-07-06T20:01:58Z

src/ReactiveUI.Maui/RoutedViewHost.cs

                .DisposeWith(disposable);

            Router?
                .Navigate
-                .SelectMany(_ => PageForViewModel(Router.GetCurrentViewModel()))
+                .ObserveOn(RxApp.MainThreadScheduler)


Added ObserveOn(RxApp.MainThreadScheduler)

ChrisPulman · 2022-07-06T20:03:26Z

src/ReactiveUI.Maui/RoutedViewHost.cs

+        if (SetTitleOnNavigate)
+        {
+            RxApp.MainThreadScheduler.Schedule(() => pg.Title = vm.UrlPathSegment);


Used RxApp.MainThreadScheduler.Schedule to handle cross threading

github-actions · 2022-07-23T00:20:28Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Fix for MAUI RoutedViewHost

f27dfb9

glennawatson reviewed Jul 3, 2022

View reviewed changes

src/ReactiveUI.Maui/RoutedViewHost.cs Outdated Show resolved Hide resolved

Update RoutedViewHost.cs

0e4ae0c

ili requested a review from glennawatson July 4, 2022 04:57

Setting Page.Title prevents binding

fcce7ac

glennawatson requested review from RLittlesII and ChrisPulman July 5, 2022 08:41

ChrisPulman added 2 commits July 5, 2022 11:04

Fix code style warnings

d0f861f

Fix cross thread issue with Title

cab60bd

Add missing using

01c2c86

ili commented Jul 5, 2022

View reviewed changes

glennawatson reviewed Jul 6, 2022

View reviewed changes

ChrisPulman added 3 commits July 6, 2022 19:29

Add SetTitleOnNavigate property

6107e6b

Use ObserveOn(RxApp.MainThreadScheduler) and RxApp.MainThreadScheduler.Schedule to handle cross threading.

Add missing usings

1f714d0

Added missing using

48e1ac1

ChrisPulman reviewed Jul 6, 2022

View reviewed changes

Merge branch 'main' into patches/fix-maui-navigation

6a2394f

glennawatson approved these changes Jul 8, 2022

View reviewed changes

glennawatson enabled auto-merge (squash) July 8, 2022 01:41

glennawatson merged commit 8fa79d1 into reactiveui:main Jul 8, 2022

github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for MAUI RoutedViewHost #3303

Fix for MAUI RoutedViewHost #3303

ili commented Jul 2, 2022

codecov bot commented Jul 2, 2022 •

edited

Loading

glennawatson commented Jul 2, 2022

ChrisPulman commented Jul 5, 2022

ChrisPulman commented Jul 5, 2022

ili Jul 5, 2022 •

edited

Loading

ChrisPulman Jul 5, 2022

ili Jul 6, 2022

glennawatson Jul 6, 2022

glennawatson commented Jul 6, 2022

ili commented Jul 6, 2022

ChrisPulman left a comment

ChrisPulman Jul 6, 2022

ChrisPulman Jul 6, 2022

ChrisPulman Jul 6, 2022

github-actions bot commented Jul 23, 2022

Fix for MAUI RoutedViewHost #3303

Fix for MAUI RoutedViewHost #3303

Conversation

ili commented Jul 2, 2022

codecov bot commented Jul 2, 2022 • edited Loading

Codecov Report

glennawatson commented Jul 2, 2022

ChrisPulman commented Jul 5, 2022

ChrisPulman commented Jul 5, 2022

ili Jul 5, 2022 • edited Loading

Choose a reason for hiding this comment

ChrisPulman Jul 5, 2022

Choose a reason for hiding this comment

ili Jul 6, 2022

Choose a reason for hiding this comment

glennawatson Jul 6, 2022

Choose a reason for hiding this comment

glennawatson commented Jul 6, 2022

ili commented Jul 6, 2022

ChrisPulman left a comment

Choose a reason for hiding this comment

ChrisPulman Jul 6, 2022

Choose a reason for hiding this comment

ChrisPulman Jul 6, 2022

Choose a reason for hiding this comment

ChrisPulman Jul 6, 2022

Choose a reason for hiding this comment

github-actions bot commented Jul 23, 2022

codecov bot commented Jul 2, 2022 •

edited

Loading

ili Jul 5, 2022 •

edited

Loading