Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 106 additions & 13 deletions apps/mobile/src/components/agents/new-session-configure-form.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,26 @@ function findElementByType(node: Node, typeName: string): Record<string, unknown
return null;
}

/** Height of the first node carrying an explicit `style.height` (an in-scroll spacer). */
function findElementHeight(node: Node): number | null {
if (node === null || typeof node !== 'object') {
return null;
}
const props = node.props ?? {};
const style = props.style as { height?: unknown } | undefined;
if (typeof style?.height === 'number') {
return style.height;
}
const children = props.children;
for (const child of Array.isArray(children) ? children : [children]) {
const found = findElementHeight(child as Node);
if (found !== null) {
return found;
}
}
return null;
}

function findElement(node: Node, typeName: string): Record<string, unknown> | null {
if (node === null || typeof node !== 'object') {
return null;
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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');

Expand Down
120 changes: 65 additions & 55 deletions apps/mobile/src/components/agents/new-session-configure-form.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -261,48 +273,46 @@ export function NewSessionConfigureForm({
</ScrollView>
);

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.
<View className="flex-1 bg-background" style={{ paddingBottom: bottom }}>
<AppAwareKeyboardPaddingView className="flex-1" containerReservesBottomInset>
{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.
*/}
<View className="px-4 pb-4">
{/*
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 ? (
<NewSessionCloudCreateError
failure={cloudCreateError}
onRetry={onRetryCloudCreate}
isRetryDisabled={isStartDisabled}
/>
) : 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 = (
<View className="bg-background px-4 pt-3" style={{ paddingBottom: bottomClearance }}>
{cloudCreateError && !isRemote ? (
<NewSessionCloudCreateError
failure={cloudCreateError}
onRetry={onRetryCloudCreate}
isRetryDisabled={isStartDisabled}
/>
) : null}

<NewSessionStartButton
isCloneEntry={isCloneEntry}
isRemote={isRemote}
isStartDisabled={isStartDisabled}
isStarting={isStarting}
onStartSession={onStartSession}
/>
</View>
</AppAwareKeyboardPaddingView>
<NewSessionStartButton
isCloneEntry={isCloneEntry}
isRemote={isRemote}
isStartDisabled={isStartDisabled}
isStarting={isStarting}
onStartSession={onStartSession}
/>
</View>
);

// 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 (
<View className="flex-1 bg-background">
{body}
<AppAwareKeyboardPaddingView contentReservesBottomInset>{footer}</AppAwareKeyboardPaddingView>
</View>
);
}
2 changes: 1 addition & 1 deletion apps/mobile/src/components/ui/segmented-control.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function SegmentedControl<T extends string>({
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}
Expand Down
Loading