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.
2026-07-10 23:04:38 -07:00 · 2026-07-10 23:04:38 -07:00 · 026516f323
parent 5b1fcb0ff0
commit 026516f323
6 changed files with 195 additions and 132 deletions
--- 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 (
-    <Animated.View
-      pointerEvents={visible ? 'auto' : 'none'}
-      style={[styles.overlay, { zIndex, elevation: zIndex }]}
-      accessibilityViewIsModal
-      aria-modal
-    >
-      <GestureHandlerRootView style={styles.root}>
-        <Animated.View style={[styles.backdrop, backdropStyle]}>
-          <Pressable style={StyleSheet.absoluteFill} onPress={dismiss} />
-        </Animated.View>
-
-        <View style={[styles.anchor, isWideLayout && styles.anchorWide]} pointerEvents="box-none">
-          <Animated.View
-            style={[
-              styles.drawer,
-              {
-                width: '100%',
-                maxWidth: isWideLayout ? modalMaxWidth : undefined,
-                maxHeight: screenHeight - insets.top - spacing.lg,
-                paddingBottom: insets.bottom + spacing.lg
-              },
-              drawerStyle
-            ]}
-          >
-            {!contentScrollable ? (
-              <>
-                <GestureDetector gesture={handlePanGesture}>
-                  <Animated.View
-                    style={styles.handleHitArea}
-                    accessibilityRole="button"
-                    accessibilityLabel="Dismiss drawer"
-                  >
-                    <View style={styles.handle} />
-                  </Animated.View>
-                </GestureDetector>
-                <View style={styles.staticContent}>{children}</View>
-              </>
-            ) : dragContentToDismiss ? (
-              <>
-                <GestureDetector gesture={handlePanGesture}>
-                  <Animated.View
-                    style={styles.handleHitArea}
-                    accessibilityRole="button"
-                    accessibilityLabel="Dismiss drawer"
-                  >
-                    <View style={styles.handle} />
-                  </Animated.View>
-                </GestureDetector>
-                <GestureDetector gesture={contentPanGesture}>
-                  <Animated.View collapsable={false}>
-                    <GestureDetector gesture={scrollGesture}>
-                      <Animated.ScrollView
-                        bounces={false}
-                        keyboardShouldPersistTaps="handled"
-                        onScroll={scrollHandler}
-                        scrollEventThrottle={16}
-                        showsVerticalScrollIndicator={false}
-                      >
-                        {children}
-                      </Animated.ScrollView>
-                    </GestureDetector>
-                  </Animated.View>
-                </GestureDetector>
-              </>
-            ) : (
-              <>
-                <GestureDetector gesture={handlePanGesture}>
-                  <Animated.View
-                    style={styles.handleHitArea}
-                    accessibilityRole="button"
-                    accessibilityLabel="Dismiss drawer"
-                  >
-                    <View style={styles.handle} />
-                  </Animated.View>
-                </GestureDetector>
-                <ScrollView
-                  bounces={false}
-                  keyboardShouldPersistTaps="handled"
-                  showsVerticalScrollIndicator={false}
-                >
-                  {children}
-                </ScrollView>
-              </>
-            )}
-            <View style={styles.bottomExtension} />
+    <Modal visible transparent animationType="none" statusBarTranslucent onRequestClose={dismiss}>
+      <Animated.View
+        pointerEvents={visible ? 'auto' : 'none'}
+        style={[styles.overlay, { zIndex, elevation: zIndex }]}
+        accessibilityViewIsModal
+        aria-modal
+      >
+        <GestureHandlerRootView style={styles.root}>
+          <Animated.View style={[styles.backdrop, backdropStyle]}>
+            <Pressable style={StyleSheet.absoluteFill} onPress={dismiss} />
          </Animated.View>
-        </View>
-      </GestureHandlerRootView>
-    </Animated.View>
+
+          <View style={[styles.anchor, isWideLayout && styles.anchorWide]} pointerEvents="box-none">
+            <Animated.View
+              style={[
+                styles.drawer,
+                {
+                  width: '100%',
+                  maxWidth: isWideLayout ? modalMaxWidth : undefined,
+                  maxHeight: screenHeight - insets.top - spacing.lg,
+                  paddingBottom: insets.bottom + spacing.lg
+                },
+                drawerStyle
+              ]}
+            >
+              {!contentScrollable ? (
+                <>
+                  <GestureDetector gesture={handlePanGesture}>
+                    <Animated.View
+                      style={styles.handleHitArea}
+                      accessibilityRole="button"
+                      accessibilityLabel="Dismiss drawer"
+                    >
+                      <View style={styles.handle} />
+                    </Animated.View>
+                  </GestureDetector>
+                  <View style={styles.staticContent}>{children}</View>
+                </>
+              ) : dragContentToDismiss ? (
+                <>
+                  <GestureDetector gesture={handlePanGesture}>
+                    <Animated.View
+                      style={styles.handleHitArea}
+                      accessibilityRole="button"
+                      accessibilityLabel="Dismiss drawer"
+                    >
+                      <View style={styles.handle} />
+                    </Animated.View>
+                  </GestureDetector>
+                  <GestureDetector gesture={contentPanGesture}>
+                    <Animated.View collapsable={false}>
+                      <GestureDetector gesture={scrollGesture}>
+                        <Animated.ScrollView
+                          bounces={false}
+                          keyboardShouldPersistTaps="handled"
+                          onScroll={scrollHandler}
+                          scrollEventThrottle={16}
+                          showsVerticalScrollIndicator={false}
+                        >
+                          {children}
+                        </Animated.ScrollView>
+                      </GestureDetector>
+                    </Animated.View>
+                  </GestureDetector>
+                </>
+              ) : (
+                <>
+                  <GestureDetector gesture={handlePanGesture}>
+                    <Animated.View
+                      style={styles.handleHitArea}
+                      accessibilityRole="button"
+                      accessibilityLabel="Dismiss drawer"
+                    >
+                      <View style={styles.handle} />
+                    </Animated.View>
+                  </GestureDetector>
+                  <ScrollView
+                    bounces={false}
+                    keyboardShouldPersistTaps="handled"
+                    showsVerticalScrollIndicator={false}
+                  >
+                    {children}
+                  </ScrollView>
+                </>
+              )}
+              <View style={styles.bottomExtension} />
+            </Animated.View>
+          </View>
+        </GestureHandlerRootView>
+      </Animated.View>
+    </Modal>
  )
 }

