Skip to content

ios: fix safe area #3232

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shonsirsha
Copy link
Collaborator

@shonsirsha shonsirsha commented Mar 11, 2025

The iOS app is bounded within the safe area causing white area / screen to be shown on the top & bottom edges on iPhones & iPads with notches and rounded corners.

@shonsirsha shonsirsha requested review from benma and thisconnect March 11, 2025 08:00
@@ -51,8 +51,8 @@
.header {
background-color: var(--color-blue);
flex-shrink: 0;
height: var(--header-height);
padding: 0 var(--space-default);
padding: 23px var(--space-default);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I ditched the fixed height and opted for padding-y: 23px, which achieves the same result as height: 70px but is easier to manipulate when factoring in safe-area-inset-top.

Comment on lines +10 to +12
padding-top: env(safe-area-inset-top, 0);
padding-left: env(safe-area-inset-left, 0);
padding-right: env(safe-area-inset-right, 0);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These safe-area-inset-* are built-in variables that provide values which vary based on the device, screen size, and display type (e.g devices with notches / rounded corners)

@shonsirsha shonsirsha force-pushed the ios-fix-safe-area branch 4 times, most recently from db69ef7 to 4293f07 Compare March 17, 2025 05:48
Comment on lines 4 to 15
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover" />
<meta name="theme-color" content="#000000" />
<title>BitBoxApp</title>
</head>

<body>
<div id="root"></div>
<script type="module" src="/src/index.tsx"></script>
</body>

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you remove the indentation on purpose? Please indent again if it was an accident

Copy link
Contributor

@benma benma left a comment

Choose a reason for hiding this comment

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

tACK, tested on my iPhone in portrait and landscape mode and it works great.

Didn't check the code much, as I am out of my depth.

Copy link
Contributor

@benma benma left a comment

Choose a reason for hiding this comment

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

Found bug: dialogs (like tor proxy settings) overlap at the top with an iOS element and the dialog title becomes unreadable.

@shonsirsha shonsirsha marked this pull request as draft April 14, 2025 03:20
@shonsirsha shonsirsha force-pushed the ios-fix-safe-area branch 3 times, most recently from ba24b32 to c1aba6c Compare April 28, 2025 08:53
@shonsirsha shonsirsha marked this pull request as ready for review April 28, 2025 08:53
@shonsirsha shonsirsha requested a review from benma April 28, 2025 08:53
@shonsirsha shonsirsha force-pushed the ios-fix-safe-area branch 7 times, most recently from 4c12fe1 to 0b3d9b3 Compare April 29, 2025 05:44
@shonsirsha
Copy link
Collaborator Author

rebased

@shonsirsha shonsirsha force-pushed the ios-fix-safe-area branch from 0b3d9b3 to 9ef2b22 Compare May 6, 2025 06:55
Copy link
Contributor

@benma benma left a comment

Choose a reason for hiding this comment

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

tACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants