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

[regression/8.0.0-preview.3.8149] - modal to modal doesnt call OnNavigatedTo #17978

Closed
Zack-G-I-T opened this issue Oct 12, 2023 · 5 comments · Fixed by #18912
Closed

[regression/8.0.0-preview.3.8149] - modal to modal doesnt call OnNavigatedTo #17978

Zack-G-I-T opened this issue Oct 12, 2023 · 5 comments · Fixed by #18912
Assignees
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@Zack-G-I-T
Copy link

Zack-G-I-T commented Oct 12, 2023

Description

When you are on a modal page and navigate to another modal page using Shell.Current.GoToAsync, the OnNavigatedTo method is called on the first modal page - but never on the second.

Steps to Reproduce

  1. Download project
  2. Add breakpoints to all OnNavigatedTo methods
  3. Run app and click buttons to go to each page - see that the breakpoints never get hit.

Link to public reproduction project repository

https://github.com/Zack-G-I-T/Net8ModalOnNavigatedTo

Version with bug

8.0.0-preview.3.8149

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.0-preview.2.7871

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

No response

@Zack-G-I-T Zack-G-I-T added the t/bug Something isn't working label Oct 12, 2023
@samhouts samhouts added the potential-regression This issue described a possible regression on a currently supported version., verification pending label Oct 12, 2023
@samhouts samhouts added this to the .NET 8 GA milestone Oct 12, 2023
@samhouts samhouts added platform/android 🤖 area-controls-shell Shell Navigation, Routes, Tabs, Flyout labels Oct 12, 2023
@samhouts samhouts changed the title Net 8 - modal to modal doesnt call OnNavigatedTo [regression/8.0.0] - modal to modal doesnt call OnNavigatedTo Oct 12, 2023
@XamlTest
Copy link

Verified this on Visual Studio Enterprise 17.8.0 Preview 3.0(8.0.0-rc.2.9373). This issue does not repro on Windows 11, Android 13.0-API33 and iOS 16.4 with below Project:
TestModalNet8.zip
Breakpoint

@XamlTest XamlTest added s/triaged Issue has been reviewed s/try-latest-version Please try to reproduce the potential issue on the latest public version labels Oct 16, 2023
@ghost
Copy link

ghost commented Oct 16, 2023

Hi @Zack-G-I-T. We have added the "s/try-latest-version" label to this issue, which indicates that we'd like you to try and reproduce this issue on the latest available public version. This can happen because we think that this issue was fixed in a version that has just been released, or the information provided by you indicates that you might be working with an older version.

You can install the latest version by installing the latest Visual Studio (Preview) with the .NET MAUI workload installed. If the issue still persists, please let us know with any additional details and ideally a reproduction project provided through a GitHub repository.

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.

@Zack-G-I-T
Copy link
Author

@XamlTest if you look at the class you are breaking on - it is the same one each time. "TestModalPage.xaml.cs". It never hits the "TestModalPage2.xaml.cs" OnNavigatedTo.
Why would it hit the OnNavigatedTo method on the first page when you are going away from it?

@ghost ghost removed the s/try-latest-version Please try to reproduce the potential issue on the latest public version label Oct 16, 2023
@samhouts samhouts added i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Oct 16, 2023
@samhouts samhouts modified the milestones: .NET 8 GA, .NET 8 SR1 Oct 16, 2023
@samhouts
Copy link
Member

samhouts commented Oct 16, 2023

Confirmed that this regressed between 8.0.0-preview.2.7871 and 8.0.0-preview.3.8149. #13025 seems sus. Note that in preview 2, the NavigatedTo event fires for Modal1 every time, but it also fires the event for the modal on top.

@samhouts samhouts changed the title [regression/8.0.0] - modal to modal doesnt call OnNavigatedTo [regression/8.0.0-preview.3.8149] - modal to modal doesnt call OnNavigatedTo Oct 16, 2023
@XamlTest
Copy link

@XamlTest if you look at the class you are breaking on - it is the same one each time. "TestModalPage.xaml.cs". It never hits the "TestModalPage2.xaml.cs" OnNavigatedTo. Why would it hit the OnNavigatedTo method on the first page when you are going away from it?

Yes, you are right, thanks for reminder. It never hits the "TestModalPage2.xaml.cs" OnNavigatedTo.

@XamlTest XamlTest added the s/verified Verified / Reproducible Issue ready for Engineering Triage label Oct 17, 2023
@PureWeen PureWeen removed their assignment Nov 3, 2023
@PureWeen PureWeen modified the milestones: .NET 8 SR1, .NET 8 SR2 Nov 3, 2023
@emaf emaf self-assigned this Nov 14, 2023
emaf added a commit that referenced this issue Nov 20, 2023
When navigating between Modal pages, Shell was returning the first page in the stack as the current page, so that page was the one being notified about the navigation. Instead, we should return the last page from the stack.

Fixes #17978
@github-project-automation github-project-automation bot moved this from Todo to Done in MAUI SDK Ongoing Nov 21, 2023
PureWeen pushed a commit that referenced this issue Nov 21, 2023
When navigating between Modal pages, Shell was returning the first page in the stack as the current page, so that page was the one being notified about the navigation. Instead, we should return the last page from the stack.

Fixes #17978
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! label Jan 31, 2024
@samhouts samhouts modified the milestones: .NET 8 SR2, .NET 8 SR1 Jan 31, 2024
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 fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants