Skip to content

Commit

Permalink
fix(vue): replacing routes now updates location state correctly (#24721)
Browse files Browse the repository at this point in the history
resolves #24432

Co-authored-by: tigohenryschultz <tigohenryschultz@users.noreply.github.com>
Co-authored-by: yoyo930021 <yoyo930021@users.noreply.github.com>
  • Loading branch information
3 people committed Feb 7, 2022
1 parent 8c22646 commit 721a461
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 13 deletions.
5 changes: 1 addition & 4 deletions packages/vue-router/__tests__/locationHistory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,12 @@ describe('Location History', () => {
expect(first.pathname).toEqual('/tabs/tab1/child/1');
});

it('should correctly get current and previous routes', () => {
it('should correctly get last route', () => {
locationHistory.add({ pathname: '/home' });
locationHistory.add({ pathname: '/login' });

const current = locationHistory.last();
expect(current.pathname).toEqual('/login');

const previous = locationHistory.previous();
expect(previous.pathname).toEqual('/home');
});

it('should correctly determine if we can go back', () => {
Expand Down
2 changes: 0 additions & 2 deletions packages/vue-router/src/locationHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ export const createLocationHistory = () => {
const index = currentHistory - initialHistory;
return locationHistory[index] || last();
}
const previous = () => locationHistory[locationHistory.length - 2] || last();
const last = () => locationHistory[locationHistory.length - 1];

/**
Expand Down Expand Up @@ -210,7 +209,6 @@ export const createLocationHistory = () => {
current,
size,
last,
previous,
add,
canGoBack,
update,
Expand Down
26 changes: 23 additions & 3 deletions packages/vue-router/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,15 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
) => {
let leavingLocationInfo: RouteInfo;
if (incomingRouteParams) {

/**
* If we are replacing the state of a route
* with another route, the "leaving" route
* is at the same position in location history
* as where the replaced route will exist.
*/
if (incomingRouteParams.routerAction === 'replace') {
leavingLocationInfo = locationHistory.previous();
leavingLocationInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition);
} else if (incomingRouteParams.routerAction === 'pop') {
leavingLocationInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition + 1);

Expand Down Expand Up @@ -334,8 +341,18 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
* item for this route. In other words, a user
* is navigating within the history without pushing
* new items within the stack.
*
* If the historySize === historyDiff,
* then we are still re-writing history
* by replacing the current route state
* with a new route state. The initial
* action when loading an app is
* going to be replace operation, so
* we want to make sure we exclude that
* action by ensuring historySize > 0.
*/
if (historySize > historyDiff && routeInfo.tab === undefined) {
const isReplacing = historySize === historyDiff && historySize > 0 && action === 'replace';
if (historySize > historyDiff || isReplacing) {
/**
* When navigating back through the history,
* if users then push a new route the future
Expand All @@ -354,7 +371,10 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
* in other scenarios.
*/

if (routeInfo.routerAction === 'push' && delta === undefined) {
if (
(routeInfo.routerAction === 'push' || routeInfo.routerAction === 'replace') &&
delta === undefined
) {
locationHistory.clearHistory(routeInfo);
locationHistory.add(routeInfo);
}
Expand Down
6 changes: 2 additions & 4 deletions packages/vue/test-app/tests/e2e/specs/routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ describe('Routing', () => {
cy.visit('http://localhost:8080');
cy.get('ion-item#routing').click();

cy.wait(500)

cy.ionPageVisible('routing')
cy.ionPageHidden('home')
cy.ionPageVisible('routing');
cy.ionPageHidden('home');
});

it('should set query params and keep view in stack', () => {
Expand Down
34 changes: 34 additions & 0 deletions packages/vue/test-app/tests/e2e/specs/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,40 @@ describe('Tabs', () => {
cy.ionPageVisible('tab1childtwo');
cy.ionPageVisible('tabs');
})

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24432
it('should properly reset location history when switching tabs after going back', () => {
cy.visit('http://localhost:8080/tabs');

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

cy.ionRouterBack();
cy.ionPageVisible('tab1');
cy.ionPageDoesNotExist('tab1childone');

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

cy.ionRouterBack();
cy.ionPageVisible('tab1');
cy.ionPageDoesNotExist('tab2');
});

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

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

cy.ionRouterReplace('/tabs/tab1');
cy.ionPageVisible('tab1');
cy.ionPageDoesNotExist('tab1childone');
})
})

describe('Tabs - Swipe to Go Back', () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/vue/test-app/tests/e2e/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ Cypress.Commands.add('ionRouterNavigate', (...args) => {
});
});

Cypress.Commands.add('ionRouterBack', () => {
cy.window().then(win => {
win.debugIonRouter.back();
});
});

Cypress.Commands.add('ionRouterReplace', (path) => {
cy.window().then(win => {
win.debugIonRouter.replace(path);
});
});

Cypress.Commands.add('ionBackButtonHidden', (pageId) => {
cy.get(`div.ion-page[data-pageid=${pageId}]`)
.should('be.visible', true)
Expand Down

0 comments on commit 721a461

Please sign in to comment.