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: vue, non-tab routes getting added to tabs location history #24859

Closed
4 of 6 tasks
tigohenryschultz opened this issue Mar 1, 2022 · 12 comments · Fixed by #25045
Closed
4 of 6 tasks

bug: vue, non-tab routes getting added to tabs location history #24859

tigohenryschultz opened this issue Mar 1, 2022 · 12 comments · Fixed by #25045
Labels
package: vue @ionic/vue package type: bug a confirmed bug report

Comments

@tigohenryschultz
Copy link
Contributor

tigohenryschultz commented Mar 1, 2022

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

When navigating around and between multiple outlets the last tab will not properly transition away. Only the parent outlet is marked as ion-page-hidden but not all children within it.

This causes problems when navigating around as re-entering the 2nd outlet could cause it to render/display the wrong ion-page.

Expected Behavior

ion-page-hidden should be added when leaving

Steps to Reproduce

Navigate between two outlets and one outlets has a tabs, the last tab will still be visbile when navigating away to another outlet.

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 6.16.3 (C:\Users\Henry\AppData\Roaming\npm\node_modules@ionic\cli)
Ionic Framework : @ionic/vue 6.0.9

Capacitor:

Capacitor CLI : 3.4.1
@capacitor/android : 3.4.1
@capacitor/core : 3.4.1
@capacitor/ios : 3.4.1

Utility:

cordova-res : not installed globally
native-run : 1.5.0

System:

NodeJS : v14.17.3 (C:\Program Files\nodejs\node.exe)
npm : 6.14.13
OS : Windows 10

Additional Information

Red is the last tab still 'visible', blue is each outlet and green is the current route.

image

This was solved here: #24433

Solving this issue the way it was done in that pull request also fixes life cycle events for tabs, which currently do not work.

The way it was solved was allowing all ion-router-outlets handle page transitions. Current code only allows root outlet to handle transition which presents this bug.

When leaving from outlet 1 back to the root(0) it is unable to find this tab because it exists only on outlet 1.

As you can see in the below screenshot, going back to the tab will render the wrong page. It shows home is selected but it rendering the settings.

image

@ionitron-bot ionitron-bot bot added the triage label Mar 1, 2022
@aezur
Copy link

aezur commented Mar 2, 2022

Exact same issue here. If I log out and log back in, I am taken back to settings (where logout is) and the route shows I am on the home tab. If I continue to navigate around the app, things get... weird.

@liamdebeasi
Copy link
Contributor

Thanks for the issue. Can you provide a GitHub repo I can use to verify this behavior?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Mar 2, 2022
@ionitron-bot ionitron-bot bot removed the triage label Mar 2, 2022
@tigohenryschultz
Copy link
Contributor Author

tigohenryschultz commented Mar 2, 2022

