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

Navigation stops working #13695

Closed
williambuchanan2 opened this issue Mar 4, 2023 · 25 comments
Closed

Navigation stops working #13695

williambuchanan2 opened this issue Mar 4, 2023 · 25 comments
Assignees
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-navigation NavigationPage platform/android 🤖 s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@williambuchanan2
Copy link

Description

I am using the following to navigate (as per example app in linked repository below):

Shell.Current.GoToAsync or
Shell.Current.FindByName with Shell.Current.CurrentItem = xxx

Both work ok, and then for no apparent reason stop working.

My guess is i'm doing something wrong, but even if that is the case some kind of exception or information would be nice. Literally nothing happens. The app just doesn't navigate and I have no idea why.

There are different scenarios that cause it. I have managed to reliably reproduce 2 of them in the example.

Steps to Reproduce

Look at the example in the repository link below.

Scenario 1 - just keep clicking Next. When you get to Page 16 the next button stops working. The code being called - I have tried both navigate methods (1 of them is commented out) but neither works.

Scenario 2 - click the Scenario 2 button on the first page then keep clicking next. Then when you get to around page 4 notice that the back button stops working.

Link to public reproduction project repository

https://github.com/williambuchanan2/MauiNavigation.git

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

Android, I was not able test on other platforms

Affected platform versions

Android 11 and 13

Did you find any workaround?

No. Need to kill the app and reload

Relevant log output

No response

@williambuchanan2 williambuchanan2 added the t/bug Something isn't working label Mar 4, 2023
@jsuarezruiz jsuarezruiz added the area-navigation NavigationPage label Mar 6, 2023
@Cybrosys
Copy link

Cybrosys commented Mar 6, 2023

Perhaps a duplicate of #13538 ?

@williambuchanan2 Have you tried wrapping the navigation calls with Dispatcher so they are executed on the UI thread?

There's an extensions class for Dispatcher reducing the code complexity a bit:
https://github.com/dotnet/maui/blob/main/src/Core/src/Dispatching/DispatcherExtensions.cs

@rachelkang rachelkang added s/needs-info Issue needs more info from the author area-controls-shell Shell Navigation, Routes, Tabs, Flyout labels Mar 6, 2023
@ghost
Copy link

ghost commented Mar 6, 2023

Hi @williambuchanan2. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@williambuchanan2
Copy link
Author

I tried that but it doesn't make any difference.

@ghost ghost added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Mar 7, 2023
@PureWeen PureWeen added this to the Backlog milestone Mar 7, 2023
@PureWeen PureWeen removed the s/needs-attention Issue has more information and needs another look label Mar 7, 2023
@ghost
Copy link

