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

[BUG] Route parameter behavior is not consistent #17517

Open
mmiller-d8 opened this issue Sep 20, 2023 · 2 comments
Open

[BUG] Route parameter behavior is not consistent #17517

mmiller-d8 opened this issue Sep 20, 2023 · 2 comments
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout platform/iOS 🍎 s/triaged Issue has been reviewed t/bug Something isn't working
Milestone

Comments

@mmiller-d8
Copy link

mmiller-d8 commented Sep 20, 2023

Description

Shell.Current.Navigation.PushAsync and Shell.Current.Navigation.PushModalAsync does call IQueryAttributable.ApplyQueryAttributes by default (with empty parameters) and does not allow you to pass in custom parameters.

Shell.Current.GoToAsync does not call IQueryAttributable.ApplyQueryAttributes by default and does allow you to pass in custom parameters.

These should behave in the same way. In my opinion, both should call IQueryAttributable.ApplyQueryAttributes by default and both should allow you to pass in your own route parameters.

If this gets picked up and a reproduction is desired, I will happily put something together.

Steps to Reproduce

Scenario #1

  1. Create a page with a view model that implements IQueryAttributable. For convenience, make the view model a public property.
  2. Use Shell navigation to navigate to that page without route parameters (or with an empty dictionary. Both behave the same)
  3. Observe that ApplyQueryAttributes does not get called

Scenario #2

  1. Create an instance of the same page
  2. Use PushAsync to navigate to that same page
  3. Observe that you do not have the option of passing your own
  4. Observe that ApplyQueryAttributes does get called

Scenario #3

  1. Create an instance of the same page
  2. Find the view model field and call ApplyQueryAttributes yourself. Store the query attributes that came in (in the view model).
  3. Call PushAsync with the page instance to navigate to that page
  4. Observe that ApplyQueryAttributes gets called and resets your stored route parameters. And causes other miscellaneous havoc in your view model if you're using it to initialize your view model.

Link to public reproduction project repository

No response

Version with bug

7.0.92

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

iOS, I was not able test on other platforms

Affected platform versions

iOS 16

Did you find any workaround?

With a bunch of custom code. I have a navigation abstraction so that I can control passing in route parameters. Because Shell.Current.GoToAsync won't call IQueryAttributable.ApplyQueryAttributes, I am passing a dummy route parameter to force it. That's gross, but I didn't see a better way.

I'm also wrapping PushAsync and PushModalAsync so that I can use dependency injection to initialize the page and view mdoel. That should also not be necessary - I should just be able to pass a type (e.g. Shell.Current.PushModalAsync(myRouteParameters, true) and it should get the page from the IoC, but that's a different issue.

Relevant log output

No response

@mmiller-d8 mmiller-d8 added the t/bug Something isn't working label Sep 20, 2023
@Eilon Eilon added the area-controls-shell Shell Navigation, Routes, Tabs, Flyout label Sep 20, 2023
@mmiller-d8
Copy link
Author

mmiller-d8 commented Oct 4, 2023

I'm not sure if I should create a separate issue or not, but I found a fourth scenario that I'm having a devil of a time figuring a way around.

If you navigate back from a modal page, Shell replays your route parameters (calling ApplyQueryAttributes) from the last time your page was loaded. And then it calls it AGAIN with the no route parameters (same as scenario 3 above).

Please, please do not replay the page with the previous route parameters. Ideally, the page should never be held in memory, but I think I remember seeing another bug related to the fact that it isn't supposed to be.

I'm passing the route parameters because there is another issue where navigating back to a tab opens the subpage that was open when you moved to another shell page. So... Signin page to Main Tab page to Account tab to Logout page back to Signin page. Then SignIn Page to Main Tab page to Account tab. Only you don't get the Account tab, you get the Logout page.

The only way I've found around this is to navigate back to the account page and perform the logout there. But then if I navigate to the Logout page and then navigate back (instead of logging out), it resends the original route parameter and you log out again.

What I propose is:

  • ApplyQueryParameters is available for all navigation methods (including Shell pages in AppShell.xaml)
  • ApplyQueryParameters always gets called from all navigation methods (including Shell pages in AppShell.xaml).
  • It never replays old parameters.
  • It only passes in what you passed to navigation, whether that is a dictionary with entries in it or an empty dictionary. Null passes an empty dictionary. Making it nullable when it is not already would be a breaking change.
  • This isn't directly related, but it would be nice if there were an async version, but that could be tricky because the async version shouldn't wait for the method to complete.

Edit:

In writing this out, I think I understand why it replays when you return from a modal page. It's so you can restore the state you were in before you navigated to the modal page. The problem here is that it is from the last time the page was open, not the current time. If that makes any sense.

@Zhanglirong-Winnie Zhanglirong-Winnie added the s/triaged Issue has been reviewed label Feb 29, 2024
@jfversluis jfversluis modified the milestones: back, Backlog Feb 29, 2024
@ghost
Copy link

ghost commented Feb 29, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout platform/iOS 🍎 s/triaged Issue has been reviewed t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants