fix: respect iOS safe area insets in fullscreen dialog (fixes #4833)#11438
Open
kaarude wants to merge 1 commit into
excalidraw:masterexcalidraw/excalidraw:masterfrom
kaarude:fix/4833-ios-safe-area-dialogkaarude/excalidraw:fix/4833-ios-safe-area-dialogCopy head branch name to clipboard
Open
fix: respect iOS safe area insets in fullscreen dialog (fixes #4833)#11438kaarude wants to merge 1 commit intoexcalidraw:masterexcalidraw/excalidraw:masterfrom kaarude:fix/4833-ios-safe-area-dialogkaarude/excalidraw:fix/4833-ios-safe-area-dialogCopy head branch name to clipboard
kaarude wants to merge 1 commit into
excalidraw:masterexcalidraw/excalidraw:masterfrom
kaarude:fix/4833-ios-safe-area-dialogkaarude/excalidraw:fix/4833-ios-safe-area-dialogCopy head branch name to clipboard
Conversation
The fullscreen dialog (used on phones, including devices with notches like iPhone X and later) placed its title and close button at the top of the screen, hidden behind the notch. The bottom controls (e.g. the Save/Cancel buttons in the export dialog) were similarly hidden behind the home indicator. The :root already exposes `--sat`, `--sar` and `--sab` CSS variables mapped to `env(safe-area-inset-*)` together with `viewport-fit=cover` in the meta tag, but they were not applied to the fullscreen dialog. Apply them to the title, close button, and content padding so the dialog is fully visible on notched devices.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Author
|
👋 Friendly ping for review on this PR — would love a maintainer's eyes on it. The fix is a 15-line CSS change scoped to @DWesley @ad1992 (the two maintainers I see most active on mobile/UI work, and @ad1992 specifically commented on the original 2022 attempt that this issue regressed from) — happy to iterate if the approach isn't what you'd prefer. The full breakdown with before/after measurements is in the PR description. |
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #4833 — the fullscreen dialog (used on phones) did not respect iOS safe area insets, so on notched devices:
The
:rootstylesheet already exposes--sat,--sarand--sabCSS variables mapped toenv(safe-area-inset-*), andindex.htmlalready setsviewport-fit=coverin the viewport meta tag. The variables were simply never being used anywhere in the component styles. This PR applies them to the fullscreen dialog.Changes
packages/excalidraw/components/Dialog.scss— only the.Dialog--fullscreenblock is touched (no impact on tablet/desktop layout):env(safe-area-inset-*)resolves to0on devices without a notch, so the change is a no-op there.Why this approach
--sat/--sar/--sabare already declared inpackages/excalidraw/css/styles.scss— this PR is just the first place to consume them..Dialog--fullscreen, which is only applied whenformFactor === "phone", so the desktop/tablet layout is untouched.env()function is supported everywhere Excalidraw is expected to run (every browser that shipsviewport-fit=coveralso supportsenv()).Verification
Built a minimal HTML reproduction matching the exact Modal → Dialog → Island → title/close/content DOM and the same SCSS, with a simulated iPhone X safe area (top: 47px, bottom: 34px) injected via
:rootoverrides. MeasuredgetBoundingClientRect()and computed styles for the title, close button, and content in both states:.Dialog__titletop edgey = 40(under the 47px notch)y = 40, text starts aty = 87(below the notch).Dialog__closetop edgey = 12(under the notch)y = 67(below the notch).Dialog__contentbottom padding0(buttons under home indicator)34px(buttons above the indicator)Tested at 390×844 (iPhone 13 viewport) with Chromium 145.
Previous attempt
PR #5384 (closed, not merged in 2022) took a much larger approach: it moved the title to the bottom of the screen, but the maintainers' feedback at the time was that the layout needed "minor design tweaks" and the PR was eventually closed. This PR keeps the title at the top (where users expect it) and only adjusts the safe-area insets, which is a smaller, more conservative change.
Test plan
Dialog) on an iPhone X or later, in both portrait and landscape. Confirm the title, close button, and bottom Save/Cancel buttons are fully visible.env()values resolve to0)..Dialog--fullscreenis only applied on phones.No automated test was added because the change is a pure CSS layout adjustment with no behavior to assert; the existing visual regression tests don't cover device-pixel-ratio variations of this kind. Happy to add a snapshot test if maintainers want one.