ghost commented Mar 7, 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] (https://github.com/dotnet/maui/blob/main/docs/TriageProcess.md).

@PureWeen PureWeen self-assigned this Mar 7, 2023
@williambuchanan2
Copy link
Author

Just checking if there is any progress on this?

@PureWeen
Copy link
Member

@williambuchanan2 if you await your shell navigation then it will surface helpful exception messages

image

@PureWeen PureWeen added the s/needs-info Issue needs more info from the author label Apr 21, 2023
@ghost
Copy link

ghost commented Apr 21, 2023

Hi @williambuchanan2. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@williambuchanan2
Copy link
Author

Hi @PureWeen, I had noticed those errors. The trouble is when you follow the advice given in the error messages you just get another error. BTW, I didn't know whether I had to take the advice in the error literally (i.e. use ///: ///) because that format doesn't seem to be documented anywhere, so I tried all the below:

await Shell.Current.GoToAsync("///: ///TabH");

image

await Shell.Current.GoToAsync("///TabH");

image

await Shell.Current.GoToAsync("//TabH");

image

Either way - none of the above works.

I then try the other method (which again works some of the time):

            ShellItem shellItem = (ShellItem)Shell.Current.FindByName("TabHShellItem");
            Shell.Current.CurrentItem = shellItem;

But it just does nothing, and doesn't throw an error, so I have no idea why on this occasion it doesn't want to work.

I guess I am doing something wrong somewhere, but it would be good to have some kind of consistent way of navigating without having to worry about it randomly not working. Why does everything work ok, and then for some unknown reason the same code just no longer works?

@ghost ghost added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Apr 22, 2023
@cat0363
Copy link
Contributor

cat0363 commented May 11, 2023

It is only for Scenario 1 to describe.

Doesn't this mean that the route name defined in AppShell.xaml has multiple TabHs and
you are wondering where to transition?

There are two definitions of TabH.

<FlyoutItem Title="Logout" x:Name="TabHShellItem">
    <ShellContent ContentTemplate="{DataTemplate views:TabH}" Route="TabH" /> <!-- This -->
</FlyoutItem>

<FlyoutItem Title="Home"  x:Name="MainShellItem">
    <Tab Title="NavIssue">
        <ShellContent Title="Tab F" ContentTemplate="{DataTemplate views:TabF}" Route="TabF"/>
        <ShellContent Title="Tab G" ContentTemplate="{DataTemplate views:TabG}" Route="TabG"/>
        <ShellContent Title="Tab H" ContentTemplate="{DataTemplate views:TabH}" Route="TabH"/> <!-- This -->
    </Tab>
</FlyoutItem>

It seems that it is not possible to decide which TabH to transition to.

Change the definition of AppShell.xaml as follows.

<ShellItem FlyoutItemIsVisible="False" x:Name="LoadingPageShellItem" Title="Loading Page" Route="Login">
    <ShellContent ContentTemplate="{DataTemplate views:NewPage1}" Route="NewPage1" />
    <ShellContent ContentTemplate="{DataTemplate views:TabH}" Route="TabH" />
</ShellItem>

Specify the name of Login in the Route of ShellItem.
Add TabH to Route in ShellContent.

Try using //Login/TabH as the argument of the GoToAsync method called
by the NextButtonPressed method in Class16.cs.

Since the method needs to be modified, it will not work as it is.

await NavigationUtil.NavigateAsync<TabH>();

At least as far as I tried, it navigated to TabH.

As an aside, I think that if you delete all TabH definitions in AppShell.xaml, it will work as intended.
This is probably because the transition destination is determined uniquely. However, Logout, Back
will not work.

I'm not sure what you're expecting, so I'm sorry if I'm wrong.

@williambuchanan2
Copy link
Author

It is only for Scenario 1 to describe.

Hi @cat0363

Thank you. Yes, you are right. 🥇

I had misunderstood what Route meant. I thought it was to take you to that route (i.e. the route to take when you click = 'TabH'). I didn't realise it actually creates a duplicate item with that name.

Any thoughts on Scenario 2? :-)

@cat0363
Copy link
Contributor

cat0363 commented May 12, 2023

In order to coexist Scenario 1 and Scenario 2, it is necessary to change as follows.
Change Route definition in AppShell.xaml as below:

<ShellItem FlyoutItemIsVisible="False" x:Name="LoadingPageShellItem" Title="Loading Page" Route="Login">
    <ShellContent ContentTemplate="{DataTemplate views:NewPage1}" />
    <ShellContent ContentTemplate="{DataTemplate views:TabH}" Route="TabH" />
</ShellItem>

<FlyoutItem Title="Home"  x:Name="MainShellItem">

    <ShellContent Title="Home" Icon="home.png" ContentTemplate="{DataTemplate views:TabA}" />
    <ShellContent Title="Notifications" Icon="alert.png" ContentTemplate="{DataTemplate views:TabB}" />
    <ShellContent Title="Set Mood" Icon="set_mood2.png" ContentTemplate="{DataTemplate views:TabC}"  />

    <Tab Title="Chat" Icon="msg.png">
        <ShellContent Title="Messages" ContentTemplate="{DataTemplate views:TabD}"  />
        <ShellContent Title="Message Board" ContentTemplate="{DataTemplate views:TabE}"  />
    </Tab>

    <Tab Title="NavIssue">
        <ShellContent Title="Tab F" ContentTemplate="{DataTemplate views:TabF}" />
        <ShellContent Title="Tab G" ContentTemplate="{DataTemplate views:TabG}" />
        <ShellContent Title="Tab H" ContentTemplate="{DataTemplate views:TabH}" />
    </Tab>
</FlyoutItem>

<FlyoutItem Title="Settings" FlyoutIcon="setting.png" x:Name="SettShellItem">
    <Tab Title="Settings" Icon="user.png">
        <ShellContent Title="Profile Options" ContentTemplate="{DataTemplate views:SettA}" />
        <ShellContent Title="App Settings" ContentTemplate="{DataTemplate views:SettB}" />
    </Tab>
</FlyoutItem>

<FlyoutItem Title="Reset" FlyoutIcon="reset.png" x:Name="ResetShellItem">
    <ShellContent ContentTemplate="{DataTemplate views:NewPage1}" />
</FlyoutItem>

<FlyoutItem Title="Logout" x:Name="TabHShellItem">
    <ShellContent ContentTemplate="{DataTemplate views:TabH}" />
</FlyoutItem>

Scenario 2 resulted in the following exception:

System.ArgumentException: Ambiguous routes matched for: 

Therefore, I deleted the unnecessary Route definition.
I referred below.
https://www.reddit.com/r/xamarindevelopers/comments/p4es4p/shell_app_navigation_error_on_android_ambiguous/

In addition to the above, you should:(Below is what I posted yesterday.)

Try using //Login/TabH as the argument of the GoToAsync method called
by the NextButtonPressed method in Class16.cs.

At least I did the above and it worked.
By the way, to catch an exception, you can't catch it unless you await the asynchronous method.

Isn't this a Discussion, not an Issue?
First of all, I think it would be better to ask for opinions on whether or not this is a
bug inherent in .NET MAUI in the discussion. Also, you should post a model that
simplifies the problem. You should at least create a minimal model that reproduces
the problem. This is because if it is too complicated, it will take time to analyze it,
and people will not want to analyze it.

Since I don't know the detailed expected result and the code is complicated, I can't write more.

@williambuchanan2
Copy link
Author

Since I don't know the detailed expected result and the code is complicated, I can't write more.

Hi @cat0363

Thanks. Yes that also seems to work. So it's fair to say the problems were to do with the way I implemented this. I didn't realise that the AppShell could be structured like that.

However, just to cover off some of the points you raised. Firstly, a couple of people from Microsoft have looked at this and provided comments which didn't fix the problem, so they clearly couldn't work out the problem either. That in itself tells me that a) this thing isn't documented well enough, and b) there should have been some kind of meaningful error message and/or compiler warning (if as you say it is generating duplicates, then surely we should be warned because this is going to cause a problem). The fact that it works some of the time just makes it more confusing than if it didn't work at all. The documentation shows some very basic examples but doesn't explain any of what you have explained. What I had done was in line with the documentation (in fact I copied the example code as my start point). The error message that gets thrown actually gives you advice which doesn't work, so rather than help it just makes things worse because I then lost time trying to follow that advice.

As for raising this as a discussion - we have lost months going round in circles trying to get things working, assuming we are doing something wrong, then finally coming to the conclusion that it must be a bug. We are at the point now where as soon as something doesn't work we just naturally assume it is yet another bug and then move on to something else rather than try to fix it. But honestly - what difference does it make raising it as a discussion? Nothing is really getting addressed anyway so it doesn't make any difference.

Thank you for your help. At least I can move on from this problem, although I would argue that some changes should be made to prevent others from having this problem.

@cat0363
Copy link
Contributor

cat0363 commented May 15, 2023

Hi, @williambuchanan2

If you've reported it as a bug, you should at least detail the expected result.
It just states that nothing happens, and I'm not sure what you're expecting.
Also, when it comes to this issue, you should write a detailed description of each scenario.
I had to parse the code as there is no detailed explanation for each scenario.
And you should build the minimal model that reproduces the problem.
When the model to reproduce the problem is large, it is difficult to find where the essence of the problem lies.

The above might have led to a quicker solution to the problem.

If you post it in Discussion, it will catch more people's eyes. I think it's an advantage.
However, if you want many people to see it, the above is necessary.
It's not wrong to report it as an issue.

As you said, I think that the following pages should have explanations for each exception.
https://learn.microsoft.com/en-us/dotnet/maui/fundamentals/shell/navigation

Since there is no explanation in the documentation, I ended up looking for similar issues on various forums.
I'm glad your problem seems to have been resolved.

@UkeHa
Copy link

UkeHa commented May 30, 2023

