Skip to content

Commit a63afa3

Browse files
ShaneKShaneK
committed
fix(modal): addressing edge cases, cleaning up
1 parent 26b6b7b commit a63afa3

File tree

7 files changed

+491
-65
lines changed

7 files changed

+491
-65
lines changed

‎core/src/components/content/content.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,13 @@ export class Content implements ComponentInterface {
203203
const parent = this.el.parentElement;
204204
if (parent && !this.parentMutationObserver && win !== undefined && 'MutationObserver' in win) {
205205
this.parentMutationObserver = new MutationObserver(() => {
206+
const prevHasHeader = this.hasHeader;
207+
const prevHasFooter = this.hasFooter;
206208
this.updateSiblingDetection();
207-
forceUpdate(this);
209+
// Only trigger re-render if header/footer detection actually changed
210+
if (prevHasHeader !== this.hasHeader || prevHasFooter !== this.hasFooter) {
211+
forceUpdate(this);
212+
}
208213
});
209214
this.parentMutationObserver.observe(parent, { childList: true });
210215
}

‎core/src/components/content/test/safe-area/content.e2e.ts‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
/**
5+
* Safe-area tests verify that ion-content correctly applies safe-area classes
6+
* based on the presence/absence of sibling ion-header and ion-footer elements.
7+
*
8+
* These tests verify the FW-6830 feature: automatic safe-area handling for content.
9+
*/
10+
11+
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
12+
test.describe(title('content: safe-area'), () => {
13+
test.beforeEach(async ({ page }) => {
14+
await page.goto('/src/components/content/test/safe-area', config);
15+
});
16+
17+
test('content without header should have safe-area-top class', async ({ page }, testInfo) => {
18+
testInfo.annotations.push({
19+
type: 'issue',
20+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
21+
});
22+
23+
const content = page.locator('#content-no-header');
24+
await expect(content).toHaveClass(/safe-area-top/);
25+
await expect(content).not.toHaveClass(/safe-area-bottom/);
26+
});
27+
28+
test('content without footer should have safe-area-bottom class', async ({ page }, testInfo) => {
29+
testInfo.annotations.push({
30+
type: 'issue',
31+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
32+
});
33+
34+
const content = page.locator('#content-no-footer');
35+
await expect(content).not.toHaveClass(/safe-area-top/);
36+
await expect(content).toHaveClass(/safe-area-bottom/);
37+
});
38+
39+
test('content with both header and footer should not have safe-area classes', async ({ page }, testInfo) => {
40+
testInfo.annotations.push({
41+
type: 'issue',
42+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
43+
});
44+
45+
const content = page.locator('#content-with-both');
46+
await expect(content).not.toHaveClass(/safe-area-top/);
47+
await expect(content).not.toHaveClass(/safe-area-bottom/);
48+
});
49+
50+
test('content without header or footer should have both safe-area classes', async ({ page }, testInfo) => {
51+
testInfo.annotations.push({
52+
type: 'issue',
53+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
54+
});
55+
56+
const content = page.locator('#content-no-both');
57+
await expect(content).toHaveClass(/safe-area-top/);
58+
await expect(content).toHaveClass(/safe-area-bottom/);
59+
});
60+
61+
test('content with wrapped header should not have safe-area-top class', async ({ page }, testInfo) => {
62+
testInfo.annotations.push({
63+
type: 'issue',
64+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
65+
});
66+
67+
const content = page.locator('#content-wrapped-header');
68+
// Wrapped header detection should find the ion-header inside my-header
69+
await expect(content).not.toHaveClass(/safe-area-top/);
70+
});
71+
72+
test('content with wrapped footer should not have safe-area-bottom class', async ({ page }, testInfo) => {
73+
testInfo.annotations.push({
74+
type: 'issue',
75+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
76+
});
77+
78+
const content = page.locator('#content-wrapped-footer');
79+
// Wrapped footer detection should find the ion-footer inside my-footer
80+
await expect(content).not.toHaveClass(/safe-area-bottom/);
81+
});
82+
83+
test('nested content should not have safe-area classes', async ({ page }, testInfo) => {
84+
testInfo.annotations.push({
85+
type: 'issue',
86+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
87+
});
88+
89+
const nestedContent = page.locator('#content-nested');
90+
// Nested content should not be treated as main content
91+
await expect(nestedContent).not.toHaveClass(/safe-area-top/);
92+
await expect(nestedContent).not.toHaveClass(/safe-area-bottom/);
93+
});
94+
95+
test('outer content should still have safe-area classes', async ({ page }, testInfo) => {
96+
testInfo.annotations.push({
97+
type: 'issue',
98+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
99+
});
100+
101+
const outerContent = page.locator('#content-outer');
102+
// Outer content has no sibling header/footer, so it should have safe-area classes
103+
await expect(outerContent).toHaveClass(/safe-area-top/);
104+
await expect(outerContent).toHaveClass(/safe-area-bottom/);
105+
});
106+
107+
test('content inside modal should not have safe-area classes', async ({ page }, testInfo) => {
108+
testInfo.annotations.push({
109+
type: 'issue',
110+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
111+
});
112+
113+
// Open the modal
114+
await page.evaluate(() => {
115+
const modal = document.getElementById('test-modal') as HTMLIonModalElement;
116+
modal.isOpen = true;
117+
});
118+
119+
// Wait for modal to be presented
120+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
121+
await ionModalDidPresent.next();
122+
123+
const modalContent = page.locator('#content-in-modal');
124+
// Content inside modal should not be treated as main content
125+
await expect(modalContent).not.toHaveClass(/safe-area-top/);
126+
await expect(modalContent).not.toHaveClass(/safe-area-bottom/);
127+
});
128+
129+
test('dynamic header addition should update safe-area classes', async ({ page }, testInfo) => {
130+
testInfo.annotations.push({
131+
type: 'issue',
132+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
133+
});
134+
135+
const content = page.locator('#content-dynamic');
136+
137+
// Initially should have safe-area-top (no header)
138+
await expect(content).toHaveClass(/safe-area-top/);
139+
140+
// Add header dynamically
141+
await page.click('#add-header-btn');
142+
143+
// Wait for mutation observer to trigger and component to update
144+
// Using expect with timeout instead of waitForTimeout for reliability
145+
await expect(content).not.toHaveClass(/safe-area-top/, { timeout: 1000 });
146+
});
147+
148+
test('dynamic header removal should update safe-area classes', async ({ page }, testInfo) => {
149+
testInfo.annotations.push({
150+
type: 'issue',
151+
description: 'https://outsystemsrd.atlassian.net/browse/FW-6830',
152+
});
153+
154+
const content = page.locator('#content-dynamic');
155+
156+
// Add header first
157+
await page.click('#add-header-btn');
158+
await expect(content).not.toHaveClass(/safe-area-top/, { timeout: 1000 });
159+
160+
// Remove header
161+
await page.click('#remove-header-btn');
162+
163+
// Should have safe-area-top again
164+
await expect(content).toHaveClass(/safe-area-top/, { timeout: 1000 });
165+
});
166+
});
167+
});

