diff --git a/apps/mobile/src/components/agents/new-session-configure-form.test.ts b/apps/mobile/src/components/agents/new-session-configure-form.test.ts index 44d723c5e7..2600e8df43 100644 --- a/apps/mobile/src/components/agents/new-session-configure-form.test.ts +++ b/apps/mobile/src/components/agents/new-session-configure-form.test.ts @@ -188,6 +188,26 @@ function findElementByType(node: Node, typeName: string): Record | null { if (node === null || typeof node !== 'object') { return null; @@ -231,6 +251,14 @@ function findOnLayoutHandler( return null; } +/** The pinned footer's own bottom padding — the form's single clearance source. */ +function findFooterPaddingBottom(node: Node): number | null { + const lift = findElementByType(node, 'AppAwareKeyboardPaddingView'); + const footer = findElementByType(lift?.children as Node, 'View'); + const style = footer?.style as { paddingBottom?: unknown } | undefined; + return typeof style?.paddingBottom === 'number' ? style.paddingBottom : null; +} + const INSTANCE: InstancePickerInstance = { connectionId: 'conn-abc', name: 'laptop', @@ -337,7 +365,7 @@ describe('NewSessionConfigureForm', () => { }); it.each(['android', 'ios'] as const)( - 'clears the navigation bar at the screen root and lifts the body above the IME on %s', + 'floors the pinned footer at the safe-area bottom and lifts it above the IME on %s', async os => { platformState.OS = os; insetsState.bottom = 42; @@ -347,10 +375,9 @@ describe('NewSessionConfigureForm', () => { // eslint-disable-next-line new-cap -- plain function call, matching repo test convention const element = NewSessionConfigureForm({ ...defaultProps() }) as Node; - // The root pads by the safe-area inset, so the pinned Start footer - // (its child) can never render inside the navigation bar's region. - expect(findElementByType(element, 'View')?.style).toEqual({ paddingBottom: 42 }); - // Neither platform resizes the window for the IME, so the body sits + // The helper floors the raw inset at 16 and adds 16: max(42, 16) + 16. + expect(findFooterPaddingBottom(element)).toBe(58); + // Neither platform resizes the window for the IME, so the footer sits // inside a keyboard-lift view that adds the IME height on top of the // safe area — the same implementation on iOS and Android. expect(findElementByType(element, 'AppAwareKeyboardPaddingView')).not.toBeNull(); @@ -377,20 +404,23 @@ describe('NewSessionConfigureForm', () => { expect(scrollBody).not.toBeNull(); expect(findElementByType(scrollBody, 'NewSessionStartButton')).toBeNull(); - // It renders in the footer instead: a sibling of the body inside the - // keyboard-lift view, so it is always on screen and the IME lifts it. + // It renders in the footer instead: a sibling of the body, wrapped in + // the keyboard-lift view, so it is always on screen and the IME lifts it. const liftView = findElement(element, 'AppAwareKeyboardPaddingView'); expect(liftView).not.toBeNull(); - expect(findElementByType(liftView, 'ScrollView')).not.toBeNull(); expect(findElementByType(liftView, 'NewSessionStartButton')).not.toBeNull(); + // The footer alone rides the lift view; the scroll body stays outside it, + // so the IME shrinks the body instead of covering the action. + expect(findElementByType(liftView, 'ScrollView')).toBeNull(); // Below the body, not above it: the pinned bottom bar. - const liftChildren = (liftView?.props as { children?: Node[] } | undefined)?.children ?? []; - const bodyIndex = liftChildren.findIndex( + const rootView = findElement(element, 'View'); + const rootChildren = (rootView?.props as { children?: Node[] } | undefined)?.children ?? []; + const bodyIndex = rootChildren.findIndex( child => (child as { type?: unknown } | undefined)?.type === 'ScrollView' ); - const footerIndex = liftChildren.findIndex( - child => findElementByType(child, 'NewSessionStartButton') !== null + const footerIndex = rootChildren.findIndex( + child => findElementByType(child, 'AppAwareKeyboardPaddingView') !== null ); expect(bodyIndex).toBeGreaterThanOrEqual(0); expect(footerIndex).toBeGreaterThan(bodyIndex); @@ -846,7 +876,70 @@ describe('NewSessionConfigureForm', () => { expect(findTextContent(remote, t => t.includes('`'))).toBe(false); }); - // ── Case 14: reorder wiring lock ── + // ── Case 14: bottom navigation-bar clearance ── + it('reserves the bottom safe-area inset on the pinned footer so Start clears the navigation bar', async () => { + const { NewSessionConfigureForm } = await import('./new-session-configure-form'); + + insetsState.bottom = 44; + try { + // The inset is 44; the helper floors at 16 and adds 16. + // eslint-disable-next-line new-cap -- plain function call, matching repo test convention + const element = NewSessionConfigureForm(defaultProps()) as Node; + + // The footer is the single source: the root adds no raw inset, so the + // padded chain is the floor once, not inset + floor (double padding). + expect(findElementByType(element, 'View')?.style).toBeUndefined(); + expect(findFooterPaddingBottom(element)).toBe(60); + } finally { + insetsState.bottom = 0; + } + }); + + // ── Case 14b: the clearance leaves no dead space in the scroll body ── + it('leaves no in-scroll spacer after the last field', async () => { + const { NewSessionConfigureForm } = await import('./new-session-configure-form'); + + insetsState.bottom = 44; + try { + // eslint-disable-next-line new-cap -- plain function call, matching repo test convention + const element = NewSessionConfigureForm(defaultProps()) as Node; + + const scrollBody = findElementByType(element, 'ScrollView'); + expect(scrollBody).not.toBeNull(); + expect(findElementHeight(scrollBody?.children as Node)).toBeNull(); + } finally { + insetsState.bottom = 0; + } + }); + + // ── Case 14a: the primary action is pinned outside the scroll body ── + it('pins Start and the cloud-create recovery outside the scroll body, under the keyboard lift', async () => { + const { NewSessionConfigureForm } = await import('./new-session-configure-form'); + + // eslint-disable-next-line new-cap -- plain function call, matching repo test convention + const element = NewSessionConfigureForm({ + ...defaultProps(), + cloudCreateError: { retryable: true, message: 'prepare failed' }, + }) as Node; + + // A Start inside the scroll sits below the fold while the composer's + // auto-focus keyboard is up, so the primary action must not live there. + const scrollBody = findElementByType(element, 'ScrollView'); + expect(scrollBody).not.toBeNull(); + expect(findElementByType(scrollBody?.children as Node, 'NewSessionStartButton')).toBeNull(); + expect( + findElementByType(scrollBody?.children as Node, 'NewSessionCloudCreateError') + ).toBeNull(); + + // Both ride the keyboard-lift footer below the body: the lift shrinks the + // body and keeps the action above the IME on either platform. + const lift = findElementByType(element, 'AppAwareKeyboardPaddingView'); + expect(lift).not.toBeNull(); + expect(findElementByType(lift?.children as Node, 'NewSessionStartButton')).not.toBeNull(); + expect(findElementByType(lift?.children as Node, 'NewSessionCloudCreateError')).not.toBeNull(); + }); + + // ── Case 13: reorder wiring lock ── it('wires onMoveAttachment and onReorderAttachments through to NewSessionPrompt', async () => { const { NewSessionConfigureForm } = await import('./new-session-configure-form'); diff --git a/apps/mobile/src/components/agents/new-session-configure-form.tsx b/apps/mobile/src/components/agents/new-session-configure-form.tsx index 7bf5b9f446..dac9dfb8c0 100644 --- a/apps/mobile/src/components/agents/new-session-configure-form.tsx +++ b/apps/mobile/src/components/agents/new-session-configure-form.tsx @@ -1,7 +1,6 @@ import { useState } from 'react'; import { type LayoutChangeEvent, ScrollView, View } from 'react-native'; import { useTranslation } from 'react-i18next'; -import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { LaunchFolderField } from '@/components/agents/folder-selector'; import { NewSessionCloudCreateError } from '@/components/agents/new-session-cloud-create-error'; @@ -17,6 +16,7 @@ import { SegmentedControl } from '@/components/ui/segmented-control'; import { Text } from '@/components/ui/text'; import { stripInlineCodeMarkers } from '@/i18n/plain-copy'; import { remoteSpawnInstanceDisconnectedNote } from '@/lib/remote-submit-outcome'; +import { useDetailScreenBottomPadding } from '@/lib/screen-insets'; /** * THE new-session screen body — one screen for every entry point (cloud, @@ -95,26 +95,38 @@ export function NewSessionConfigureForm({ // edge (rounded corner, top padding, the prompt's first line) comes back // clipped under the header. const composerReveal = useComposerRevealScroll(); + // The pinned footer's single source of bottom clearance: it clears the system + // navigation bar under Start. Without it the primary action can sit in the + // bar's translucent region a formSheet leaves exposed below itself (the + // picker's bottom strip showed its sliver). It rides the footer itself (see + // pr-comment-cta.tsx for the same bar pattern) rather than a spacer inside + // the ScrollView, which the pinned Start no longer needs and which left dead + // space below the last field of a long form. + const bottomClearance = useDetailScreenBottomPadding(); // The form is edge-to-edge and the window never resizes for the IME on - // either platform, so the screen needs two floors: the navigation-bar inset - // — the Start action sits in a footer below the scroll body, and without the - // inset the footer would render in the navigation bar's region (a formSheet - // over this screen no longer leaves that region exposed below itself: the - // sheet is fixed at its shared options, `sheetShouldOverflowTopInset`) — and - // the keyboard height, because the composer auto-focuses on open and without - // the keyboard floor the Start control stays half-hidden behind the keyboard - // strip. The keyboard-lift view is the app's cross-platform IME primitive - // (keyboardDidShow/DidHide on Android, keyboardWillShow/WillHide on iOS), so - // the same implementation runs on both platforms; the footer is its second - // child, so the IME lifts the action too. + // either platform, so the screen needs two floors. The navigation-bar inset + // is the first: the Start action sits in a footer below the scroll body, and + // without the inset the footer would render in the navigation bar's region + // (a formSheet over this screen no longer leaves that region exposed below + // itself: the sheet is fixed at its shared options, + // `sheetShouldOverflowTopInset`). The keyboard height is the second: the + // composer auto-focuses on arrival, and with the keyboard up the scroll body + // is only ~1300 px tall while the form is ~2000 px, so a Start inside the + // scroll sits below the fold — the user had to dismiss the keyboard (a + // scroll drag with `keyboardDismissMode="on-drag"` did that for them) to + // reach the primary action. Start therefore lives in a footer *outside* the + // ScrollView. The keyboard-lift view is the app's cross-platform IME + // primitive (keyboardDidShow/DidHide on Android, keyboardWillShow/WillHide + // on iOS), so the same implementation runs on both platforms; the lift view + // wraps the footer alone, so the IME lifts the action and shrinks the scroll + // body instead of covering Start. // The ScrollView's keyboard-inset adjustment stays on for focused-field // scroll-into-view; it sizes against the scroll view's own frame, which - // already ends above the IME, so the two never stack into a double lift. + // already ends above the footer, so the two never stack into a double lift. // (The picker-sheet sliver of the e1 spot check is fixed at the sheet // triggers: a formSheet anchors over the keyboard that is up at its first // layout and never re-anchors, so the keyboard must be dismissed before // the sheet opens.) - const { bottom } = useSafeAreaInsets(); const isRemote = runOnInstance !== null; // The frame this form scrolls in: the height left once the navigation-bar // inset and the keyboard-lift padding are taken out. `NewSessionPrompt` @@ -261,48 +273,46 @@ export function NewSessionConfigureForm({ ); - return ( - // The root reserves the navigation-bar inset, so the keyboard-lift view - // pads from its own bottom edge and must not add the inset again. - - - {body} - {/* - The primary action is pinned below the scroll body, never part of it. - A Start button inside the form scrolled out of the viewport on a short - screen: only the top of the control stayed visible above the - navigation bar, which read as a button the bottom bar had cut off. - As the keyboard-lift view's second child the footer is always on - screen, clear of the navigation bar, and lifted above the IME. - */} - - {/* - Persistent failure feedback for the cloud create, in the reserved - spot above Start. A retryable rejection carries the retry control; - a terminal one says what the server reported instead. It rides with - the action it answers, so the feedback is on screen wherever the - body is scrolled. The form owns this feedback, so the creator hook - stays silent for it. Cloud-only: the route also clears the failure - when the target changes, and this gate keeps a stale one off a - remote target no matter which path selected it. - */} - {cloudCreateError && !isRemote ? ( - - ) : null} + // Persistent failure feedback for the cloud create, in the reserved spot + // directly above Start. A retryable rejection carries the retry control; a + // terminal one says what the server reported instead. The form owns this + // feedback, so the creator hook stays silent for it. It rides the pinned + // footer with Start so the recovery control is visible with the keyboard up + // too. Cloud-only: the route also clears the failure when the target + // changes, and this gate keeps a stale one off a remote target no matter + // which path selected it. + const footer = ( + + {cloudCreateError && !isRemote ? ( + + ) : null} - - - + + + ); + + // The primary action is pinned below the scroll body, never part of it: a + // Start inside the form scrolled below the fold on a short screen, so only + // the top of the control stayed visible above the navigation bar. The lift + // view wraps the footer alone, so the IME shrinks the body instead of + // covering the action, and Start stays on screen above the navigation bar. + // The footer's own padding already reserves the bottom inset + // (`bottomClearance`), so `contentReservesBottomInset` keeps the + // screen-bottom-anchored occlusion from counting that inset a second time. + return ( + + {body} + {footer} ); } diff --git a/apps/mobile/src/components/ui/segmented-control.tsx b/apps/mobile/src/components/ui/segmented-control.tsx index 16b1b7ec1e..843052c6d5 100644 --- a/apps/mobile/src/components/ui/segmented-control.tsx +++ b/apps/mobile/src/components/ui/segmented-control.tsx @@ -66,7 +66,7 @@ export function SegmentedControl({ selected ? 'font-medium text-foreground' : 'text-muted-foreground' )} // One line per option: a wrapped label makes the two choices - // uneven. Longer locales ellipsize instead of growing a second + // uneven. Longer locales shrink to fit instead of growing a second // line; the radio's accessibilityLabel still carries the full text. > {option.label}