I also ran into edge cases where the back button would break the navigation behaviour in Android. I was unable to figure out why the navigation broke reproducably in certain scenarios. In my case it happened once i hit the back button to navigate up in the stack. That led to images not being loaded anymore (or more accurately it was rendered but then disappeared) and accessing the same page again left me with a blank page.

I fixed my issue by removing the page once the back button is pressed, which then shows the previous page as i intended.

Shell.Current.CurrentPage.Navigation.RemovePage(Shell.Current.CurrentPage);

@MissedSte4k
Copy link

@UkeHa Had the same problem. I'm pretty sure this was solved by this. The blank page was always accompanied by this issue/error for us, so I would imagine it was it. Updating my app to .net 8 where this fix is pushed solved the issue for me.

@Zhanglirong-Winnie Zhanglirong-Winnie added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jun 25, 2023
@Zhanglirong-Winnie
Copy link

Verified this issue with Visual Studio Enterprise 17.7.0 Preview 2.0. Can repro on android platform with sample project.
MauiNavigation-main.zip
13695

@UkeHa
Copy link

UkeHa commented Jun 26, 2023

@samhouts could the fix @MissedSte4k mentioned be backported to .net7?

@PureWeen
Copy link
Member

@williambuchanan2 apologies on the late reply here

I'm having a little trouble following what parts of the documentation would be best to improve here.

It seems like you had two routes with the same name so Shell couldn't figure out which one you wanted just based on ///TabH so we should expand on what "Ambiguous match" means/refers to?

@UkeHa
Copy link

UkeHa commented Jun 26, 2023

@PureWeen, I think the biggest issue is that navigation is broken in .net 7 if there's an image on the shell. That makes it kinda hard to release..

@williambuchanan2
Copy link
Author

@williambuchanan2 apologies on the late reply here

I'm having a little trouble following what parts of the documentation would be best to improve here.

It seems like you had two routes with the same name so Shell couldn't figure out which one you wanted just based on ///TabH so we should expand on what "Ambiguous match" means/refers to?

I don't think the documentation is the real problem. I think using the word "Ambiguous" in the error is the main issue, but in general the way the error is dealt with is confusing. Yes, I misunderstood how the shell parameters worked (my bad), but had I straight away seen an error telling me there are "duplicate" routes (rather than Ambiguous) I might have had a better chance of working it out. What happens is everything seems to work ok for a while, and no error message gets thrown (unless you know to await the call) - all this adds to the confusion.

So, my thoughts in hindsight are:
a. generate a compile error in this scenario.
b. if that's not possible throw a "duplicate route in shell" exception (without needing to await to catch it) as soon as the shell loads.
c. and, have this documented.

For the documentation - I assumed that the "route" was just the name of the view you want to load, so I didn't think anything of the fact that I had the same one twice. Seems obvious now but I don't think the documentation even mentions "Route" so pretty sure I was left guessing...

@UkeHa
Copy link

UkeHa commented Jul 25, 2023

@williambuchanan2 you can use images, you simply have to null the variable before setting it and navigation stops breaking. I tested it in the latest .net7 build and it worked in Android 13.

sample:
imagesource = null
imagesource = imageUrl;

source:
#14471 (comment)

@samhouts
Copy link
Member

samhouts commented Aug 4, 2023

Duplicate of #14052

@samhouts samhouts marked this as a duplicate of #14052 Aug 4, 2023
@samhouts samhouts closed this as not planned Won't fix, can't repro, duplicate, stale Aug 4, 2023
@samhouts
Copy link
Member

samhouts commented Aug 4, 2023

We'll see if we can safely backport the fix to NET 7, friends! :)

@UkeHa
Copy link

UkeHa commented Aug 25, 2023

@samhouts that would be great. I also ran into the issue when using Camera.Maui and trying to scan a barcode on android. Removing the page from the stack isn't a viable solution in this case, as i want to pass the query parameter. Could you provide some info if this will be backported and when the next SR will be? Thanks!

@samhouts
Copy link
Member

It looks like the related fix is pending merge: #16640. I would not expect this to be available until October at this point.

If by any chance this does not resolve your issue, please submit a new one for your specific scenario. This issue may have multiple root causes. If you're able to test with NET 8 previews today, that could speed up the resolution. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-navigation NavigationPage platform/android 🤖 s/needs-attention Issue has more information and needs another look 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

10 participants