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

fix(vue): navigating between parameterized pages now results in page transition #23525

Merged
merged 2 commits into from
Jun 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 15 additions & 25 deletions packages/vue-router/src/viewStacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,7 @@ export const createViewStacks = (router: Router) => {
}

const findViewItemByRouteInfo = (routeInfo: RouteInfo, outletId?: number) => {
let viewItem = findViewItemByPath(routeInfo.pathname, outletId, false);

/**
* Given a route such as /path/:id,
* going from /page/1 to /home
* to /page/2 will cause the same
* view item from /page/1 to match
* for /page/2 so we need to make
* sure any params get updated.
* Not normally an issue for accessing
* the params via useRouter from vue-router,
* but when passing params as props not doing
* this would cause the old props to show up.
*/
if (viewItem && viewItem.params !== routeInfo.params) {
/**
* Clear the props function result
* as the value may have changed due
* to different props.
*/
delete viewItem.vueComponentData.propsFunctionResult;
viewItem.params = routeInfo.params;
}

return viewItem;
return findViewItemByPath(routeInfo.pathname, outletId, false);
}

const findLeavingViewItemByRouteInfo = (routeInfo: RouteInfo, outletId?: number, mustBeIonRoute: boolean = true) => {
Expand Down Expand Up @@ -81,6 +57,20 @@ export const createViewStacks = (router: Router) => {
const findMatchedRoute = resolvedPath.matched.find((matchedRoute: RouteLocationMatched) => matchedRoute === viewItem.matchedRoute);

if (findMatchedRoute) {

/**
* /page/1 and /page/2 should not match
* to the same view item otherwise there will
* be not page transition and we will need to
* explicitly clear out parameters from page 1
* so the page 2 params are properly passed
* to the developer's app.
*/
const hasParameter = findMatchedRoute.path.includes(':');
if (hasParameter && path !== viewItem.pathname) {
return false;
}

return viewItem;
}

Expand Down
9 changes: 3 additions & 6 deletions packages/vue/test-app/src/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,9 @@ const routes: Array<RouteRecordRaw> = [
component: () => import('@/views/Tab1.vue'),
},
{
path: 'tab1/child-one',
component: () => import('@/views/Tab1ChildOne.vue')
},
{
path: 'tab1/child-two',
component: () => import('@/views/Tab1ChildTwo.vue')
path: 'tab1/:id',
component: () => import('@/views/Tab1Parameter.vue'),
props: true
},
{
path: 'tab2',
Expand Down
2 changes: 1 addition & 1 deletion packages/vue/test-app/src/views/RoutingParameter.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<ion-page data-pageid="routingparameter">
<ion-page :data-pageid="'routingparameter-' + $props.id">
<ion-header :translucent="true">
<ion-toolbar>
<ion-buttons>
Expand Down
2 changes: 1 addition & 1 deletion packages/vue/test-app/src/views/Tab1.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

<ExploreContainer name="Tab 1 page" />

<ion-item button router-link="/tabs/tab1/child-one" id="child-one">
<ion-item button router-link="/tabs/tab1/childone" id="child-one">
<ion-label>Go to Tab 1 Child 1</ion-label>
</ion-item>
<ion-item button router-link="/nested" id="nested">
Expand Down
44 changes: 0 additions & 44 deletions packages/vue/test-app/src/views/Tab1ChildTwo.vue

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
<template>
<ion-page data-pageid="tab1childone">
<ion-page :data-pageid="'tab1' + $props.id">
<ion-header>
<ion-toolbar>
<ion-buttons>
<ion-back-button></ion-back-button>
</ion-buttons>
<ion-title>Tab 1 Child 1</ion-title>
<ion-title>Tab 1 Child {{ $props.id }}</ion-title>
</ion-toolbar>
</ion-header>
<ion-content :fullscreen="true">
<ion-header collapse="condense">
<ion-toolbar>
<ion-title size="large">Tab 1 Child 1</ion-title>
<ion-title size="large">Tab 1 Child {{ $props.id }}</ion-title>
</ion-toolbar>
</ion-header>

<ion-item router-link="child-two" id="child-two">
<ion-item router-link="childone" id="child-one">
<ion-label>Tab 1 Child 1</ion-label>
</ion-item>

<ion-item router-link="childtwo" id="child-two">
<ion-label>Tab 1 Child 2</ion-label>
</ion-item>

Expand All @@ -37,6 +41,7 @@ import {
} from '@ionic/vue';

export default {
props: { id: String },
components: {
IonButtons,
IonBackButton,
Expand Down
46 changes: 32 additions & 14 deletions packages/vue/test-app/tests/e2e/specs/routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,23 @@ describe('Routing', () => {
cy.visit('http://localhost:8080/routing');

cy.get('#parameter-abc').click();
cy.ionPageVisible('routingparameter');
cy.get('[data-pageid=routingparameter] #parameter-value').should('have.text', 'abc');
cy.ionBackClick('routingparameter');
cy.ionPageVisible('routingparameter-abc');
cy.get('[data-pageid=routingparameter-abc] #parameter-value').should('have.text', 'abc');
cy.ionBackClick('routingparameter-abc');

cy.ionPageDoesNotExist('routingparameter');
cy.ionPageDoesNotExist('routingparameter-abc');

cy.get('#parameter-xyz').click();
cy.ionPageVisible('routingparameter');
cy.get('[data-pageid=routingparameter] #parameter-value').should('have.text', 'xyz');
cy.ionPageVisible('routingparameter-xyz');
cy.get('[data-pageid=routingparameter-xyz] #parameter-value').should('have.text', 'xyz');
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/22359
it('should handle parameterized urls properly', () => {
cy.visit('http://localhost:8080/routing');

cy.get('#parameter-abc').click();
cy.ionPageVisible('routingparameter');
cy.ionPageVisible('routingparameter-abc');

cy.get('#parameter-view').click();

Expand Down Expand Up @@ -149,32 +149,32 @@ describe('Routing', () => {
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/22658
it('should select correct leaving view when navigating between paramter urls', () => {
it('should select correct leaving view when navigating between parameter urls', () => {
cy.visit('http://localhost:8080');

cy.routerPush('/routing/123');
cy.ionPageVisible('routingparameter');
cy.ionPageVisible('routingparameter-123');
cy.ionPageHidden('home');

cy.routerPush('/routing/456');
cy.ionPageVisible('routingparameter');
cy.ionPageVisible('routingparameter-456');
cy.ionPageHidden('home');

cy.routerPush('/navigation');
cy.ionPageVisible('navigation');
cy.ionPageHidden('routingparameter');
cy.ionPageHidden('routingparameter-456');

cy.routerPush('/routing/789');
cy.ionPageVisible('routingparameter');
cy.ionPageVisible('routingparameter-789');
cy.ionPageHidden('home');

cy.routerPush('/routing/000');
cy.ionPageVisible('routingparameter');
cy.ionPageVisible('routingparameter-000');
cy.ionPageHidden('home');

cy.routerPush('/navigation');
cy.ionPageVisible('navigation');
cy.ionPageHidden('routingparameter');
cy.ionPageHidden('routingparameter-000');
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/22528
Expand All @@ -191,6 +191,24 @@ describe('Routing', () => {

cy.ionBackButtonHidden('home');
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/22662
it('should push a new instance of a parameterized page so there is a transition', () => {
cy.visit('http://localhost:8080');

cy.routerPush('/routing/123');
cy.ionPageVisible('routingparameter-123');
cy.ionPageHidden('home');

cy.routerPush('/routing/456');
cy.ionPageVisible('routingparameter-456');
cy.ionPageHidden('routingparameter-123');

cy.ionBackClick('routingparameter-456');

cy.ionPageVisible('routingparameter-123')
cy.ionPageDoesNotExist('routingparameter-456');
})
});

describe('Routing - Swipe to Go Back', () => {
Expand Down
1 change: 0 additions & 1 deletion packages/vue/test-app/tests/e2e/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ Cypress.Commands.add('ionPageVisible', (pageId) => {
Cypress.Commands.add('ionPageHidden', (pageId) => {
cy.get(`div.ion-page[data-pageid=${pageId}]`)
.should('have.class', 'ion-page-hidden')
.should('have.length', 1)
})

Cypress.Commands.add('ionBackClick', (pageId) => {
Expand Down
4 changes: 3 additions & 1 deletion packages/vue/test-app/tests/unit/routing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,8 @@ describe('Routing', () => {
router.push('/page/2');
await waitForRouter();

expect(page.props()).toEqual({ id: '2' });
const pageAgain = wrapper.findAllComponents(Page);
expect(pageAgain[0].props()).toEqual({ id: '1' });
expect(pageAgain[1].props()).toEqual({ id: '2' });
});
});