diff --git a/apps/mobile/src/app/(app)/_layout.tsx b/apps/mobile/src/app/(app)/_layout.tsx index b906648f86..ed169ee76b 100644 --- a/apps/mobile/src/app/(app)/_layout.tsx +++ b/apps/mobile/src/app/(app)/_layout.tsx @@ -158,6 +158,15 @@ export default function AppLayout() { headerShown: false, }} /> + ({ back: vi.fn() })); +const slot = vi.hoisted(() => ({ bridge: undefined as BranchPickerBridge | undefined })); + +vi.mock('expo-router', () => ({ + useRouter: () => router, +})); +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + ScrollView: 'ScrollView', + View: 'View', +})); +vi.mock('@/components/picker-sheet', () => ({ + // The fake shell renders the header contract (title + both dismiss + // controls) and the rows below it, so a test can assert the header + // controls and the rows in one tree. + PickerSheet: (props: { + title: string; + onDone: () => void; + onCancel?: () => void; + expired?: boolean; + children?: React.ReactNode; + }) => + createElement( + 'PickerSheet', + { + title: props.title, + expired: props.expired === true, + onCancel: props.onCancel, + onDone: props.onDone, + }, + props.children + ), +})); +vi.mock('@/components/ui/text', async () => { + const React = await import('react'); + return { Text: 'Text', TextClassContext: React.createContext(undefined) }; +}); +vi.mock('@/components/ui/icons', () => ({ Check: 'Check' })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ primary: '#0a84ff' }), +})); +vi.mock('@/lib/route-registry', () => ({ + UNFENCED_ROUTE_KEY: 'unscoped', + useRouteRegistry: vi.fn(), + branchPickerSlot: { + get: () => slot.bridge, + clear: vi.fn(), + }, +})); + +function texts(renderer: TestRenderer.ReactTestRenderer): string[] { + return renderer.root + .findAllByType('Text' as never) + .flatMap(node => node.children) + .filter((child): child is string => typeof child === 'string'); +} + +function branchLabel(branch: string): string { + return i18n.t('agentChat.newSession.branchAccessibility', { label: branch }); +} + +function branchRow(renderer: TestRenderer.ReactTestRenderer, branch: string) { + return renderer.root.findAll( + node => node.props.accessibilityLabel === branchLabel(branch) && typeof node.props.onPress === 'function' + )[0]; +} + +/** Fire a node's `onPress`, the way a tap would. */ +function press(node: TestRenderer.ReactTestInstance | undefined) { + act(() => { + (node?.props.onPress as (() => void) | undefined)?.(); + }); +} + +/** Mount the screen inside act, so i18n's subscription settles inside it. */ +function mount(): TestRenderer.ReactTestRenderer { + const ref: { current: TestRenderer.ReactTestRenderer | null } = { current: null }; + act(() => { + ref.current = TestRenderer.create(createElement(BranchPickerScreen)); + }); + const created = ref.current; + if (created === null) { + throw new Error('the branch picker route did not render'); + } + return created; +} + +function setBridge(overrides: Partial = {}) { + slot.bridge = { + branches: ['main', 'release/2.0'], + defaultBranch: 'main', + selectedBranch: 'main', + onSelect: vi.fn(() => undefined), + ...overrides, + }; +} + +beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + slot.bridge = undefined; + router.back.mockClear(); +}); + +describe('BranchPickerScreen', () => { + it('renders the header shell with both dismiss controls and one row per branch', () => { + setBridge(); + const renderer = mount(); + + const shell = renderer.root.findByType('PickerSheet' as never); + expect(shell.props.title).toBe(i18n.t('agentChat.newSession.branchPickerTitle')); + expect(typeof shell.props.onCancel).toBe('function'); + expect(typeof shell.props.onDone).toBe('function'); + + expect(branchRow(renderer, 'main')).toBeDefined(); + expect(branchRow(renderer, 'release/2.0')).toBeDefined(); + }); + + it('marks the provider default row and the selected row', () => { + setBridge({ selectedBranch: 'release/2.0' }); + const renderer = mount(); + + expect(texts(renderer)).toContain(i18n.t('agentChat.newSession.branchDefault')); + expect(branchRow(renderer, 'release/2.0')?.props.accessibilityState).toEqual({ + selected: true, + }); + expect(branchRow(renderer, 'main')?.props.accessibilityState).toEqual({ selected: false }); + }); + + it('hands the picked branch name back and dismisses', () => { + const onSelect = vi.fn(() => undefined); + setBridge({ onSelect }); + const renderer = mount(); + + press(branchRow(renderer, 'release/2.0')); + + expect(onSelect).toHaveBeenCalledWith('release/2.0'); + expect(router.back).toHaveBeenCalledTimes(1); + }); + + it('hands the default branch name back too — the trigger owns the override decision', () => { + const onSelect = vi.fn(() => undefined); + setBridge({ onSelect }); + const renderer = mount(); + + press(branchRow(renderer, 'main')); + + expect(onSelect).toHaveBeenCalledWith('main'); + }); + + it('dismisses from the header Cancel without reporting a pick', () => { + const onSelect = vi.fn(() => undefined); + setBridge({ onSelect }); + const renderer = mount(); + + const shell = renderer.root.findByType('PickerSheet' as never); + act(() => { + (shell.props.onCancel as () => void)(); + }); + + expect(router.back).toHaveBeenCalledTimes(1); + expect(onSelect).not.toHaveBeenCalled(); + }); + + it('renders the standard expired shell when the slot is gone', () => { + const renderer = mount(); + + const shell = renderer.root.findByType('PickerSheet' as never); + expect(shell.props.expired).toBe(true); + expect(texts(renderer)).not.toContain('main'); + }); +}); diff --git a/apps/mobile/src/app/(app)/agent-chat/branch-picker.tsx b/apps/mobile/src/app/(app)/agent-chat/branch-picker.tsx new file mode 100644 index 0000000000..aed93e8a85 --- /dev/null +++ b/apps/mobile/src/app/(app)/agent-chat/branch-picker.tsx @@ -0,0 +1,81 @@ +import { useRouter } from 'expo-router'; +import { Check } from '@/components/ui/icons'; +import { useState } from 'react'; +import { Pressable, View } from 'react-native'; +import { useTranslation } from 'react-i18next'; + +import { PickerSheet } from '@/components/picker-sheet'; +import { Text } from '@/components/ui/text'; +import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { type BranchPickerBridge } from '@/lib/picker-bridge'; +import { branchPickerSlot, UNFENCED_ROUTE_KEY, useRouteRegistry } from '@/lib/route-registry'; + +/** + * The new-session branch picker, presented as the standard formSheet (same + * shell as the repo/mode/model pickers). The shell's header carries the + * dismiss controls and the rows render below it, so a Cancel control can + * never float over — or drift away from — the branch rows. + */ +export default function BranchPickerScreen() { + const router = useRouter(); + const colors = useThemeColors(); + const { t } = useTranslation(); + useRouteRegistry(UNFENCED_ROUTE_KEY); + // Lazy init reads the slot synchronously on first render — no effect, no + // "Options expired" flash before a later effect populates state. + const [bridge] = useState(() => branchPickerSlot.get(UNFENCED_ROUTE_KEY)); + + function close() { + router.back(); + } + + function handleSelect(picker: BranchPickerBridge, branch: string) { + picker.onSelect(branch); + branchPickerSlot.clear(UNFENCED_ROUTE_KEY); + router.back(); + } + + if (!bridge) { + return ( + + ); + } + + return ( + + + {bridge.branches.map(branch => { + const isSelected = branch === bridge.selectedBranch; + const isDefault = branch === bridge.defaultBranch; + return ( + { + handleSelect(bridge, branch); + }} + > + + {branch} + + {isDefault ? ( + + {t('agentChat.newSession.branchDefault')} + + ) : null} + {isSelected ? : null} + + ); + })} + + + ); +} diff --git a/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx b/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx index 3c9f99b9dc..18f623894b 100644 --- a/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx +++ b/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx @@ -12,15 +12,23 @@ import { setSelectedBranchOverride, } from './new-session-repository-state'; import { type RepositoryBranchesState, useRepositoryBranches } from '@/lib/use-new-session-repos'; +import { branchPickerSlot, UNFENCED_ROUTE_KEY } from '@/lib/route-registry'; +const router = vi.hoisted(() => ({ push: vi.fn() })); +const keyboard = vi.hoisted(() => ({ dismiss: vi.fn() })); + +vi.mock('expo-router', () => ({ + useRouter: () => router, + // The selector only builds an href literal; Href stays a type. +})); vi.mock('react-native', async () => { const React = await import('react'); return { View: 'View', - Modal: 'Modal', Pressable: 'Pressable', - // The real ScrollView scrolls; the fake renders every row so the - // picker's rows are assertable. + Keyboard: keyboard, + // The real ScrollView scrolls; the fake renders every row so a picker's + // rows are assertable. ScrollView: ({ children }: { children?: React.ReactNode }) => React.createElement('ScrollView', {}, children), }; @@ -85,13 +93,6 @@ function mountSelector( return created; } -function texts(renderer: TestRenderer.ReactTestRenderer): string[] { - return renderer.root - .findAllByType('Text' as never) - .flatMap(node => node.children) - .filter((child): child is string => typeof child === 'string'); -} - function pressableWithLabel(renderer: TestRenderer.ReactTestRenderer, label: string) { return renderer.root.findAll( node => node.props.accessibilityLabel === label && typeof node.props.onPress === 'function' @@ -113,23 +114,13 @@ function openPicker(renderer: TestRenderer.ReactTestRenderer, selected: string) press(pressableWithLabel(renderer, branchLabel(selected))); } -/** The picker's row for a branch — the trigger row carries the same label. */ -function pickerRow(renderer: TestRenderer.ReactTestRenderer, branch: string) { - return renderer.root - .findAll( - node => - node.props.accessibilityLabel === branchLabel(branch) && - typeof node.props.onPress === 'function' - ) - .at(-1); -} - beforeEach(() => { // Silences React's "environment is not configured to support act(...)" // warning, the same way the other mounted suites do. (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; vi.clearAllMocks(); resetSelectedBranchOverrides(); + branchPickerSlot.clear(UNFENCED_ROUTE_KEY); }); describe('RepositoryBranchSelector', () => { @@ -146,11 +137,33 @@ describe('RepositoryBranchSelector', () => { expect(texts(renderer)).toContain(i18n.t('agentChat.newSession.branchDefault')); }); + it('opens the standard picker route, publishing the branches to the slot', () => { + const renderer = mountSelector(githubRow); + + openPicker(renderer, 'main'); + + const bridge = branchPickerSlot.get(UNFENCED_ROUTE_KEY); + expect(bridge?.branches).toEqual(['main', 'release/2.0']); + expect(bridge?.defaultBranch).toBe('main'); + expect(bridge?.selectedBranch).toBe('main'); + expect(router.push).toHaveBeenCalledWith('/(app)/agent-chat/branch-picker'); + }); + + it('dismisses the keyboard when the picker opens', () => { + const renderer = mountSelector(githubRow); + + openPicker(renderer, 'main'); + + expect(keyboard.dismiss).toHaveBeenCalled(); + }); + it('records a non-default choice for exactly this repository', () => { const renderer = mountSelector(githubRow); openPicker(renderer, 'main'); - press(pickerRow(renderer, 'release/2.0')); + act(() => { + branchPickerSlot.get(UNFENCED_ROUTE_KEY)?.onSelect('release/2.0'); + }); expect(getSelectedBranchOverride(githubRow)).toBe('release/2.0'); expect(getSelectedBranchOverride({ ...githubRow, platform: 'gitlab' })).toBeNull(); @@ -161,7 +174,9 @@ describe('RepositoryBranchSelector', () => { const renderer = mountSelector(githubRow); openPicker(renderer, 'release/2.0'); - press(pickerRow(renderer, 'main')); + act(() => { + branchPickerSlot.get(UNFENCED_ROUTE_KEY)?.onSelect('main'); + }); expect(getSelectedBranchOverride(githubRow)).toBeNull(); }); @@ -234,7 +249,7 @@ describe('RepositoryBranchSelector', () => { const renderer = mountSelector(githubRow, branchesState({ branches: [], defaultBranch: null })); expect(texts(renderer)).toContain(i18n.t('agentChat.newSession.branchEmpty')); - expect(renderer.root.findAllByType('Modal' as never)).toHaveLength(0); + expect(branchPickerSlot.get(UNFENCED_ROUTE_KEY)).toBeUndefined(); expect(getSelectedBranchOverride(githubRow)).toBeNull(); }); @@ -252,8 +267,12 @@ describe('RepositoryBranchSelector', () => { expect(texts(renderer)).not.toContain(i18n.t('agentChat.newSession.branchDefault')); openPicker(renderer, placeholder); - press(pickerRow(renderer, 'trunk')); + const bridge = branchPickerSlot.get(UNFENCED_ROUTE_KEY); + expect(bridge?.selectedBranch).toBeNull(); + act(() => { + bridge?.onSelect('trunk'); + }); expect(getSelectedBranchOverride(githubRow)).toBe('trunk'); }); @@ -264,6 +283,15 @@ describe('RepositoryBranchSelector', () => { // RN blocks the press itself; the row has to say so to VoiceOver too. expect(trigger?.props.disabled).toBe(true); expect(trigger?.props.accessibilityState).toEqual({ disabled: true }); - expect(renderer.root.findAllByType('Modal' as never)).toHaveLength(0); + press(trigger); + expect(router.push).not.toHaveBeenCalled(); + expect(branchPickerSlot.get(UNFENCED_ROUTE_KEY)).toBeUndefined(); }); }); + +function texts(renderer: TestRenderer.ReactTestRenderer): string[] { + return renderer.root + .findAllByType('Text' as never) + .flatMap(node => node.children) + .filter((child): child is string => typeof child === 'string'); +} diff --git a/apps/mobile/src/components/agents/repository-branch-selector.tsx b/apps/mobile/src/components/agents/repository-branch-selector.tsx index f28cfa5843..201b6f1e3f 100644 --- a/apps/mobile/src/components/agents/repository-branch-selector.tsx +++ b/apps/mobile/src/components/agents/repository-branch-selector.tsx @@ -1,8 +1,9 @@ -import { useState, useSyncExternalStore } from 'react'; -import { Modal, Pressable, ScrollView, View } from 'react-native'; +import { useSyncExternalStore } from 'react'; +import { Keyboard, Pressable, View } from 'react-native'; +import { type Href, useRouter } from 'expo-router'; import { useTranslation } from 'react-i18next'; -import { Check, ChevronDown } from '@/components/ui/icons'; +import { ChevronDown } from '@/components/ui/icons'; import { Button } from '@/components/ui/button'; import { Skeleton } from '@/components/ui/skeleton'; import { Text } from '@/components/ui/text'; @@ -15,6 +16,7 @@ import { } from '@/components/agents/new-session-repository-state'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { useRepositoryBranches } from '@/lib/use-new-session-repos'; +import { branchPickerSlot, UNFENCED_ROUTE_KEY } from '@/lib/route-registry'; import { cn } from '@/lib/utils'; type RepositoryBranchSelectorProps = { @@ -39,6 +41,12 @@ const NOTE_MIN_HEIGHT = 'min-h-12'; * exactly this repository (see `setSelectedBranchOverride`), which * `useNewSessionCreator` sends as `upstreamBranch`. * + * The picker itself is the standard formSheet route (`agent-chat/branch-picker`), + * like the repo/mode/model pickers: an opaque sheet with the dismiss controls + * in its header and the rows below them. The trigger publishes the branches to + * the picker slot and dismisses the keyboard, so the sheet never floats over + * the form translucently or under the keyboard. + * * The interactive states render at the same height as the trigger row, so the * section below never jumps between loading, branches, and a retryable error. * A note row is at least that tall and grows to show its whole message. @@ -48,13 +56,13 @@ export function RepositoryBranchSelector({ disabled, }: Readonly) { const { t } = useTranslation(); + const router = useRouter(); const colors = useThemeColors(); const branches = useRepositoryBranches(repository); const branchState = useSyncExternalStore( subscribeNewSessionBranchState, getNewSessionBranchState ); - const [isPickerOpen, setIsPickerOpen] = useState(false); const handleRetry = branches.retry; if (!repository) { @@ -68,7 +76,6 @@ export function RepositoryBranchSelector({ {t('common.branch')} {renderBody()} - {isPickerOpen ? renderPicker() : null} ); @@ -147,9 +154,7 @@ export function RepositoryBranchSelector({ const isDefault = selectedBranch !== null && selectedBranch === branches.defaultBranch; return ( { - setIsPickerOpen(true); - }} + onPress={openPicker} disabled={disabled} accessibilityRole="button" accessibilityLabel={t('agentChat.newSession.branchAccessibility', { label })} @@ -173,81 +178,23 @@ export function RepositoryBranchSelector({ ); } - function renderPicker() { - const close = () => { - setIsPickerOpen(false); - }; - return ( - - - - { - event.stopPropagation(); - }} - > - - {t('agentChat.newSession.branchPickerTitle')} - - {/* ScrollView, not FlatList: a FlatList stretches to the space - its container offers, so two branch rows rendered as a mostly - empty sheet. A ScrollView hugs its rows and only scrolls once - the card's max height is reached. */} - - {branches.branches.map(branch => renderBranchRow(branch, close))} - - - - - - - - ); - } - - function renderBranchRow(branch: string, close: () => void) { - if (!repository) { - return null; + function openPicker() { + if (!repository || disabled) { + return; } - const isSelected = branch === selectedBranch; - const isDefault = branch === branches.defaultBranch; - return ( - { - // The provider default is stored as "no override", so the create body - // only carries `upstreamBranch` for a real, non-default choice. - setSelectedBranchOverride(repository, isDefault ? null : branch); - close(); - }} - > - - {branch} - - {isDefault ? ( - - {t('agentChat.newSession.branchDefault')} - - ) : null} - {isSelected ? : null} - - ); + // The keyboard belongs to the form; the sheet must not slide up over an + // open keyboard (the form keeps first responder across taps). + Keyboard.dismiss(); + branchPickerSlot.set(UNFENCED_ROUTE_KEY, { + branches: branches.branches, + defaultBranch: branches.defaultBranch, + selectedBranch, + onSelect: branch => { + // The provider default is stored as "no override", so the create body + // only carries `upstreamBranch` for a real, non-default choice. + setSelectedBranchOverride(repository, branch === branches.defaultBranch ? null : branch); + }, + }); + router.push('/(app)/agent-chat/branch-picker' as Href); } } diff --git a/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx b/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx new file mode 100644 index 0000000000..49d78f12a0 --- /dev/null +++ b/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx @@ -0,0 +1,108 @@ +/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (same pattern as pr-diff-hunk-rows.test.tsx) */ +import { createElement } from 'react'; +import TestRenderer, { act } from 'react-test-renderer'; +import { describe, expect, it, vi } from 'vitest'; + +import '@/i18n'; +import { DiffLine } from './diff-line'; +import { type ParsedDiffLine } from '@/lib/pr-review/diff/parse-patch'; + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + Text: 'RNText', + View: 'View', +})); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ + background: '#FFFFFF', + foreground: '#111111', + good: '#0a0', + destructive: '#d00', + mutedForeground: '#777777', + }), +})); + +function line(overrides: Partial = {}): ParsedDiffLine { + return { + type: 'context', + oldLine: 12, + newLine: 12, + text: 'const value = computeSomething(x);', + noNewlineAtEndOfFile: false, + ...overrides, + }; +} + +/** Mount a DiffLine inside act, so subscription updates stay inside it. */ +function mountLine(props: { + line: ParsedDiffLine; + language: string | null; + keyId: string; +}): TestRenderer.ReactTestRenderer { + const ref: { current: TestRenderer.ReactTestRenderer | null } = { current: null }; + act(() => { + ref.current = TestRenderer.create(createElement(DiffLine, props)); + }); + const created = ref.current; + if (created === null) { + throw new Error('the diff line did not render'); + } + return created; +} + +/** The row is the only `flex-row items-stretch` View in a DiffLine. */ +function findRow(renderer: TestRenderer.ReactTestRenderer): TestRenderer.ReactTestInstance { + const rows = renderer.root.findAll( + node => + node.type === ('View' as never) && + typeof node.props.className === 'string' && + node.props.className.includes('flex-row items-stretch') + ); + const [row] = rows; + if (rows.length !== 1 || row === undefined) { + throw new Error(`expected exactly one diff row, found ${rows.length}`); + } + return row; +} + +describe('DiffLine gutter alignment', () => { + // The row is `flex-row items-stretch`, so the gutter View stretches to the + // row's full height. A long code line wraps and makes the row several + // visual lines tall; the line number must sit on the FIRST visual line — + // aligned with the code's first line via the same top padding the code + // container uses — never centered onto a later visual line. + it('aligns the gutter number with the start of the row on a wrapped line', () => { + const renderer = mountLine({ + line: line({ + text: 'const wrappedValue = someVeryLongExpression(thatDoesNotFitOnOneLine, atPhoneWidth) + trailingOperand;', + }), + language: null, + keyId: 'line-12', + }); + + const row = findRow(renderer); + const [gutter, code] = row.props.children as [ + TestRenderer.ReactTestInstance, + TestRenderer.ReactTestInstance, + ]; + + expect(gutter.props.className).toContain('justify-start'); + expect(gutter.props.className).not.toContain('justify-center'); + expect((gutter.props.style as { paddingTop: number }).paddingTop).toBe(2); + // The code container pads by the same amount, so the gutter's first line + // and the code's first visual line share one baseline. + expect((code.props.style as { paddingVertical: number }).paddingVertical).toBe(2); + }); + + it('keeps the same alignment for add and delete rows', () => { + for (const type of ['add', 'del', 'context'] as const) { + const renderer = mountLine({ line: line({ type }), language: null, keyId: `k-${type}` }); + const row = findRow(renderer); + const [gutter] = row.props.children as [ + TestRenderer.ReactTestInstance, + TestRenderer.ReactTestInstance + ]; + expect(gutter.props.className).toContain('justify-start'); + } + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/diff-line.tsx b/apps/mobile/src/components/pr-review/diff/diff-line.tsx index 07001afad6..795ab003fa 100644 --- a/apps/mobile/src/components/pr-review/diff/diff-line.tsx +++ b/apps/mobile/src/components/pr-review/diff/diff-line.tsx @@ -121,6 +121,11 @@ function DiffLineImpl({ line, language, onTap, isSelected }: Readonly - + {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme color + mono font for gutter */} ({ + Text: 'RNText', + View: 'View', +})); +vi.mock('@/components/ui/text', async () => { + const React = await import('react'); + return { Text: 'Text', TextClassContext: React.createContext(undefined) }; +}); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ + background: '#FFFFFF', + foreground: '#111111', + mutedForeground: '#777777', + }), +})); + +function line(overrides: Partial = {}): ParsedDiffLine { + return { + type: 'context', + oldLine: 7, + newLine: 7, + text: 'const value = computeSomething(x);', + noNewlineAtEndOfFile: false, + ...overrides, + }; +} + +function row(overrides: Partial = {}): SideBySideRowData { + return { left: { line: line(overrides) }, right: { line: line(overrides) } }; +} + +function mountRow(data: SideBySideRowData): TestRenderer.ReactTestRenderer { + const ref: { current: TestRenderer.ReactTestRenderer | null } = { current: null }; + act(() => { + ref.current = TestRenderer.create( + createElement(SideBySideRow, { row: data, language: null, rowKeyId: 'row-7' }) + ); + }); + const created = ref.current; + if (created === null) { + throw new Error('the side-by-side row did not render'); + } + return created; +} + +describe('SideBySideRow gutter alignment', () => { + // Same defect class as the unified DiffLine gutter: a wrapped code line + // makes the column several visual lines tall, and a centered number would + // drift onto a later visual line instead of the column's start. + it('top-aligns both column gutters with the code first line', () => { + const renderer = mountRow( + row({ + text: 'const wrappedValue = someVeryLongExpression(thatDoesNotFitOnOneLine, atPhoneWidth);', + }) + ); + + const columns = renderer.root.findAll( + node => + node.type === ('View' as never) && + typeof node.props.className === 'string' && + node.props.className.includes('flex-1 flex-row items-stretch') + ); + expect(columns).toHaveLength(2); + + for (const column of columns) { + const children = column.props.children as TestRenderer.ReactTestInstance[]; + const gutter = children[0]; + if (gutter === undefined) { + throw new Error('the column rendered without a gutter'); + } + expect(gutter.props.className).toContain('justify-start'); + expect(gutter.props.className).not.toContain('justify-center'); + expect((gutter.props.style as { paddingTop: number }).paddingTop).toBe(2); + } + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx index 8dde148dac..31ce207518 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx @@ -90,6 +90,9 @@ function SideColumnImpl({ line, side, language, isDark, foreground }: SideColumn const gutterStyle: ViewStyle = { width: COLUMN_GUTTER_WIDTH, minHeight: metrics.rowMinHeight, + // Top-aligned with the code's first line (see DiffLine's gutter): a + // centered number drifts onto a later visual line when the code wraps. + paddingTop: VERTICAL_PADDING, }; const codeContainerStyle: ViewStyle = { paddingVertical: VERTICAL_PADDING }; const codeBaseStyle: TextStyle = { @@ -114,7 +117,7 @@ function SideColumnImpl({ line, side, language, isDark, foreground }: SideColumn style={rowStyle} > {/* eslint-disable-next-line react-native/no-inline-styles, react-native/no-color-literals -- dynamic theme muted color */} diff --git a/apps/mobile/src/lib/picker-bridge.ts b/apps/mobile/src/lib/picker-bridge.ts index 04dbbe3344..ee13ed4873 100644 --- a/apps/mobile/src/lib/picker-bridge.ts +++ b/apps/mobile/src/lib/picker-bridge.ts @@ -75,6 +75,20 @@ export type RepoPickerBridge = { onSelect: (repo: string) => void; }; +/** + * Bridge for the new-session branch picker. `onSelect` receives the picked + * branch NAME; the trigger's closure decides default-vs-override, so the + * route stays display-only. + */ +export type BranchPickerBridge = { + branches: string[]; + /** The provider's default branch; its row carries the "Default" label. */ + defaultBranch: string | null; + /** The branch the trigger row shows as selected (override or default). */ + selectedBranch: string | null; + onSelect: (branch: string) => void; +}; + /** The complete normalized router row, including all advertised capabilities. */ export type InstancePickerInstance = inferRouterOutputs['activeSessions']['listInstances']['instances'][number]; diff --git a/apps/mobile/src/lib/pr-review/pr-review-connect-gate-view.test.ts b/apps/mobile/src/lib/pr-review/pr-review-connect-gate-view.test.ts index 93dd6a3b1f..a7c7209ada 100644 --- a/apps/mobile/src/lib/pr-review/pr-review-connect-gate-view.test.ts +++ b/apps/mobile/src/lib/pr-review/pr-review-connect-gate-view.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest'; +import { parseProviderPrRoute, providerPrRoutePath } from './provider-pr-ref'; +import { parseProviderPrUrl } from './provider-pr-url'; import { selectPrReviewGateView } from './pr-review-connect-gate-view'; /** A GitHub arm with everything settled and connected. */ @@ -115,4 +117,43 @@ describe('selectPrReviewGateView — Bitbucket', () => { }) ).toBe('org-only'); }); + + /** + * The spot-check defect behind s7's org-only arm: a Bitbucket PR link + * opened in the personal scope must reach the org-only explanation, never + * the invalid-route state. This binds the full in-app chain — the URL + * every entry point parses (badge, paste, recents), the route it pushes, + * the layout's own re-parse of that route, and the gate's decision. + */ + it('a Bitbucket PR link lands on the org-only gate in the personal scope', () => { + const url = 'https://bitbucket.org/workspace/repo/pull-requests/12'; + const ref = parseProviderPrUrl(url); + if (ref === null) { + throw new Error('the Bitbucket URL did not parse'); + } + const route = providerPrRoutePath(ref); + expect(route).toBe('/(app)/pr-review/bitbucket/workspace/repo/12'); + + const reparsed = parseProviderPrRoute({ + platform: 'bitbucket', + identity: ['workspace', 'repo', '12'], + }); + if (reparsed === null) { + throw new Error('the pushed route did not re-parse'); + } + expect(reparsed.platform).toBe('bitbucket'); + + // The personal scope (organizationId null) is the terminal org-only + // state — the gate never falls through to a retryable error or Connect. + expect( + selectPrReviewGateView({ + platform: 'bitbucket', + isError: false, + isLoading: true, + connected: false, + revoked: false, + organizationId: null, + }) + ).toBe('org-only'); + }); }); diff --git a/apps/mobile/src/lib/route-registry.ts b/apps/mobile/src/lib/route-registry.ts index 0223dea6d3..f77054d0a1 100644 --- a/apps/mobile/src/lib/route-registry.ts +++ b/apps/mobile/src/lib/route-registry.ts @@ -11,6 +11,7 @@ import { useEffect } from 'react'; import { type DiffSelection } from './pr-review/diff-selection-bridge'; import { + type BranchPickerBridge, type FolderPickerBridge, type InstancePickerBridge, type ModelPickerBridge, @@ -47,6 +48,7 @@ type SlotValue = { modelPicker: ModelPickerBridge; modePicker: ModePickerBridge; repoPicker: RepoPickerBridge; + branchPicker: BranchPickerBridge; instancePicker: InstancePickerBridge; folderPicker: FolderPickerBridge; prFileNav: Set; @@ -62,6 +64,7 @@ const slots: RegistrySlots = { modelPicker: new Map(), modePicker: new Map(), repoPicker: new Map(), + branchPicker: new Map(), instancePicker: new Map(), folderPicker: new Map(), prFileNav: new Map>(), @@ -73,6 +76,7 @@ const ALL_SLOT_KINDS: readonly SlotKind[] = [ 'modelPicker', 'modePicker', 'repoPicker', + 'branchPicker', 'instancePicker', 'folderPicker', 'prFileNav', @@ -106,6 +110,7 @@ function createSlot(kind: K): RouteSlot { export const modelPickerSlot = createSlot('modelPicker'); export const modePickerSlot = createSlot('modePicker'); export const repoPickerSlot = createSlot('repoPicker'); +export const branchPickerSlot = createSlot('branchPicker'); export const instancePickerSlot = createSlot('instancePicker'); export const folderPickerSlot = createSlot('folderPicker'); export const prFileNavSlot = createSlot('prFileNav');