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.

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

fix(vue): ensure that only tab pages get added to the tab navigation stack #25045

Merged
merged 7 commits into from
Apr 12, 2022
Merged

fix(vue): ensure that only tab pages get added to the tab navigation stack #25045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fc00d80
test(vue): add failing test
liamdebeasi Apr 1, 2022
a89ddab
fix(vue): ensure that only IonTabs determines if a route is in a tab
liamdebeasi Apr 1, 2022
ba6d75f
fix(vue): remove a few more instances where tab we being set
liamdebeasi Apr 1, 2022
f2a9365
test(): remove redundant test
liamdebeasi Apr 1, 2022
28ba8d6
chore(): clean up
liamdebeasi Apr 1, 2022
7eb163e
Merge branch 'main' into FW-907
liamdebeasi Apr 5, 2022
5e46098
Merge branch 'main' into FW-907
liamdebeasi Apr 12, 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
25 changes: 15 additions & 10 deletions packages/vue-router/src/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>

let currentRouteInfo: RouteInfo;
let incomingRouteParams: RouteParams;
let currentTab: string | undefined;

// TODO types
let historyChangeListeners: any[] = [];
Expand Down Expand Up @@ -238,8 +237,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
if (action === 'replace') {
incomingRouteParams = {
routerAction: 'replace',
routerDirection: 'none',
tab: currentTab
routerDirection: 'none'
}
} else if (action === 'pop') {
const routeInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition - delta);
Expand All @@ -254,16 +252,15 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
} else {
incomingRouteParams = {
routerAction: 'pop',
routerDirection: 'none',
tab: currentTab
routerDirection: 'none'
}
}
}

if (!incomingRouteParams) {
incomingRouteParams = {
routerAction: 'push',
routerDirection: direction || 'forward',
tab: currentTab
routerDirection: direction || 'forward'
}
}
}
Expand All @@ -288,7 +285,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
}

if (isPushed) {
routeInfo.tab = leavingLocationInfo.tab;
routeInfo.pushedByRoute = (leavingLocationInfo.pathname !== '') ? leavingLocationInfo.pathname : undefined;
} else if (routeInfo.routerAction === 'pop') {
const route = locationHistory.findLastLocation(routeInfo);
Expand Down Expand Up @@ -460,9 +456,18 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
}
}

/**
* This method is invoked by the IonTabs component
* during a history change callback. It is responsible
* for ensuring that tabbed routes have the correct
* "tab" field in its routeInfo object.
*
* IonTabs will determine if the current route
* is in tabs and assign it the correct tab.
* If the current route is not in tabs,
* then IonTabs will not invoke this.
*/
const handleSetCurrentTab = (tab: string) => {
currentTab = tab;

const ri = { ...locationHistory.last() };
if (ri.tab !== tab) {
ri.tab = tab;
Expand Down
32 changes: 31 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 @@ -435,7 +435,7 @@ describe('Tabs', () => {
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24432
it('should correct replace a route in a child tab route', () => {
it('should correctly replace a route in a child tab route', () => {
cy.visit('http://localhost:8080/tabs');

cy.routerPush('/tabs/tab1/childone');
Expand All @@ -446,6 +446,36 @@ describe('Tabs', () => {
cy.ionPageVisible('tab1');
cy.ionPageDoesNotExist('tab1childone');
})

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24859
it('should go back to the root page after navigating between tab and non tab outlets', () => {
cy.visit('http://localhost:8080');

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

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

cy.routerPush('/routing');
cy.ionPageVisible('routing');
cy.ionPageHidden('tabs');

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

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

cy.ionBackClick('tab2');
cy.ionPageVisible('home');
cy.ionPageDoesNotExist('tabs');
});
})

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