Skip to content

Compose sheets with final presentation-preference values on first composition; stop presenter churn re-evaluating confirmation dialogs - #505

Open
Aecasorg wants to merge 3 commits into
skiptools:mainfrom
Aecasorg:presentation-preferences
Open

Compose sheets with final presentation-preference values on first composition; stop presenter churn re-evaluating confirmation dialogs#505
Aecasorg wants to merge 3 commits into
skiptools:mainfrom
Aecasorg:presentation-preferences

Conversation

@Aecasorg

@Aecasorg Aecasorg commented Aug 6, 2026

Copy link
Copy Markdown

Addresses the preference-settle and presenter-churn items — (c) and part of (a) — of #487.

What changes

1. Sheets compose with final presentation-preference values on their first composition.

preference(key:value:) now emits PreferenceModifier, a SideEffectModifier subclass that carries its key and value as inspectable fields (contribution behavior is byte-for-byte unchanged). SheetPresentation harvests statically-applied presentationDetents / presentationDragIndicator / interactiveDismissDisabled values from the content renderables it already evaluates (the same forEachModifier scan as the existing backDismissDisabled special case) and constructs the preference states with the harvested values as initialValue.

Result: a sheet with .presentationDetents([.medium]) composes at medium-detent geometry on frame 1 instead of full height with a visible jump one composition later — and because the later collector contribution equals the seed, the settle recomposition itself disappears. Dynamic preference updates flow through the collectors exactly as before; preferences applied deeper than the content's own modifier chain keep the previous settle behavior (no regression, harvest is additive).

2. ModalBottomSheet arguments stabilized. The shape is remembered keyed on its inputs, the dismiss callback is remembered over rememberUpdatedState, and backDismissDisabled is harvested once per presentation (only its first composition is honored anyway, per the existing comment). Fresh unequal instances per presenter pass were forcing the sheet machinery to recompose on every presenter recomposition.

3. Confirmation dialogs stop re-running action/message bodies on presenter churn. ConfirmationDialogPresentation fully evaluated its actions and message (options: 0 — bodies run) in the presenter's composition scope on every presenter recomposition while open. The evaluation now runs inside the ModalBottomSheet content lambda — the dialog window's own composition.

Verification

On-device A/B (API 35 emulator; standalone instrumented app https://github.com/Aecasorg/skip-fuse-perf-repro, scene 4: a sheet with .presentationDetents([.medium]) presented over a presenter that ticks an unrelated @Observable every second):

stock 1.59.1 patched
height seen by the sheet content's 1st evaluation 801.5 (full height) → 438.1 ~50ms later, visible jump 438.1 from the 1st evaluation
sheet-content evaluations during open n n − 1 (settle pass gone)
evaluations per presenter tick while open +1 +1 (unchanged — see note)

Note on the last row: content re-evaluation on presenter churn persists because the content lambda's captures (fresh renderables per presenter pass) invalidate the dialog composition — that residue is the bridged-peer identity problem (skip-bridge#113 / #486 territory) and is out of scope here. We considered memoizing contentRenderables as the issue suggested, but with isKeepNonModified evaluation the sheet's liveness for builder-level state rides on the presenter-scope re-evaluate — caching it without content identity would freeze open sheets, so we deliberately did not.

Alert presentation has the same presenter-scope evaluation pattern (plus text-field state extraction) — left for a follow-up to keep this reviewable.

Independent of #499 (different files except Presentation.swift, no overlapping hunks); the two merge in either order.

Happy to adjust naming, split the commits into separate PRs, or extend the harvest to alerts here if you'd prefer.

Aecasorg and others added 2 commits August 6, 2026 18:07
…bilize ModalBottomSheet arguments

preference(key:value:) now emits PreferenceModifier, a SideEffectModifier
subclass carrying its key and value as inspectable fields (contribution
behavior unchanged). SheetPresentation harvests statically-applied
presentationDetents / presentationDragIndicator / interactiveDismissDisabled
values from the already-evaluated content renderables (mirroring the
existing backDismissDisabled scan) and seeds the preference states with
them, so the sheet's first composition uses final detent geometry and
gesture gating instead of defaults that settle — with a visible jump —
one composition later. Dynamic preference updates flow through the
collectors exactly as before; preferences applied deeper than the content's
own modifier chain keep the previous settle-pass behavior.

Also stabilizes the ModalBottomSheet call: shape remembered keyed on its
inputs, dismiss callback remembered over rememberUpdatedState, and
backDismissDisabled harvested once per presentation (only its first
composition is honored anyway, per the existing comment) — so presenter
recompositions stop churning the sheet machinery through unstable
arguments.

Part of skiptools#487 (a) and (c).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 7e51963)
…own composition

ConfirmationDialogPresentation fully evaluated its actions and message
views (options: 0 — bodies run) in the presenter's composition scope on
every presenter recomposition while the dialog was open or animating
closed. Moving the evaluation inside the ModalBottomSheet content lambda
runs it in the dialog window's composition instead, and the presenter-scope
cost per recomposition drops to the sheet machinery call itself. Also
stabilizes the dialog's onDismissRequest the same way as SheetPresentation.

Part of skiptools#487 (a).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 6c215ea)
@cla-bot cla-bot Bot added the cla-signed label Aug 6, 2026
Matches the sheet and confirmation-dialog sites: a fresh closure per pass
is an unstable Dialog argument. Note this does NOT address AlertPresentation's
presenter-scope evaluation of actions and message — unlike the other two
sites that cannot be relocated into the dialog's composition, because
SkipAlertDialog's slot API needs the button structure (neutral-button count,
presence of cancel/text fields) before the Dialog call, and discovering that
structure is the evaluation itself. A real fix needs SkipAlertDialog to take
a builder that runs inside the Dialog.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 4756b50)
@Aecasorg

