Skip to content
Open
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
56 changes: 46 additions & 10 deletions core/src/components/app/test/safe-area/app.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,56 @@ configs({ directions: ['ltr'] }).forEach(({ config, title, screenshot }) => {

await expect(page).toHaveScreenshot(screenshot(`app-${screenshotModifier}-diff`));
};

test.beforeEach(async ({ page }) => {
await page.goto(`/src/components/app/test/safe-area`, config);
});
test('should not have visual regressions with action sheet', async ({ page }) => {
await testOverlay(page, '#show-action-sheet', 'ionActionSheetDidPresent', 'action-sheet');
});
test('should not have visual regressions with menu', async ({ page }) => {
await testOverlay(page, '#show-menu', 'ionDidOpen', 'menu');
});
test('should not have visual regressions with picker', async ({ page }) => {
await testOverlay(page, '#show-picker', 'ionPickerDidPresent', 'picker');

test.describe(title('Ionic safe area variables'), () => {
test.beforeEach(async ({ page }) => {
const htmlTag = page.locator('html');
const hasSafeAreaClass = await htmlTag.evaluate((el) => el.classList.contains('safe-area'));

expect(hasSafeAreaClass).toBe(true);
});

test('should not have visual regressions with action sheet', async ({ page }) => {
await testOverlay(page, '#show-action-sheet', 'ionActionSheetDidPresent', 'action-sheet');
});
test('should not have visual regressions with menu', async ({ page }) => {
await testOverlay(page, '#show-menu', 'ionDidOpen', 'menu');
});
test('should not have visual regressions with picker', async ({ page }) => {
await testOverlay(page, '#show-picker', 'ionPickerDidPresent', 'picker');
});
test('should not have visual regressions with toast', async ({ page }) => {
await testOverlay(page, '#show-toast', 'ionToastDidPresent', 'toast');
});
});
test('should not have visual regressions with toast', async ({ page }) => {
await testOverlay(page, '#show-toast', 'ionToastDidPresent', 'toast');

test.describe(title('Capacitor safe area variables'), () => {
test.beforeEach(async ({ page }) => {
const toggleButton = page.locator('#toggle-safe-area');
await toggleButton.click();

const htmlTag = page.locator('html');
const hasSafeAreaClass = await htmlTag.evaluate((el) => el.classList.contains('safe-area-capacitor'));

expect(hasSafeAreaClass).toBe(true);
});

test('should not have visual regressions with action sheet', async ({ page }) => {
await testOverlay(page, '#show-action-sheet', 'ionActionSheetDidPresent', 'capacitor-action-sheet');
});
test('should not have visual regressions with menu', async ({ page }) => {
await testOverlay(page, '#show-menu', 'ionDidOpen', 'capacitor-menu');
});
test('should not have visual regressions with picker', async ({ page }) => {
await testOverlay(page, '#show-picker', 'ionPickerDidPresent', 'capacitor-picker');
});
test('should not have visual regressions with toast', async ({ page }) => {
await testOverlay(page, '#show-toast', 'ionToastDidPresent', 'capacitor-toast');
});
Comment on lines +48 to +70
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than adding additional screenshot tests for this, what if we just checked that the value of the CSS variables are being used when set:

Suggested change
test.describe(title('Capacitor safe area variables'), () => {
test.beforeEach(async ({ page }) => {
const toggleButton = page.locator('#toggle-safe-area');
await toggleButton.click();
const htmlTag = page.locator('html');
const hasSafeAreaClass = await htmlTag.evaluate((el) => el.classList.contains('safe-area-capacitor'));
expect(hasSafeAreaClass).toBe(true);
});
test('should not have visual regressions with action sheet', async ({ page }) => {
await testOverlay(page, '#show-action-sheet', 'ionActionSheetDidPresent', 'capacitor-action-sheet');
});
test('should not have visual regressions with menu', async ({ page }) => {
await testOverlay(page, '#show-menu', 'ionDidOpen', 'capacitor-menu');
});
test('should not have visual regressions with picker', async ({ page }) => {
await testOverlay(page, '#show-picker', 'ionPickerDidPresent', 'capacitor-picker');
});
test('should not have visual regressions with toast', async ({ page }) => {
await testOverlay(page, '#show-toast', 'ionToastDidPresent', 'capacitor-toast');
});
test.describe(title('Capacitor safe area variables'), () => {
test('should use safe-area-inset vars when safe-area class is defined', async ({ page }) => {
await page.evaluate(() => {
const html = document.documentElement;
// Remove the safe area class
html.classList.remove('safe-area');
// Set the safe area inset variables
html.style.setProperty('--safe-area-inset-top', '10px');
html.style.setProperty('--safe-area-inset-bottom', '20px');
html.style.setProperty('--safe-area-inset-left', '30px');
html.style.setProperty('--safe-area-inset-right', '40px');
});
const top = await page.evaluate(() =>
getComputedStyle(document.documentElement).getPropertyValue('--ion-safe-area-top').trim()
);
const bottom = await page.evaluate(() =>
getComputedStyle(document.documentElement).getPropertyValue('--ion-safe-area-bottom').trim()
);
const left = await page.evaluate(() =>
getComputedStyle(document.documentElement).getPropertyValue('--ion-safe-area-left').trim()
);
const right = await page.evaluate(() =>
getComputedStyle(document.documentElement).getPropertyValue('--ion-safe-area-right').trim()
);
expect(top).toBe('10px');
expect(bottom).toBe('20px');
expect(left).toBe('30px');
expect(right).toBe('40px');
});
});
});

});
});
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff is only the new toggle button

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really should update these tests so all screenshots aren't updated when a new button is added.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions core/src/components/app/test/safe-area/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@
--ion-safe-area-bottom: 40px;
--ion-safe-area-left: 40px;
}

.safe-area-capacitor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to add much to this test besides forcing all screenshots to be updated and adding new ones. Even the values are the exact same so there won't be any screenshot difference. Can we not just check that the right value is being used (see my e2e edit)?

--safe-area-inset-top: 40px;
--safe-area-inset-right: 40px;
--safe-area-inset-bottom: 40px;
--safe-area-inset-left: 40px;
}
</style>
</head>

Expand Down Expand Up @@ -48,6 +55,7 @@
</ion-header>

<ion-content id="main">
<button id="toggle-safe-area" onclick="toggleSafeArea()">Toggle Safe Area Classes</button>
<ion-card>
<ion-card-header>
<ion-card-title>Card</ion-card-title>
Expand Down Expand Up @@ -143,6 +151,19 @@
document.body.appendChild(toast);
await toast.present();
}

