Skip to content

Conversation

@thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Dec 12, 2025

Issue number: internal


What is the current behavior?

The safe area variables are only reliant on env variables that are provided by devices.

What is the new behavior?

Capacitor 8 has released safe area variable fallbacks to provide consistent behaviors with older Android devices:

Due to a bug in some older versions of Android WebView (< 140), correct safe area values are not available via the safe-area-inset-x CSS env variables. This plugin will inject the correct inset values into a new CSS variable(s) named --safe-area-inset-x that you can use as a fallback in your frontend styles.

  • Updated safe area variables to use the fallbacks provided by Capacitor.

Does this introduce a breaking change?

  • Yes
  • No

Other information

N/A

@vercel
Copy link

vercel bot commented Dec 12, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
ionic-framework Ready Ready Preview Comment Dec 12, 2025 8:04pm

@github-actions github-actions bot added the package: core @ionic/core package label Dec 12, 2025
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.

@thetaPC thetaPC marked this pull request as ready for review December 12, 2025 20:13
@thetaPC thetaPC requested a review from a team as a code owner December 12, 2025 20:13
@thetaPC thetaPC requested a review from gnbm December 12, 2025 20:13
Copy link
Member

@ShaneK ShaneK left a comment

Choose a reason for hiding this comment

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

This looks good to me! Excellent work 🎉

--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.

Comment on lines +156 to +166
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');
}
}
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');
}

Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Some questions / feedback on tests here.

Comment on lines +48 to +70
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');
});
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');
});
});
});

--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)?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants