diff --git a/mobile/src/components/CustomKeyModal.tsx b/mobile/src/components/CustomKeyModal.tsx index 0cc24f4c030..598b17c6905 100644 --- a/mobile/src/components/CustomKeyModal.tsx +++ b/mobile/src/components/CustomKeyModal.tsx @@ -191,6 +191,7 @@ export function CustomKeyModal({ visible, onClose, onKeysChanged, onManageShortc [styles.backButton, pressed && styles.backButtonPressed]} onPress={onBack} + accessibilityRole="button" accessibilityLabel="Back" > diff --git a/mobile/src/mobile-web-shell/page-served-back-control-a11y.test.ts b/mobile/src/mobile-web-shell/page-served-back-control-a11y.test.ts index c63f836d905..916cf4917bd 100644 --- a/mobile/src/mobile-web-shell/page-served-back-control-a11y.test.ts +++ b/mobile/src/mobile-web-shell/page-served-back-control-a11y.test.ts @@ -204,3 +204,48 @@ describe('Back controls in the screens the page serves', () => { ).toEqual([]) }) }) + +/** + * The trees a route not yet registered will bring under the rule, held to it before it does. + * + * `screenTree` takes a screen's directory, so registering the review route puts the whole of + * `src/components` under these two rules and the source-control route puts `src/source-control`. + * Fixing that in the PR that registers would make a route entry carry unrelated accessibility + * work; fixing it here means C4.4 adds two rows to the table above and nothing else moves. + * + * This describe is what the rows replace: once they are in `PAGE_SERVED_SCREENS`, `CONTROLS` + * covers these trees and the cases below become a second reading of the same thing. + * + * The modules, with their trees derived, for the reason the table above gives per screen: a tree + * holds more than one Back, so presence asserted over the tree lets one answer for another. + * `src/components` has two, and the review header's could have been renamed into a dismiss with + * `CustomKeyModal`'s standing in for it. + */ +const ARRIVING_SCREENS = [ + 'src/source-control/MobileSourceControlHeader.tsx', + 'src/components/MobileDiffReviewHeader.tsx' +] +const ARRIVING_TREES = [...new Set(ARRIVING_SCREENS.map(screenTree))] +const ARRIVING = ARRIVING_TREES.flatMap((tree) => backControlsUnder(tree)) + +describe('Back controls in the trees a registered route will add', () => { + it('finds a control in each arriving screen, so the rules below cannot pass vacuously', () => { + expect(ARRIVING_SCREENS.filter((screen) => backControlsIn(screen).length === 0)).toEqual([]) + }) + + it('gives every one of them the button role', () => { + expect( + ARRIVING.filter((control) => !control.role.known || control.role.value !== 'button').map( + describeControl + ) + ).toEqual([]) + }) + + it('names every one of them in the app’s own wording for Back', () => { + expect( + ARRIVING.filter( + (control) => !control.label.known || !/^Back\b/.test(control.label.value) + ).map(describeControl) + ).toEqual([]) + }) +}) diff --git a/mobile/src/source-control/MobileSourceControlHeader.test.ts b/mobile/src/source-control/MobileSourceControlHeader.test.ts new file mode 100644 index 00000000000..152578d3874 --- /dev/null +++ b/mobile/src/source-control/MobileSourceControlHeader.test.ts @@ -0,0 +1,85 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { MobileSourceControlHeader } from './MobileSourceControlHeader' + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + Text: 'Text', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'ChevronLeft', + ExternalLink: 'ExternalLink', + RefreshCw: 'RefreshCw', + X: 'X' +})) + +vi.mock('./mobile-source-control-styles', () => ({ styles: new Proxy({}, { get: () => ({}) }) })) + +/** + * The dock closes and the route goes back, and only one of them is a Back control. + * + * Nothing else pins this: no golden names this component and no parity census covers + * `src/source-control`. The Back census reads what a control is called, so a single control + * serving both modes has to be named "Back" in a mode where it dismisses a dock — which satisfies + * the rule by making the wording wrong. + */ +describe('the source-control header control', () => { + let tree: ReactTestRenderer | null = null + + afterEach(() => { + act(() => tree?.unmount()) + tree = null + }) + + function render(embedded: boolean, handlers: { onBack: () => void; onClose: () => void }) { + let rendered: ReactTestRenderer | null = null + act(() => { + rendered = create( + createElement(MobileSourceControlHeader, { + embedded, + worktreeLabel: 'wt', + ioBusy: false, + onRefresh: () => {}, + ...handlers + }) + ) + }) + if (rendered === null) { + throw new Error('the header did not render') + } + tree = rendered + return rendered + } + + const labelled = (rendered: ReactTestRenderer, label: string) => + rendered.root.findAll((node) => node.props.accessibilityLabel === label) + + it('goes back from the route, named as a Back and pressing onBack', () => { + const calls: string[] = [] + const rendered = render(false, { + onBack: () => calls.push('back'), + onClose: () => calls.push('close') + }) + const control = labelled(rendered, 'Back to session').at(0) + expect(control?.props.accessibilityRole).toBe('button') + act(() => control?.props.onPress()) + expect(calls).toEqual(['back']) + expect(labelled(rendered, 'Close source control')).toHaveLength(0) + }) + + it('closes the dock when embedded, named as a Close and pressing onClose', () => { + const calls: string[] = [] + const rendered = render(true, { + onBack: () => calls.push('back'), + onClose: () => calls.push('close') + }) + const control = labelled(rendered, 'Close source control').at(0) + expect(control?.props.accessibilityRole).toBe('button') + act(() => control?.props.onPress()) + expect(calls).toEqual(['close']) + expect(labelled(rendered, 'Back to session')).toHaveLength(0) + }) +}) diff --git a/mobile/src/source-control/MobileSourceControlHeader.tsx b/mobile/src/source-control/MobileSourceControlHeader.tsx index 4485f2ac803..26bca973539 100644 --- a/mobile/src/source-control/MobileSourceControlHeader.tsx +++ b/mobile/src/source-control/MobileSourceControlHeader.tsx @@ -7,7 +7,10 @@ type Props = { embedded: boolean worktreeLabel: string ioBusy: boolean + /** Pops the route. The embedded dock does not pop anything, which is why `onClose` is separate. */ onBack: () => void + /** Dismisses the dock beside the terminal, which is a close and must not be called Back. */ + onClose: () => void onRefresh: () => void // When set (PR segment ready with a host URL), show open-on-web flush-right of // the title so the control stays visible while the PR body scrolls. @@ -20,24 +23,36 @@ export function MobileSourceControlHeader({ worktreeLabel, ioBusy, onBack, + onClose, onRefresh, onOpenPrWeb, prNumber = null }: Props) { return ( - [styles.backButton, pressed && styles.backButtonPressed]} - onPress={onBack} - hitSlop={8} - accessibilityLabel={embedded ? 'Close source control' : 'Back to session'} - > - {embedded ? ( + {/* Two controls, not one with a conditional label: the dock's dismiss is a close, and a + single control serving both modes has to be named Back in a mode where it closes. */} + {embedded ? ( + [styles.backButton, pressed && styles.backButtonPressed]} + onPress={onClose} + hitSlop={8} + accessibilityRole="button" + accessibilityLabel="Close source control" + > - ) : ( + + ) : ( + [styles.backButton, pressed && styles.backButtonPressed]} + onPress={onBack} + hitSlop={8} + accessibilityRole="button" + accessibilityLabel="Back to session" + > - )} - + + )} Source Control @@ -70,6 +85,7 @@ export function MobileSourceControlHeader({ onPress={onRefresh} disabled={ioBusy} hitSlop={8} + accessibilityRole="button" accessibilityLabel="Refresh source control" > diff --git a/mobile/src/source-control/MobileSourceControlPanel.tsx b/mobile/src/source-control/MobileSourceControlPanel.tsx index 3244d440b01..b49d56d0922 100644 --- a/mobile/src/source-control/MobileSourceControlPanel.tsx +++ b/mobile/src/source-control/MobileSourceControlPanel.tsx @@ -214,8 +214,11 @@ export function MobileSourceControlPanel({ void refetchPr({ includeDetails: false }) }, [activeTab, isHostedRepo, loadStatus, refetchPr]) - // Embedded mode docks beside the terminal: close the dock instead of popping a route; skip safe-area chrome (the dock column owns it). - const onBack = embedded ? (onRequestClose ?? (() => router.back())) : () => router.back() + // Embedded mode docks beside the terminal: close the dock instead of popping a route; skip + // safe-area chrome (the dock column owns it). Two handlers rather than one chosen by mode, + // because the header renders a Close or a Back and they are not the same control. + const onBack = () => router.back() + const onClose = onRequestClose ?? (() => router.back()) // Chromeless PR body has no header, so surface open-on-web on the hub chrome while the PR segment is active. const prWebUrl = activeTab === 'pr' && @@ -231,6 +234,7 @@ export function MobileSourceControlPanel({ worktreeLabel={worktreeLabel} ioBusy={ioBusy} onBack={onBack} + onClose={onClose} onRefresh={onRefresh} onOpenPrWeb={prWebUrl ? () => openMobilePrUrl(prWebUrl) : undefined} prNumber={prWebNumber}