// toggle between safe area classes based on button click
function toggleSafeArea() {
console.log('Toggling safe area classes');
const htmlTag = document.documentElement;
if (htmlTag.classList.contains('safe-area-capacitor')) {
htmlTag.classList.remove('safe-area-capacitor');
htmlTag.classList.add('safe-area');
} else {
htmlTag.classList.remove('safe-area');
htmlTag.classList.add('safe-area-capacitor');
}
}
Comment on lines +156 to +166
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function toggleSafeArea() {
console.log('Toggling safe area classes');
const htmlTag = document.documentElement;
if (htmlTag.classList.contains('safe-area-capacitor')) {
htmlTag.classList.remove('safe-area-capacitor');
htmlTag.classList.add('safe-area');
} else {
htmlTag.classList.remove('safe-area');
htmlTag.classList.add('safe-area-capacitor');
}
}
function toggleSafeArea() {
console.log('Toggling safe area classes');
const htmlTag = document.documentElement;
htmlTag.classList.toggle('safe-area-capacitor');
htmlTag.classList.toggle('safe-area');
}

</script>
</body>
</html>
10 changes: 6 additions & 4 deletions core/src/css/core.scss
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,12 @@ html.plt-ios.plt-hybrid, html.plt-ios.plt-pwa {

@supports (padding-top: env(safe-area-inset-top)) {
html {
--ion-safe-area-top: env(safe-area-inset-top);
--ion-safe-area-bottom: env(safe-area-inset-bottom);
--ion-safe-area-left: env(safe-area-inset-left);
--ion-safe-area-right: env(safe-area-inset-right);
// `--safe-area-inset-*` are set by Capacitor
// @see https://capacitorjs.com/docs/apis/system-bars#android-note
--ion-safe-area-top: var(--safe-area-inset-top, env(safe-area-inset-top, 0px));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to fallback to 0px here? I remember it causing issues for us in the past.

--ion-safe-area-bottom: var(--safe-area-inset-bottom, env(safe-area-inset-bottom, 0px));
--ion-safe-area-left: var(--safe-area-inset-left, env(safe-area-inset-left, 0px));
--ion-safe-area-right: var(--safe-area-inset-right, env(safe-area-inset-right, 0px));
}
}

Expand Down
Loading