fix(sheet): make presentation sheets honour the soft keyboard on Android - #483
#483Open
BPHvZ wants to merge 1 commit into
Open
fix(sheet): make presentation sheets honour the soft keyboard on Android#483BPHvZ wants to merge 1 commit into
BPHvZ wants to merge 1 commit into
Conversation
A `.medium` sheet containing a text field was unusable on Android: the keyboard covered the field, so you could not see what you were typing. Cause is structural rather than a missing inset. The sheet cannot react to the IME at all — `contentWindowInsets` is zeroed on the `ModalBottomSheet`, and `PresentationRoot` only applies `imePadding()` when `.bottom` survives in `systemBarEdges`, which it does not for a non-edge-to-edge detent sheet. Since `.medium` is implemented as "content pinned to the bottom half of the screen", and the keyboard covers roughly that same half, the usable area collapsed to a few hundred pixels. Padding the content is not a fix: inside a fixed-height sheet it only squeezes the content and clips it (measured on-device — the field was pushed out of the accessibility tree entirely, so it could not even be selected by a UI test). Instead shift the content area: shrink the detent's top spacer by the IME height and grow the bottom spacer by the same amount. The content area keeps its exact height and translates clear of the keyboard. `topInset` tracks the adjusted value too, since it drives the sheet's rounded-top clip shape. Also fixes the bottom spacer, which computed `max(0, systemBars - ime)`. That clamps to zero the moment the keyboard opens — actively letting content run underneath it. `max(systemBars, ime)` keeps the resting behaviour and clears the keyboard when open. Both spacers collapse to their previous values when the keyboard is closed, so the resting layout is unchanged — verified on-device: with the keyboard down the field bounds are byte-identical to before the patch. Forked from upstream 1.59.0.
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.
A
.mediumpresentation sheet containing aTextFieldis unusable on Android: the soft keyboard covers the field, so you cannot see what you are typing.Cause — structural, not a missing inset. The sheet cannot react to the IME at all:
contentWindowInsetsis zeroed on theModalBottomSheet.PresentationRootappliesimePadding()only when.bottomsurvives insystemBarEdges, which it does not for a non-edge-to-edge detent sheet..mediumis implemented as "content pinned to the bottom half of the screen", and the keyboard covers approximately that same half.Measured on a 2424 px screen: ~330 px of usable area above the keyboard.
There is no app-level workaround.
presentationDetents(_:selection:)is a no-op on Android, and applyingimePadding()to the sheet's content only squeezes it inside a fixed-height sheet — measured on-device, that pushed the field out of the accessibility tree entirely, so it could not even be selected by a UI test.The change. Shift the content area instead of padding it:
topInset, because it also drives the sheet's rounded-top clip shape — otherwise the clip detaches from the visible top edge.It also fixes a second bug in the same function. The bottom spacer computed:
which clamps to 0 the moment the keyboard opens, actively letting content run underneath it. Taking
max(systemBars, ime)instead preserves the resting behaviour (IME closed ⇒ system bars) and clears the keyboard when open.Before / after. Same app, same Release configuration, same device, same docked keyboard — differing only by this commit. The "before" build is upstream 1.59.0 (
9c39113), this branch's exact parent commit, so the patch is the only variable.Before — upstream 1.59.0. The sheet is crushed into the strip above the keyboard: the "Sign in" title sits on the bottom edge, the email field is almost entirely behind the keyboard, and the typed text is not visible at all.
After — this branch. The field and the "Send me a link" button sit well clear of the keyboard, and the typed text is visible.
Verification.
focused=true); typing lands.hw.keyboard. An AVD withhw.keyboard=yes(the AVD default) hides this bug completely. Gboard then runs in physical-keyboard mode and shows a floating toolbar that contributes no bottom insets, so the field looks fine. Reproduce withhw.keyboard=no, which matches real device behaviour. Without this note the report would reasonably be closed as unreproducible.Notes on the checklist below. iOS is left unchecked because the change is inside the Compose-only
#if SKIPlayer ofPresentation.swiftand touches no iOS code path.swift teston this branch runs 74 tests (10 skipped) with 1 failure,LayoutTests.testDrawTextDefaultFont; that failure is pre-existing and unrelated — checking out the parent commit9c39113and running the same filter fails identically, and nothing here touches text drawing. For Skip Fuse UI: no companion PR appears to be needed, becauseskip-fuse-uiconsumes this code as a dependency (.package(url: "…/skip-ui.git", from: "1.59.0"), withSkipSwiftUIdepending on.product(name: "SkipUI", package: "skip-ui")), so the fix propagates transitively, and this patch changes no API surface — only internal layout arithmetic. Happy to add a Showcase example if you would like the regression covered there; reproducing it needs a.mediumsheet containing aTextFieldand an AVD withhw.keyboard=no.Skip Pull Request Checklist:
swift testThe diagnosis, the patch and this description were produced with AI assistance (Claude). Every claim above was checked against the source or on a device rather than accepted from the model:
Presentation.swift(contentWindowInsets,PresentationRoot'ssystemBarEdgesgate, the.mediumdetent arithmetic), read directly in the SkipUI source rather than inferred.imePadding()) was actually implemented, measured on-device — it pushed the field out of the accessibility tree — and then reverted. It is described as rejected because it was tried, not because it sounded wrong.swift testwas run on the branch, and its single failure was proven pre-existing by re-running that test against unmodified upstream 1.59.0.