--- a/mobile/src/source-control/MobileSourceControlBranchCard.tsx
+++ b/mobile/src/source-control/MobileSourceControlBranchCard.tsx
@ -55,23 +55,28 @@ export function MobileSourceControlBranchCard({
        <Text style={styles.countText}>{unstagedCount} changed</Text>
        <Text style={styles.countText}>{stagedCount} staged</Text>
        {branchCount > 0 ? <Text style={styles.countText}>{branchCount} on branch</Text> : null}
-        {showConflict ? (
-          <View style={styles.conflictRow}>
-            <Text style={styles.conflictText}>{conflictOperation}</Text>
-            {conflictOperation === 'merge' || conflictOperation === 'rebase' ? (
-              <Pressable
-                style={({ pressed }) => [styles.abortButton, pressed && styles.abortPressed]}
-                disabled={conflictBusy}
-                onPress={() => onAbortConflict(conflictOperation)}
-              >
-                <Text style={styles.abortText}>
-                  {mobileConflictAbortLabel(conflictOperation, conflictAborting)}
-                </Text>
-              </Pressable>
-            ) : null}
-          </View>
-        ) : null}
      </View>
+      {/* Own row so Abort never overflows past the card when counts are long. */}
+      {showConflict ? (
+        <View style={styles.conflictRow}>
+          <Text style={styles.conflictText}>{conflictOperation}</Text>
+          {conflictOperation === 'merge' || conflictOperation === 'rebase' ? (
+            <Pressable
+              style={({ pressed }) => [
+                styles.abortButton,
+                conflictBusy && styles.abortButtonDisabled,
+                pressed && !conflictBusy && styles.abortPressed
+              ]}
+              disabled={conflictBusy}
+              onPress={() => onAbortConflict(conflictOperation)}
+            >
+              <Text style={styles.abortText}>
+                {mobileConflictAbortLabel(conflictOperation, conflictAborting)}
+              </Text>
+            </Pressable>
+          ) : null}
+        </View>
+      ) : null}
      {prChip ? <MobileSourceControlPrChip summary={prChip} onPress={onOpenPr} /> : null}
    </View>
  )
--- 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 = (
+    <BranchCompareFooter
+      state={{
+        shouldShowBranchCompareSection: state.shouldShowBranchCompareSection,
+        branchCompareSummaryText: state.branchCompareSummaryText,
+        branchEntries: state.branchEntries,
+        branchCompareState: state.branchCompareState,
+        branchCompareResult: state.branchCompareResult,
+        busyAction,
+        openBranchDiff,
+        openingBranchPath,
+        openingPath
+      }}
+    />
+  )

  return (
    <>
@ -126,9 +150,15 @@ export function MobileSourceControlContent({ state }: Props) {

      {!hasVisibleChanges ? (
        <View style={styles.state}>
-          <Text style={styles.stateTitle}>No Changes</Text>
+          <Text style={styles.stateTitle}>No local changes</Text>
          <Text style={styles.stateText}>Working tree is clean.</Text>
        </View>
+      ) : sections.length === 0 ? (
+        // Why: RN SectionList with empty `sections` often skips ListFooterComponent,
+        // which hid "Committed on Branch" when only branch files remain.
+        <ScrollView style={hubStyles.tabBody} contentContainerStyle={styles.listContent}>
+          {branchCompareFooter}
+        </ScrollView>
      ) : (
        <SectionList
          style={hubStyles.tabBody}
@ -148,21 +178,7 @@ export function MobileSourceControlContent({ state }: Props) {
              <Text style={styles.sectionCount}>{section.data.length}</Text>
            </View>
          )}
-          ListFooterComponent={
-            <BranchCompareFooter
-              state={{
-                shouldShowBranchCompareSection: state.shouldShowBranchCompareSection,
-                branchCompareSummaryText: state.branchCompareSummaryText,
-                branchEntries: state.branchEntries,
-                branchCompareState: state.branchCompareState,
-                branchCompareResult: state.branchCompareResult,
-                busyAction,
-                openBranchDiff,
-                openingBranchPath,
-                openingPath
-              }}
-            />
-          }
+          ListFooterComponent={branchCompareFooter}
          stickySectionHeadersEnabled={false}
          contentContainerStyle={styles.listContent}
        />
--- 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'
  },
--- 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')
--- 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' ||