Skip to content

Android sheet layout fixes - #537

Open
tifroz wants to merge 10 commits into
skiptools:mainfrom
tifroz:tifroz/android-sheet-layout-fixes
Open

tifroz wants to merge 10 commits into
skiptools:mainfrom
tifroz:tifroz/android-sheet-layout-fixes

Conversation

@tifroz

@tifroz tifroz commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Thank you for contributing to the Skip project! Please review the contribution guide at https://skip.dev/docs/contributing/ for advice and guidance on making high-quality PRs.

Use this space to describe your change and add any labels (bug, enhancement, documentation, etc.) to help categorize your contribution.

Skip Pull Request Checklist:

  • REQUIRED: I have signed the Contributor Agreement
  • REQUIRED: I have tested my change locally with swift test
  • OPTIONAL: I have tested my change on an iOS simulator or device
  • OPTIONAL: I have tested my change on an Android emulator or device
  • REQUIRED: I have checked whether this change requires a corresponding update in the Skip Fuse UI repository (link related PR if applicable)
  • OPTIONAL: I have added an example of any UI changes to the Showcase sample app

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

Code generated by Codex under supervision - all changes were validated manually on test devices


Summary

This fixes multiple visual glitches observed while working with the Android sheet view: sizing and transient clipping + support solid presentation background colors.

Breaking it down:

  • Make onGeometryChange report the full measured layout size even when a scroll viewport clips the view. Frame queries retain their existing clipped bounds.
  • Treat .height(...) as usable content height, accounting for bottom system bars and capping oversized values at the large-sheet boundary.
  • Read the current top inset when constructing the sheet's clipping outline. Previously, the outer composition captured the old inset before modal content resolved the new one. During expansion, the header could move above that stale clipping boundary and disappear for a frame. The change aligns clipping with content without changing the detent calculation or suppressing recomposition. More details below under Sheet clipping regression
  • Support .presentationBackground(Color) for sheets and full-screen covers, resolving adaptive colors inside the presentation's color scheme. Clear backgrounds reveal the presenter; omitting the modifier preserves the default background.

Usage Example

.sheet(isPresented: $isPresented) {
    SheetContent()
        .presentationDetents([.height(320)])
        .presentationBackground(Color.clear)
}

API Shape

public func presentationBackground(_ color: Color) -> any View

The Android PresentationRoot entry point also gains an optional backgroundColor: Color? = nil parameter. Geometry observation keeps its existing public API.

Testing

After rebasing onto upstream 0787680:

  • Android build and 31 instrumented tests passed on Samsung SM-S721U, Android 16; zero failures or skips.
  • 17 geometry tests cover clipped measurements, resizing, coordinate queries, and observation behavior.
  • 14 presentation tests cover detent transitions, full-screen covers, clear/solid/adaptive backgrounds, open-sheet appearance changes, rotation, keyboard/Back, scrolling, nested sheets, WebView identity, dismissal/reopening, and settling without continued redraw or recomposition.
  • The clipping regression was separately verified to fail before the fix and pass after it. See the sheet clipping regression section below for the mechanism and test approach.

Limitations

  • Background support is limited to Color, applied directly to presented content; other shape styles and custom background views remain unsupported.

Pre-existing limitations, unchanged by this PR

Neither of the following is introduced by this PR:

  • Android already selects a single detent rather than supporting interactive resizing between multiple detents.
  • A short fixed-height sheet can lose its content when the keyboard opens. This was reproduced with a 260-point sheet both before and after the clipping fix. Expanded-sheet keyboard behavior passes; compact-sheet keyboard avoidance remains a separate issue.

Sheet clipping regression

Tests/SkipUITests/Skip/SheetPresentationTests.kt exercises the production Android
SheetPresentation with a minimal Compose body. It needs no application,
network service, account, or measured-content sizing code.

The regression changes a fixed detent from 260 to 520 points. The header's
layout moves immediately. Before the fix, the clipping outline can still use
the previous top inset for one frame, hiding the header. The fix reads the inset
while constructing the outline instead of capturing it earlier in composition.

Why the original code could hide content

The sheet uses the same top inset for two jobs: positioning the content below
its reserved top space, and clipping away that space. Those values must agree
in the frame being drawn.

Previously, SheetPresentation read topInset.value in its outer composition
and captured the resulting pixel value in the GenericShape closure. The
ModalBottomSheet content then resolved the current detent and window insets
and wrote a new value to topInset later in composition.

For example, when a sheet expands from 260 to 520 points:

  1. The shape closure initially captures the larger top inset of the short sheet.
  2. The modal content computes a smaller inset and lays out the header higher up.
  3. Until the outer composition catches up, the old clipping boundary can cut
    away the newly positioned header. The content exists but is not drawn.

The change keeps the inset calculation and content layout intact. It moves only
its state read into outline construction:

// Before: the shape captures a pixel value from an earlier composition.
let topInsetPx = with(LocalDensity.current) { topInset.value.toPx() }
let shape = GenericShape { size, _ in
    let y = topInsetPx - handleHeightPx - handlePaddingPx
    // ...
}

// After: the outline uses the current inset when it is constructed.
let density = LocalDensity.current
let shape = GenericShape { size, _ in
    let topInsetPx = with(density) { topInset.value.toPx() }
    let y = topInsetPx - handleHeightPx - handlePaddingPx
    // ...
}

This addresses inconsistent clipping during a layout transition. It does not
suppress general recomposition or change detent selection, measurement, or
animation. The modified presenter serves Android sheets and full-screen covers;
it does not change the shared geometry observer, alerts, or iOS presentation.

The regression fails on the original code and passes with the change on a
Samsung SM-S721U and Pixel 9 Pro emulator, both running Android 16. Frame inspection supports the narrow clipping
finding; the brief artifact is not a persuasive normal-speed video demonstration
of a broader flicker improvement.

@cla-bot cla-bot Bot added the cla-signed label Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant