Skip to content

fix(vue): correct views are now unmounted in tabs #25270

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 18, 2022
Merged

fix(vue): correct views are now unmounted in tabs #25270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
47e2336
test(vue): add failing test
liamdebeasi May 10, 2022
b5b1633
fix(vue): correct views are now unmounted in tabs apps
liamdebeasi May 10, 2022
ba60afe
test(): add todo for other test
liamdebeasi May 10, 2022
4ac58d6
Merge branch 'main' into FW-1487
liamdebeasi May 10, 2022
592f749
Merge branch 'main' into FW-1487
liamdebeasi May 16, 2022
bcb63f8
Merge branch 'main' into FW-1487
liamdebeasi May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/vue-router/src/viewStacks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ import { shallowRef } from 'vue';
export const createViewStacks = (router: Router) => {
let viewStacks: ViewStacks = {};

/**
* Returns the number of active stacks.
* This is useful for determining if an app
* is using linear navigation only or non-linear
* navigation. Multiple stacks indiciate an app
* is using non-linear navigation.
*/
const size = () => Object.keys(viewStacks).length;

const clear = (outletId: number) => {
delete viewStacks[outletId];
}
Expand Down Expand Up @@ -211,6 +220,7 @@ export const createViewStacks = (router: Router) => {
add,
remove,
registerIonPage,
getViewStack
getViewStack,
size
}
}
15 changes: 13 additions & 2 deletions packages/vue/src/components/IonRouterOutlet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ See https://ionicframework.com/docs/vue/navigation#ionpage for more information.
leavingEl.classList.add('ion-page-hidden');
leavingEl.setAttribute('aria-hidden', 'true');

const usingLinearNavigation = viewStacks.size() === 1;

if (routerAction === 'replace') {
leavingViewItem.mount = false;
leavingViewItem.ionPageElement = undefined;
Expand All @@ -327,9 +329,18 @@ See https://ionicframework.com/docs/vue/navigation#ionpage for more information.
leavingViewItem.mount = false;
leavingViewItem.ionPageElement = undefined;
leavingViewItem.ionRoute = false;
viewStacks.unmountLeavingViews(id, enteringViewItem, delta);

/**
* router.go() expects navigation to be
* linear. If an app is using multiple stacks then
* it is not using linear navigation. As a result, router.go()
* will not give the results that developers are expecting.
*/
if (usingLinearNavigation) {
viewStacks.unmountLeavingViews(id, enteringViewItem, delta);
}
}
} else {
} else if (usingLinearNavigation) {
viewStacks.mountIntermediaryViews(id, leavingViewItem, delta);
}

Expand Down
48 changes: 47 additions & 1 deletion packages/vue/test-app/tests/e2e/specs/tabs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ describe('Tabs', () => {

cy.ionPageVisible('tab1');

cy.ionPageDoesNotExist('tab1childone');
// TODO(FW-1420)
//cy.ionPageDoesNotExist('tab1childone');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still an issue which is why I am commenting it out. It was previously working by chance and I mistakenly thought it was fixed so I uncommented this line.

cy.ionPageDoesNotExist('tab1childtwo');
})

Expand Down Expand Up @@ -534,6 +535,51 @@ describe('Tabs', () => {
cy.ionPageVisible('tab2');
cy.ionPageHidden('tab1');
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/25255
it('should not error when going back to root tab multiple times', () => {
cy.visit('http://localhost:8080/tabs');

cy.routerPush('/tabs/tab1/childone');
cy.ionPageVisible('tab1childone');
cy.ionPageHidden('tab1');

cy.get('ion-tab-button#tab-button-tab2').click();
cy.ionPageHidden('tab1childone');
cy.ionPageVisible('tab2');

cy.get('ion-tab-button#tab-button-tab1').click();
cy.ionPageHidden('tab2');
cy.ionPageVisible('tab1childone');

cy.get('ion-tab-button#tab-button-tab1').click();
cy.ionPageDoesNotExist('tab1childone');
cy.ionPageVisible('tab1');

cy.get('ion-tab-button#tab-button-tab2').click();
cy.ionPageHidden('tab1');
cy.ionPageVisible('tab2');

cy.get('ion-tab-button#tab-button-tab1').click();
cy.ionPageHidden('tab2');
cy.ionPageVisible('tab1');

cy.routerPush('/tabs/tab1/childone');
cy.ionPageVisible('tab1childone');
cy.ionPageHidden('tab1');

cy.get('ion-tab-button#tab-button-tab2').click();
cy.ionPageHidden('tab1childone');
cy.ionPageVisible('tab2');

cy.get('ion-tab-button#tab-button-tab1').click();
cy.ionPageHidden('tab2');
cy.ionPageVisible('tab1childone');

cy.get('ion-tab-button#tab-button-tab1').click();
cy.ionPageDoesNotExist('tab1childone');
cy.ionPageVisible('tab1');
})
})

describe('Tabs - Swipe to Go Back', () => {
Expand Down