Repository navigation
fix(mobile): keep a form sheet open while its content scrolls - #8249
Merged
Merged
Conversation
A full-window sheet no longer dismisses on a downward drag, so scrolling a form back up keeps the sheet and the values typed into it. Every form sheet asks Keep editing / Discard before a close, or Android Back, drops unsaved edits. The @expo/ui patch separates a bottom sheet's drag gesture from its back and scrim dismissal, and lets the sheet own the Back press so a caller can answer it first.
The sheet patch carries Kotlin, and expo-ui otherwise resolves to its published artifact. Its Gradle project name is expo-ui.
The invite sheet now opens a Keep editing / Discard confirm and answers Android Back, so its mounted suite needs those modules stubbed.
The host stubs expose props untyped, so one small narrowing helper per suite replaces the inline casts.
iscekic
added
drain
Collaborator
Author
|
bot: Verified on the Android emulator (local backend and Metro, dev client built from this branch's native inputs) and on an iOS simulator lane (dev client of this branch's fingerprint, Metro on the lease's offset). Fake local account, no production data. Android, profile agent sheet:
iOS, same sheet: after three downward drags inside the form the sheet stays open (the reported gesture dismissed it before this change; the same build with the old gesture reproduced that dismissal). The simulator screenshot endpoint does not include the native confirmation sheet, so the confirmation itself is evidenced by the accessibility snapshot text, not an image.
|
expo-ui only reached androidx.activity.compose transitively, so compiling the patched module failed on CI. Declare it in the patched module.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the last reviewed commit ( Verification
Files Reviewed (1 file)
Previous Review Summaries (6 snapshots, latest commit e8cbf5c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e8cbf5c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the last reviewed commit (bf62c03) is a new mounted test in Verification
Files Reviewed (1 file)
Previous review (commit bf62c03)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryNo code changed since the last reviewed commit ( Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit 40948d2)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new whitespace test is correct, but the OAuth-to-bearer-and-back regression fixed in Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit b366a0d)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental fix correctly stops counting a typed token when the auth method is not Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 1e07de7)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental fix keeps Cancel usable during saves and scopes Back handling to focus, but the remote MCP dirty check still reports an edit when an OAuth server's typed token is dropped by the saved draft. Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (12 files)
Fix these issues in Kilo Cloud Previous review (commit ad8403d)Status: 5 Issues Found | Recommendation: Address before merge Executive SummaryThe invite-member sheet becomes impossible to close on iOS until a valid email is typed, because its header Cancel is coupled to validation while this PR removes the sheet's swipe dismissal. Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (119 files)
Reviewed by deepseek-v4.1-flash-1017 · Input: 61.7K · Output: 4.9K · Cached: 385.8K Review guidance: REVIEW.md from base branch |
The header can disable Done alone, so Cancel still closes a sheet whose commit is unavailable or waiting: the invite form with an empty address, a new list without a name, and any form while its save is in flight. The two route-sheet Back listeners now run only while their screen is focused, so the pickers they push keep their own Back. The remote MCP form compares what the saved draft keeps, so whitespace alone no longer asks to discard.
The saved draft keeps a token only for the bearer method, so the dirty check follows the same rule.
The prior review summary predates the resolved remarks.
The split follows responsibility between the fields and their requests, not a line count.
iscekic
added
merge:human
pandemicsyn
approved these changes
Oct 20, 2026
…-while-scrolling # Conflicts: # apps/mobile/src/components/picker-sheet.tsx
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.
What changed
A full-window sheet no longer dismisses when the user drags the content of its form. Before, on iOS the native sheet claimed every downward drag that started over the form's scroll view, so scrolling the form back up dismissed the sheet and dropped its values. Now the shared sheet answer is:
SheetgainsdismissGestures(off for the full-windowSessionPageSheet, so the drag belongs to the content) andonRequestDismiss, which lets a form answer a dismissal the sheet received by itself (Android Back) before it happens.@expo/uipatch: the bottom sheet separates its drag gesture from its back/scrim dismissal. Because Material3 hides the sheet by itself before JavaScript hears the request (measured on the emulator), the Kotlin side now owns Back: it passesshouldDismissOnBackPress = falseto Material3 and forwards the press to JavaScript through aBackHandler, so the sheet hides only when JavaScript drops it.apps/mobile/package.jsoncompilesexpo-uifrom its Android sources (its Gradle project name) so the Kotlin patch is in the build.useSheetDismissGuard: the shared Keep editing / Discard confirmation. A form reads its own unsaved state at the moment of the dismissal and only closes after Discard.apps/mobile/src/i18n/locales/en.jsonplus every locale catalog:common.discardChangesTitleandcommon.discardChangesMessage.End-to-end evidence
Emulator (
omp36, local backend, Metro, fresh dev client built from this branch's native inputs):Discard changes? / Your changes will be lost. / Discard / Keep editing; Keep editing kept the sheet and its scroll position; Discard closed it to the agents list.iOS simulator (lane, dev client of this branch's fingerprint, Metro on the lease's offset):
alphastill in Agent name, and Cancel showedDiscard changes? / Your changes will be lost.with Discard and Keep editing; Keep editing returned to the sheet intact.Checks
pnpm --filter kilo-app typecheck,lint,check:i18n,check:pluralspnpm --filter kilo-app test(whole suite) andnode tools/i18n/check-catalogs.mjsVisual proof
Screenshots are attached in a follow-up comment on this pull request.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Shared + all profile form sheets
Confidence: 0.82 · Precedent: Fix the root cause and the class: session 75f8833fe8c1, "fix this for all shared services model pickers on all surfaces"
Answer: Yes, confirm on dirty dismissal
Confidence: 0.82 · Precedent: Fix the root cause and the class: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class"
Answer: BackHandler routes to confirm
Confidence: 0.85 · Precedent: UX best-in-class standard: session 00eb2a1d3e4b, "design and UX MUST BE BEST-IN-CLASS"
Answer: Accept
Confidence: 0.60 · Precedent: Keep similar elements consistent across surfaces: session a247f3c19b1e, the thought row should match the tool call row
Answer: Patch @expo/ui JS (Recommended)
Confidence: 0.65 · Precedent: Fix the root cause and the class of the issue: "fix class of issue preventing progress"; Choose maximum performance and full automation even if this adds a dependency: "I don't mind an additional dep if performance is legit better"
Answer: Every form sheet
Confidence: 0.60 · Precedent: Fix the root cause and the class of the issue on every affected surface: "fix this for all shared services model pickers on all surfaces (web, app)"
Answer: Keep editing / Discard only (Recommended)
Confidence: 0.75 · Precedent: Do the least work possible with no drop in quality: "skip work that doesn't need to be done"
Answer: Disable swipe + add close control (Recommended)
Confidence: 0.75 · Precedent: Fix the root cause and the class of the issue on every affected surface: session 75f8833fe8c1, "fix this for all shared services model pickers on all surfaces (web, app)"