Aecasorg commented Aug 7, 2026

Copy link
Copy Markdown
Author

Pushed a third commit: the alert dialog's onDismissRequest gets the same remembered-callback treatment as the sheet and confirmation-dialog sites in this PR — same file, same pattern, so it seemed to belong here rather than in a PR of its own.

Worth being explicit that this is a consistency fix only: it does not address AlertPresentation's presenter-scope evaluation of actions/message. Unlike the two sites above, that evaluation cannot simply move inside the dialog, because SkipAlertDialog's slot API needs the button structure (neutral-button count, whether a cancel button exists, whether there are text fields) before the Dialog call — and discovering that structure is the evaluation itself. Fixing it properly looks like reshaping SkipAlertDialog to take a builder that discovers structure inside the Dialog, which also runs through the text-field slot where re-keying would cost FocusRequester/TextFieldValue state. Happy to attempt it if you'd like it in scope, but it seemed wrong to bundle a change of that size in here.

@Aecasorg

Aecasorg commented Aug 7, 2026

Copy link
Copy Markdown
Author

Re-ran the verification properly, because the A/B in the PR description was confounded: its "stock" side was skip-ui 1.57.0 with skip 1.9.4 / skip-fuse-ui 1.17.2, while the patched side was built on 1.59.1 with skip 1.9.5 / skip-fuse-ui 1.18.1 — so it could not distinguish this change from the upstream version delta. Apologies; that was sloppy of me.

Redone with skip-ui as the only variable (skip 1.9.5 / skip-fuse-ui 1.18.1 both sides, upstream 1.59.1 vs the same tree plus these commits), API 35 emulator, 10 scripted sheet open/close cycles per build, sheet declaring .presentationDetents([.medium]) over a presenter ticking an unrelated @Observable every second:

Metric over 10 opens upstream 1.59.1 with these commits
sheet-content evaluations observing the pre-settle full height (801.5 rather than the 438.1 medium detent) 10 — exactly one per open 0
sheet-content body evaluations 41 32
geometry-proxy evaluations 346 326
presenter ticks (control variable) 44 44

So the settle pass is deterministic upstream — one per sheet open, every time — and it is gone with the preference seeding, along with ~22% of the sheet-content evaluations for the same number of presenter ticks.

Being straight about what this does not show: frame-level timing over the same runs was 7.8% vs 8.7% janky frames and 32ms vs 34ms at the 95th percentile — i.e. no measurable end-user smoothness win, and the small differences sit inside single-run emulator noise in both directions (the 99th percentile went the other way, 57ms vs 53ms). I would not claim a perceptible improvement from this data, and an emulator is a poor instrument for that question anyway. The defensible claim is the one above: a guaranteed extra composition per presentation, with its geometry jump, is removed.

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