Use your sample project in ionic-framework/packages/vue/test-app

  1. http://localhost:8100/
  2. Click Tabs
  3. Click Tab 2
  4. Click go to /routing (You will see tab2 does not have ion-page-hidden in the dom)
  5. Click go to /tabs/tab1 (tab1 triggered ion-page-hidden somehow, my app doesn't do this same thing)
  6. Click tab 2 again
  7. Click go to /routing
  8. Click back multiple times and it is stuck on tab 2(slightly different issue than mine, mine is because another router replaced to the previous tab, vs pushed the tab back on) I can explain the refactor I did to setupViewItems/IonRouterOutlet on discord if you have time that solved this in the other ticket.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Mar 2, 2022
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Mar 2, 2022

The culprit appears to be https://github.com/ionic-team/ionic-framework/blob/main/packages/vue-router/src/router.ts#L291.

When you are on step 7, the /routing route gets the tab2 tab assigned, so it goes into the Tab 2 navigation stack. So when you go back from /tabs/tab2, it tries to go to /routing (what it thinks is the previous route).

Tab 2 also has an ion-back-button with a default-href of /. So really, it should go back to / instead of /routing. Note that it should not go back to /tabs/tab1 because each tab is its own stack. Items in one tab stack do not interact with items in another tab stack.

edit: When leaving the outlet, the last tab should not be marked as hidden. The reason for this is we consider the active tab state to be local to the tabs outlet. In other words, while the tabs outlet itself is hidden, the tab within the outlet is still considered active.

@liamdebeasi liamdebeasi changed the title bug: The last tab never gets marked as 'ion-page-hidden' when leaving outlet bug: vue, non-tab routes getting added to tabs location history Mar 2, 2022
@liamdebeasi liamdebeasi added package: vue @ionic/vue package type: bug a confirmed bug report labels Mar 2, 2022
@ionitron-bot ionitron-bot bot removed the triage label Mar 2, 2022
@tigohenryschultz
Copy link
Contributor Author

The culprit appears to be https://github.com/ionic-team/ionic-framework/blob/main/packages/vue-router/src/router.ts#L291.

When you are on step 7, the /routing route gets the tab2 tab assigned, so it goes into the Tab 2 navigation stack. So when you go back from /tabs/tab2, it tries to go to /routing (what it thinks is the previous route).

Tab 2 also has an ion-back-button with a default-href of /. So really, it should go back to / instead of /routing. Note that it should not go back to /tabs/tab1 because each tab is its own stack. Items in one tab stack do not interact with items in another tab stack.

edit: When leaving the outlet, the last tab should not be marked as hidden. The reason for this is we consider the active tab state to be local to the tabs outlet. In other words, while the tabs outlet itself is hidden, the tab within the outlet is still considered active.

If the last tab in that outlet is not marked as hidden, do the life cycles still fire for it as expected?

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Mar 7, 2022

No, but this is a behavior we would like to change. The tab lifecycle behavior is inconsistent across the Angular, React, and Vue integrations. Both Angular and Vue do not fire lifecycles on the active tab when the tab context leaves. React does fire lifecycles on the active tab when the tab context leaves, but that is more of a coincidence rather than an intentional behavior.


The problem stems from differing definitions of what an "active view" is. In the early days of Ionic, the team defined an "active view" as the top-most page that was matched by the router. In this case, that would be the Tabs component (the component that renders <ion-tabs>). The individual Tab component would not be considered the active view.

After having seen many apps over the past few years, our definition has broadened to essentially be "any component that is wrapped in an <ion-page> and is matched by the router". In other words, multiple active views are possible. Under this new definition, both the Tabs component and the individual Tab component would be considered active views and therefore, should both receive lifecycle hooks.

@tigohenryschultz
Copy link
Contributor Author

Yeah the multi-outlet/nested-outlet is a quite complex problem. Took me a good two weeks to wrap my head around it to understand how to solve my issues. I ended up letting all outlets handle route changes but transitions could be triggered multiple times.

Wonder if it makes sense to just make an ionic-router.

@liamdebeasi
Copy link
Contributor

Yeah that's more or less what we plan to do. We are looking to extract all of the Ionic-specific routing logic into a single package that can be easily reused across integrations.

We want to enable developers to still use their preferred router (React Router, Vue Router, etc), but the "under the hood" logic is going to be consolidated. We're also looking to better document routing patterns for some of the more complex scenarios like tabs. For example, there are patterns in web tabs that are anti-patterns when matching native mobile tabs. (such as content in Tab A should never route users to Tab B because each tab is its own stack)

@tigohenryschultz
Copy link
Contributor Author

tigohenryschultz commented Mar 7, 2022

Yeah that's more or less what we plan to do. We are looking to extract all of the Ionic-specific routing logic into a single package that can be easily reused across integrations.

We want to enable developers to still use their preferred router (React Router, Vue Router, etc), but the "under the hood" logic is going to be consolidated. We're also looking to better document routing patterns for some of the more complex scenarios like tabs. For example, there are patterns in web tabs that are anti-patterns when matching native mobile tabs. (such as content in Tab A should never route users to Tab B because each tab is its own stack)

If a user did add a button in tab a that pointed to tab b I would imagine it would still attempt to handle it properly instead of just throwing an error. I see how that could be considered wrong but if the underlying routing logic was simple enough then it should still just work. tab a would be transitioned out, tab b transitioned in, clicking back would go to tab a again

The refactor I thought about was:
Instead of multiple ion-router-outlets you could have just one. The purpose of the outlet would be to watch the route url/params/etc and determine which view was transitioning in/out. To replace the rendering side of the outlet you could create a new wrapper called ion-render-outlet and separate some of their responsibilities.

Then you could add multiple render outlets that would handle hiding/showing their content, life cycles. Tabs would be their own render outlet and the outer router-outlet would just determine which render outlet/view you are leaving, render outlet/view you are entering and then the render outlet can handle the rest.

All the scenarios really need to be white-boarded out too to see where holes arise. I'm sure theres a holy grail solution out there somewhere.

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Apr 1, 2022

@tigohenryschultz Can you give this dev build a try and let me know if it resolves the issue?

6.0.15-dev.11648833150.1f2a9365

edit: I updated the dev build. The one ending in 9365 is the correct one to use.

@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #25045, and a fix will be available in an upcoming release of Ionic Framework.

@ionitron-bot
Copy link

ionitron-bot bot commented May 12, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators May 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: vue @ionic/vue package type: bug a confirmed bug report
Projects
None yet
3 participants