From 026516f32360f93cafc65843cd95da032860d34d Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 10 Jul 2026 23:04:38 -0700 Subject: [PATCH] the-sandbox-flag-is-2 (#8236) * Fix mobile source control drawer overflow and branch-compare state loss - Render BottomDrawer in a native Modal so it covers the full viewport even when mounted inside a ScrollView. - Move the conflict/Abort row onto its own line so it never overflows the branch card, and enlarge the Abort hit target. - Show the committed-on-branch footer even when the changed-files SectionList has no sections, since RN skips ListFooterComponent for empty sections. - Stop branch-compare state from collapsing to idle/error on transient base-ref resolution failures when a ready result should be preserved. * Add mobile source control drawer reload screenshot Attaches an evidence screenshot for the mobile source control drawer overflow / branch-compare state loss fix. * Remove stray temp screenshot file Accidentally committed debug artifact from mobile source control drawer work; not needed in the repo. --- mobile/src/components/BottomDrawer.tsx | 188 +++++++++--------- .../MobileSourceControlBranchCard.tsx | 37 ++-- .../MobileSourceControlContent.tsx | 52 +++-- .../mobile-source-control-styles.ts | 27 ++- .../use-mobile-source-control-loaders.ts | 21 +- .../use-mobile-source-control-state.ts | 2 + 6 files changed, 195 insertions(+), 132 deletions(-) diff --git a/mobile/src/components/BottomDrawer.tsx b/mobile/src/components/BottomDrawer.tsx index 0f83bbf5f..4e24e8c2e 100644 --- a/mobile/src/components/BottomDrawer.tsx +++ b/mobile/src/components/BottomDrawer.tsx @@ -7,7 +7,8 @@ import { useWindowDimensions, ScrollView, Keyboard, - BackHandler + BackHandler, + Modal } from 'react-native' import { useSafeAreaInsets } from 'react-native-safe-area-context' import { Gesture, GestureDetector, GestureHandlerRootView } from 'react-native-gesture-handler' @@ -264,96 +265,105 @@ function MountedBottomDrawer({ return { opacity: progress.value * dragFade } }) + // Why: rendering through a full-screen Modal lifts the sheet into its own + // native window so it always covers the viewport — even when the drawer is + // mounted deep inside a ScrollView, where a plain absolute overlay anchors to + // the scrolled content and clips the sheet. The Modal stays mounted (visible) + // for the whole life of MountedBottomDrawer so the reanimated exit animation + // runs before the parent unmounts us; show/hide is driven by `progress`, so + // animationType stays "none". onRequestClose handles the Android back button. return ( - - - - - - - - - {!contentScrollable ? ( - <> - - - - - - {children} - - ) : dragContentToDismiss ? ( - <> - - - - - - - - - - {children} - - - - - - ) : ( - <> - - - - - - - {children} - - - )} - + + + + + - - - + + + + {!contentScrollable ? ( + <> + + + + + + {children} + + ) : dragContentToDismiss ? ( + <> + + + + + + + + + + {children} + + + + + + ) : ( + <> + + + + + + + {children} + + + )} + + + + + + ) } diff --git a/mobile/src/source-control/MobileSourceControlBranchCard.tsx b/mobile/src/source-control/MobileSourceControlBranchCard.tsx index 04aae01b0..9be5430c8 100644 --- a/mobile/src/source-control/MobileSourceControlBranchCard.tsx +++ b/mobile/src/source-control/MobileSourceControlBranchCard.tsx @@ -55,23 +55,28 @@ export function MobileSourceControlBranchCard({ {unstagedCount} changed {stagedCount} staged {branchCount > 0 ? {branchCount} on branch : null} - {showConflict ? ( - - {conflictOperation} - {conflictOperation === 'merge' || conflictOperation === 'rebase' ? ( - [styles.abortButton, pressed && styles.abortPressed]} - disabled={conflictBusy} - onPress={() => onAbortConflict(conflictOperation)} - > - - {mobileConflictAbortLabel(conflictOperation, conflictAborting)} - - - ) : null} - - ) : null} + {/* Own row so Abort never overflows past the card when counts are long. */} + {showConflict ? ( + + {conflictOperation} + {conflictOperation === 'merge' || conflictOperation === 'rebase' ? ( + [ + styles.abortButton, + conflictBusy && styles.abortButtonDisabled, + pressed && !conflictBusy && styles.abortPressed + ]} + disabled={conflictBusy} + onPress={() => onAbortConflict(conflictOperation)} + > + + {mobileConflictAbortLabel(conflictOperation, conflictAborting)} + + + ) : null} + + ) : null} {prChip ? : null} ) diff --git a/mobile/src/source-control/MobileSourceControlContent.tsx b/mobile/src/source-control/MobileSourceControlContent.tsx index a28504a5c..885d8bf0c 100644 --- a/mobile/src/source-control/MobileSourceControlContent.tsx +++ b/mobile/src/source-control/MobileSourceControlContent.tsx @@ -1,4 +1,12 @@ -import { ActivityIndicator, Pressable, SectionList, Text, TextInput, View } from 'react-native' +import { + ActivityIndicator, + Pressable, + ScrollView, + SectionList, + Text, + TextInput, + View +} from 'react-native' import { Minus, MoreHorizontal, Plus, Sparkles } from 'lucide-react-native' import { colors, spacing } from '../theme/mobile-theme' import { MobileSourceControlCreatePrEntry } from './MobileSourceControlCreatePrEntry' @@ -13,7 +21,8 @@ type Props = { state: MobileSourceControlState } -// The ready-state Changes segment body: quick actions, changed-files list, and commit bar. +// Changes tab: local file changes only — uncommitted (staged/unstaged) plus +// committed-on-branch vs base. PR conflicts and push status live elsewhere. export function MobileSourceControlContent({ state }: Props) { const { insets, @@ -49,6 +58,21 @@ export function MobileSourceControlContent({ state }: Props) { const shouldShowGenerateButton = stagedCount > 0 || generatingMessage const createPrHeroActive = createPrAction.visible && !createPrAction.disabled && !createPrAction.pushFirst + const branchCompareFooter = ( + + ) return ( <> @@ -126,9 +150,15 @@ export function MobileSourceControlContent({ state }: Props) { {!hasVisibleChanges ? ( - No Changes + No local changes Working tree is clean. + ) : sections.length === 0 ? ( + // Why: RN SectionList with empty `sections` often skips ListFooterComponent, + // which hid "Committed on Branch" when only branch files remain. + + {branchCompareFooter} + ) : ( {section.data.length} )} - ListFooterComponent={ - - } + ListFooterComponent={branchCompareFooter} stickySectionHeadersEnabled={false} contentContainerStyle={styles.listContent} /> diff --git a/mobile/src/source-control/mobile-source-control-styles.ts b/mobile/src/source-control/mobile-source-control-styles.ts index ea7667318..d8dc20a14 100644 --- a/mobile/src/source-control/mobile-source-control-styles.ts +++ b/mobile/src/source-control/mobile-source-control-styles.ts @@ -92,6 +92,7 @@ const baseStyles = StyleSheet.create({ }, countRow: { flexDirection: 'row', + flexWrap: 'wrap', gap: spacing.md, marginTop: spacing.sm }, @@ -99,29 +100,43 @@ const baseStyles = StyleSheet.create({ color: colors.textSecondary, fontSize: typography.metaSize }, + // Separate line under counts — keeps Abort inside the card on narrow phones. conflictRow: { flexDirection: 'row', + flexWrap: 'wrap', alignItems: 'center', - gap: spacing.sm + gap: spacing.sm, + marginTop: spacing.sm, + alignSelf: 'flex-start', + maxWidth: '100%' }, conflictText: { color: colors.statusAmber, fontSize: typography.metaSize, textTransform: 'capitalize' }, + // Match bulk-action hit target so Abort reads as a real control, not a chip. abortButton: { - paddingHorizontal: spacing.sm, - paddingVertical: 2, + minHeight: 32, + paddingHorizontal: spacing.md, + paddingVertical: spacing.xs, borderRadius: radii.button, borderWidth: 1, - borderColor: colors.statusAmber + borderColor: colors.statusAmber, + backgroundColor: colors.bgRaised, + alignItems: 'center', + justifyContent: 'center', + flexShrink: 0 }, abortPressed: { - backgroundColor: colors.bgRaised + opacity: 0.75 + }, + abortButtonDisabled: { + opacity: 0.45 }, abortText: { color: colors.statusAmber, - fontSize: typography.metaSize, + fontSize: typography.bodySize, fontWeight: '600', textTransform: 'capitalize' }, diff --git a/mobile/src/source-control/use-mobile-source-control-loaders.ts b/mobile/src/source-control/use-mobile-source-control-loaders.ts index 5a373ea50..86055d21d 100644 --- a/mobile/src/source-control/use-mobile-source-control-loaders.ts +++ b/mobile/src/source-control/use-mobile-source-control-loaders.ts @@ -104,8 +104,18 @@ export function useMobileSourceControlLoaders(params: Params): MobileSourceContr return false } if (!baseRef) { - setBranchCompareState({ kind: 'idle' }) - return true + // Why: wiping a prior ready compare to idle makes Changes say "No + // Changes" even when commits still exist (e.g. after abort-merge refresh). + setBranchCompareState((prev) => { + if (options?.preserveReadyOnFailure && prev.kind === 'ready') { + return prev + } + return { + kind: 'error', + message: 'Unable to resolve the base branch for comparison.' + } + }) + return false } const response = await client.sendRequest('git.branchCompare', { worktree: `id:${worktreeId}`, @@ -116,7 +126,12 @@ export function useMobileSourceControlLoaders(params: Params): MobileSourceContr } if (!response.ok) { if (isMobileGitUnavailable(response.error?.code, response.error?.message)) { - setBranchCompareState({ kind: 'idle' }) + setBranchCompareState((prev) => { + if (options?.preserveReadyOnFailure && prev.kind === 'ready') { + return prev + } + return { kind: 'idle' } + }) return false } throw new Error(response.error?.message || 'Unable to load committed changes') diff --git a/mobile/src/source-control/use-mobile-source-control-state.ts b/mobile/src/source-control/use-mobile-source-control-state.ts index c9a8ca406..e50525f3c 100644 --- a/mobile/src/source-control/use-mobile-source-control-state.ts +++ b/mobile/src/source-control/use-mobile-source-control-state.ts @@ -15,6 +15,7 @@ import { canOpenMobileBranchCompareDiff, formatMobileBranchCompareSummary } from './mobile-branch-compare' + import { buildMobileSourceControlSections, countStagedEntries, @@ -140,6 +141,7 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara })), [branchCompareCanOpen, branchCompareSection] ) + // Local changes only: dirty files + committed file diffs vs base (not PR/push). const shouldShowBranchCompareSection = branchEntries.length > 0 || branchCompareState.kind === 'loading' ||