‎core/src/components/content/test/safe-area/index.html‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
<!DOCTYPE html>
2+
<html lang="en" dir="ltr">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Content - Safe Area</title>
6+
<meta
7+
name="viewport"
8+
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
9+
/>
10+
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
11+
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
12+
<script src="../../../../../scripts/testing/scripts.js"></script>
13+
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
14+
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
15+
<style>
16+
/* Simulate safe-area insets for testing */
17+
:root {
18+
--ion-safe-area-top: 44px;
19+
--ion-safe-area-bottom: 34px;
20+
--ion-safe-area-left: 0px;
21+
--ion-safe-area-right: 0px;
22+
}
23+
24+
.test-section {
25+
border: 2px solid #ccc;
26+
margin: 10px;
27+
height: 200px;
28+
position: relative;
29+
}
30+
31+
.test-section .ion-page {
32+
position: relative;
33+
height: 100%;
34+
}
35+
36+
/* Custom wrapper component for testing wrapped header/footer detection */
37+
my-header,
38+
my-footer {
39+
display: contents;
40+
}
41+
</style>
42+
</head>
43+
44+
<body>
45+
<ion-app>
46+
<!-- Test 1: Content without header - should have safe-area-top -->
47+
<div id="test-no-header" class="test-section">
48+
<div class="ion-page">
49+
<ion-content id="content-no-header">
50+
<p>Content without header - should have safe-area-top class</p>
51+
</ion-content>
52+
<ion-footer>
53+
<ion-toolbar>
54+
<ion-title>Footer</ion-title>
55+
</ion-toolbar>
56+
</ion-footer>
57+
</div>
58+
</div>
59+
60+
<!-- Test 2: Content without footer - should have safe-area-bottom -->
61+
<div id="test-no-footer" class="test-section">
62+
<div class="ion-page">
63+
<ion-header>
64+
<ion-toolbar>
65+
<ion-title>Header</ion-title>
66+
</ion-toolbar>
67+
</ion-header>
68+
<ion-content id="content-no-footer">
69+
<p>Content without footer - should have safe-area-bottom class</p>
70+
</ion-content>
71+
</div>
72+
</div>
73+
74+
<!-- Test 3: Content with both header and footer - should NOT have safe-area classes -->
75+
<div id="test-with-both" class="test-section">
76+
<div class="ion-page">
77+
<ion-header>
78+
<ion-toolbar>
79+
<ion-title>Header</ion-title>
80+
</ion-toolbar>
81+
</ion-header>
82+
<ion-content id="content-with-both">
83+
<p>Content with both header and footer - should NOT have safe-area classes</p>
84+
</ion-content>
85+
<ion-footer>
86+
<ion-toolbar>
87+
<ion-title>Footer</ion-title>
88+
</ion-toolbar>
89+
</ion-footer>
90+
</div>
91+
</div>
92+
93+
<!-- Test 4: Content without both header and footer - should have both safe-area classes -->
94+
<div id="test-no-both" class="test-section">
95+
<div class="ion-page">
96+
<ion-content id="content-no-both">
97+
<p>Content without header or footer - should have both safe-area classes</p>
98+
</ion-content>
99+
</div>
100+
</div>
101+
102+
<!-- Test 5: Content with wrapped header component - should NOT have safe-area-top -->
103+
<div id="test-wrapped-header" class="test-section">
104+
<div class="ion-page">
105+
<my-header>
106+
<ion-header>
107+
<ion-toolbar>
108+
<ion-title>Wrapped Header</ion-title>
109+
</ion-toolbar>
110+
</ion-header>
111+
</my-header>
112+
<ion-content id="content-wrapped-header">
113+
<p>Content with wrapped header - should NOT have safe-area-top class</p>
114+
</ion-content>
115+
</div>
116+
</div>
117+
118+
<!-- Test 6: Content with wrapped footer component - should NOT have safe-area-bottom -->
119+
<div id="test-wrapped-footer" class="test-section">
120+
<div class="ion-page">
121+
<ion-content id="content-wrapped-footer">
122+
<p>Content with wrapped footer - should NOT have safe-area-bottom class</p>
123+
</ion-content>
124+
<my-footer>
125+
<ion-footer>
126+
<ion-toolbar>
127+
<ion-title>Wrapped Footer</ion-title>
128+
</ion-toolbar>
129+
</ion-footer>
130+
</my-footer>
131+
</div>
132+
</div>
133+
134+
<!-- Test 7: Nested content - should NOT have safe-area classes (inner content) -->
135+
<div id="test-nested" class="test-section">
136+
<div class="ion-page">
137+
<ion-content id="content-outer">
138+
<p>Outer content</p>
139+
<div style="height: 100px; border: 1px solid blue">
140+
<ion-content id="content-nested">
141+
<p>Nested content - should NOT have safe-area classes</p>
142+
</ion-content>
143+
</div>
144+
</ion-content>
145+
</div>
146+
</div>
147+
148+
<!-- Test 8: Content inside modal - should NOT have safe-area classes -->
149+
<ion-modal id="test-modal" is-open="false">
150+
<ion-content id="content-in-modal">
151+
<p>Content inside modal - should NOT have safe-area classes</p>
152+
</ion-content>
153+
</ion-modal>
154+
155+
<!-- Test 9: Dynamic header/footer - for testing mutation observer -->
156+
<div id="test-dynamic" class="test-section">
157+
<div class="ion-page" id="dynamic-page">
158+
<ion-content id="content-dynamic">
159+
<p>Content with dynamic header/footer</p>
160+
<button id="add-header-btn" onclick="addHeader()">Add Header</button>
161+
<button id="remove-header-btn" onclick="removeHeader()">Remove Header</button>
162+
</ion-content>
163+
</div>
164+
</div>
165+
166+
<script>
167+
function addHeader() {
168+
const page = document.getElementById('dynamic-page');
169+
const content = document.getElementById('content-dynamic');
170+
if (!page.querySelector('ion-header')) {
171+
const header = document.createElement('ion-header');
172+
header.innerHTML = '<ion-toolbar><ion-title>Dynamic Header</ion-title></ion-toolbar>';
173+
page.insertBefore(header, content);
174+
}
175+
}
176+
177+
function removeHeader() {
178+
const page = document.getElementById('dynamic-page');
179+
const header = page.querySelector('ion-header');
180+
if (header) {
181+
header.remove();
182+
}
183+
}
184+
185+
// Helper to open modal for testing
186+
function openModal() {
187+
document.getElementById('test-modal').isOpen = true;
188+
}
189+
</script>
190+
</ion-app>
191+
</body>
192+
</html>

0 commit comments

Comments
 (0)