Skip to content

fix(nav): pop() will unmount views within a modal #25638

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 4 commits into from
Jul 25, 2022
Merged

fix(nav): pop() will unmount views within a modal #25638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a659b7
fix(nav): pop unmount views within a modal
sean-perkins Jul 14, 2022
8a4e4d1
test(nav): use toHaveText
sean-perkins Jul 19, 2022
b6c9325
Merge branch 'main' into FW-1873
sean-perkins Jul 19, 2022
de2226e
Merge branch 'main' into FW-1873
sean-perkins Jul 21, 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
20 changes: 16 additions & 4 deletions core/src/components/nav/nav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ export class Nav implements NavOutlet {
this.swipeGestureChanged();
}

connectedCallback() {
this.destroyed = false;
}

disconnectedCallback() {
for (const view of this.views) {
lifecycle(view.element!, LIFECYCLE_WILL_UNLOAD);
Expand Down Expand Up @@ -879,9 +883,13 @@ export class Nav implements NavOutlet {
leavingView: ViewController | undefined,
opts: NavOptions
): NavResult {
const cleanupView = hasCompleted ? enteringView : leavingView;
if (cleanupView) {
this.cleanup(cleanupView);
/**
* If the transition did not complete, the leavingView will still be the active
* view on the stack. Otherwise unmount all the views after the enteringView.
*/
const activeView = hasCompleted ? enteringView : leavingView;
if (activeView) {
this.unmountInactiveViews(activeView);
}

return {
Expand Down Expand Up @@ -944,9 +952,13 @@ export class Nav implements NavOutlet {
}

/**
* Unmounts all inactive views after the specified active view.
*
* DOM WRITE
*
* @param activeView The view that is actively visible in the stack. Used to calculate which views to unmount.
*/
private cleanup(activeView: ViewController) {
private unmountInactiveViews(activeView: ViewController) {
// ok, cleanup time!! Destroy all of the views that are
// INACTIVE and come after the active view
// only do this if the views exist, though
Expand Down
103 changes: 103 additions & 0 deletions core/src/components/nav/test/modal-navigation/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<!DOCTYPE html>
<html lang="en" dir="ltr">
<head>
<meta charset="UTF-8" />
<title>Nav - Modal Navigation</title>
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
/>
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
<script src="../../../../../scripts/testing/scripts.js"></script>
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
</head>

<body>
<ion-app>
<ion-header>
<ion-toolbar>
<ion-title>Modal Navigation</ion-title>
</ion-toolbar>
</ion-header>
<ion-content>
<ion-button id="openModal">Open Modal</ion-button>
<ion-modal trigger="openModal">
<ion-header>
<ion-toolbar>
<ion-title>Modal</ion-title>
<ion-buttons slot="end">
<ion-button onclick="dismiss()"> Close </ion-button>
</ion-buttons>
</ion-toolbar>
</ion-header>
<ion-content>
<ion-nav></ion-nav>
</ion-content>
</ion-modal>
</ion-content>
</ion-app>

<script>
const modal = document.querySelector('ion-modal');
const nav = document.querySelector('ion-nav');

modal.addEventListener('willPresent', () => {
nav.setRoot('page-one');
});

const dismiss = () => modal.dismiss();

const navigate = (component, componentProps) => {
nav.push(component, componentProps);
};

const navigateBack = () => {
nav.pop();
};

const navigateToRoot = () => {
nav.popToRoot();
};

class PageOne extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<ion-content class="ion-padding">
<h1>Page One</h1>
<ion-button id="goto-page-two" onclick="navigate('page-two')">Go to Page Two</ion-button>
</ion-content>
`;
}
}

class PageTwo extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<ion-content class="ion-padding">
<h1>Page Two</h1>
<ion-button id="goto-page-three" onclick="navigate('page-three')">Go to Page Three</ion-button>
</ion-content>
`;
}
}

class PageThree extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<ion-content class="ion-padding">
<h1>Page Three</h1>
<ion-button id="go-back" onclick="navigateBack()">Go Back</ion-button>
<ion-button id="goto-root"onclick="navigateToRoot()">Go to Root</ion-button>
</ion-content>
`;
}
}

customElements.define('page-one', PageOne);
customElements.define('page-two', PageTwo);
customElements.define('page-three', PageThree);
</script>
</body>
</html>
77 changes: 77 additions & 0 deletions core/src/components/nav/test/modal-navigation/nav.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { expect } from '@playwright/test';
import type { E2EPage } from '@utils/test/playwright';
import { test } from '@utils/test/playwright';

test.describe('nav: modal-navigation', () => {
test.beforeEach(async ({ page }) => {
await page.goto(`/src/components/nav/test/modal-navigation`);
await openModal(page);
});

test('should render the root page', async ({ page }) => {
const pageOne = page.locator('page-one');
const pageOneHeading = page.locator('page-one h1');

await expect(pageOne).toBeVisible();
await expect(pageOneHeading).toHaveText('Page One');
});

test('should push to the next page', async ({ page }) => {
await page.click('#goto-page-two');

const pageTwo = page.locator('page-two');
const pageTwoHeading = page.locator('page-two h1');

await expect(pageTwo).toBeVisible();
await expect(pageTwoHeading).toHaveText('Page Two');
});

test('should pop to the previous page', async ({ page }) => {
await page.click('#goto-page-two');
await page.click('#goto-page-three');

const pageThree = page.locator('page-three');
const pageThreeHeading = page.locator('page-three h1');

await expect(pageThree).toBeVisible();
await expect(pageThreeHeading).toHaveText('Page Three');

await page.click('#go-back');

const pageTwo = page.locator('page-two');
const pageTwoHeading = page.locator('page-two h1');

// Verifies the leavingView was unmounted
await expect(pageThree).toHaveCount(0);
await expect(pageTwo).toBeVisible();
await expect(pageTwoHeading).toHaveText('Page Two');
});

test.describe('popping to the root', () => {
test('should render the root page', async ({ page }) => {
const pageTwo = page.locator('page-two');
const pageThree = page.locator('page-three');

await page.click('#goto-page-two');
await page.click('#goto-page-three');

await page.click('#goto-root');

const pageOne = page.locator('page-one');
const pageOneHeading = page.locator('page-one h1');

// Verifies all views besides the root were unmounted
await expect(pageTwo).toHaveCount(0);
await expect(pageThree).toHaveCount(0);

await expect(pageOne).toBeVisible();
await expect(pageOneHeading).toHaveText('Page One');
});
});
});

const openModal = async (page: E2EPage) => {
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
await page.click('#openModal');
await ionModalDidPresent.next();
};