diff --git a/mobile/src/components/RightDrawer.tsx b/mobile/src/components/RightDrawer.tsx index ba44822aac5..5c30da7f427 100644 --- a/mobile/src/components/RightDrawer.tsx +++ b/mobile/src/components/RightDrawer.tsx @@ -108,7 +108,11 @@ function MountedRightDrawer({ }, [onHidden, visible]) useEffect(() => { - if (!visible) { + // Native only, ahead of need: the review screen is this drawer's one caller and C4 is what + // serves that route from the page. React Native Web answers `BackHandler.addEventListener` + // with a console warning and an inert subscription, and a WebView has no hardware back to + // intercept; the shell owns the one the phone has. + if (!visible || Platform.OS === 'web') { return } const sub = BackHandler.addEventListener('hardwareBackPress', () => { diff --git a/mobile/src/components/right-drawer-hardware-back.test.tsx b/mobile/src/components/right-drawer-hardware-back.test.tsx new file mode 100644 index 00000000000..8ca2b8f9b1b --- /dev/null +++ b/mobile/src/components/right-drawer-hardware-back.test.tsx @@ -0,0 +1,125 @@ +import type { ReactElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const native = vi.hoisted(() => { + // Annotated rather than asserted: the literal alone narrows to 'ios' and the tests reassign it. + const platform: { os: 'ios' | 'android' | 'web' } = { os: 'ios' } + const remove = vi.fn() + return { + platform, + remove, + addEventListener: vi.fn((_event: string, _handler: () => boolean) => ({ remove })) + } +}) + +vi.mock('react-native', () => ({ + BackHandler: { + addEventListener: (event: string, handler: () => boolean) => + native.addEventListener(event, handler) + }, + Keyboard: { dismiss: () => {} }, + get Platform() { + return { + OS: native.platform.os, + select: (options: Record) => options[native.platform.os] ?? options.default + } + }, + Pressable: 'Pressable', + StyleSheet: { create: (styles: T) => styles, absoluteFillObject: {} }, + View: 'View', + useWindowDimensions: () => ({ width: 390, height: 844 }) +})) +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 62, bottom: 34, left: 0, right: 0 }) +})) +vi.mock('react-native-gesture-handler', () => { + const chain: Record = {} + for (const method of ['activeOffsetX', 'simultaneousWithExternalGesture', 'onUpdate', 'onEnd']) { + chain[method] = () => chain + } + return { + Gesture: { Pan: () => chain, Native: () => chain }, + GestureDetector: 'GestureDetector', + GestureHandlerRootView: 'GestureHandlerRootView' + } +}) +vi.mock('react-native-reanimated', () => ({ + default: { View: 'AnimatedView', ScrollView: 'AnimatedScrollView' }, + useSharedValue: (initial: number) => ({ value: initial }), + useAnimatedStyle: () => ({}), + useAnimatedScrollHandler: () => () => {}, + withSpring: (to: number) => to, + withTiming: (to: number) => to, + runOnJS: (fn: () => void) => fn, + interpolate: () => 0, + Extrapolation: { CLAMP: 'clamp' } +})) + +import { RightDrawer } from './RightDrawer' + +function DrawerBody(): null { + return null +} + +function drawer(visible: boolean): ReactElement { + return ( + {}}> + + + ) +} + +function render(visible: boolean): ReactTestRenderer { + let renderer!: ReactTestRenderer + act(() => { + renderer = create(drawer(visible)) + }) + return renderer +} + +beforeEach(() => { + native.platform.os = 'ios' + native.addEventListener.mockClear() + native.remove.mockClear() +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +/** + * React Native Web logs "BackHandler is not supported on web and should not be used." and hands + * back an inert subscription, so inside the shell's page every open of this drawer put that line on + * the console and armed nothing. There is no hardware back in a WebView; the shell owns the phone's. + */ +describe('the right drawer and the phone hardware back button', () => { + it('arms it on iOS while the drawer is open', () => { + const renderer = render(true) + expect(native.addEventListener).toHaveBeenCalledTimes(1) + expect(native.addEventListener.mock.calls[0]?.[0]).toBe('hardwareBackPress') + act(() => renderer.unmount()) + }) + + it('arms it on Android while the drawer is open', () => { + native.platform.os = 'android' + const renderer = render(true) + expect(native.addEventListener).toHaveBeenCalledTimes(1) + act(() => renderer.unmount()) + }) + + it('releases it when the drawer hides', () => { + const renderer = render(true) + expect(native.remove).not.toHaveBeenCalled() + act(() => renderer.update(drawer(false))) + expect(native.remove).toHaveBeenCalledTimes(1) + act(() => renderer.unmount()) + }) + + it('does not reach for it on web', () => { + native.platform.os = 'web' + const renderer = render(true) + expect(native.addEventListener).not.toHaveBeenCalled() + act(() => renderer.unmount()) + }) +}) diff --git a/mobile/src/host-screen/host-screen-header-control-a11y.test.ts b/mobile/src/host-screen/host-screen-header-control-a11y.test.ts new file mode 100644 index 00000000000..7bce56fa2c6 --- /dev/null +++ b/mobile/src/host-screen/host-screen-header-control-a11y.test.ts @@ -0,0 +1,134 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import ts from 'typescript' +import { describe, expect, it } from 'vitest' +import { + PRESSABLE_TAGS, + readAttribute, + type Read +} from '../mobile-web-shell/pressable-control-source-reader' + +/** + * This header renders two toolbars and the phone sees the narrow one. Its controls carried no role + * and no name, so a screen reader could not find them and C2.9's render check could only assert + * their absence at 390 px. The wide toolbar already names every control, and the name is computed + * from the same state, so the two must agree rather than each invent wording. + * + * A spread reads as unknown rather than absent: a control whose handler or whose accessibility + * props arrive through one is a control this scan cannot judge, so it fails both rules and says so, + * instead of passing quietly or reading as an unnamed control. + */ +const HEADER = 'src/host-screen/host-screen-header.tsx' +const MOBILE_ROOT = join(import.meta.dirname, '..', '..') + +/** + * One entry per control both toolbars render, keyed by the handler it presses, which is what makes + * two elements the same control. Each must be found twice, so the naming rule below always has + * pairs to compare: the rule derives its own groups, and over a file with no repeated handler it + * would hold vacuously. + */ +const SHARED_CONTROLS = [ + '() => state.setShowFilterModal(true)', + '() => state.setShowSortPicker(true)', + '() => state.setShowGroupPicker(true)', + '() => actions.navigateFromHostList(`/h/${encodeURIComponent(hostId)}/accounts`)', + '() => actions.navigateFromHostList(`/h/${encodeURIComponent(hostId)}/tasks`)', + '() => state.setShowSearch((s) => !s)' +] + +type Control = { line: number; press: Read; role: Read; label: Read } + +function headerControls(): Control[] { + const source = ts.createSourceFile( + HEADER, + readFileSync(join(MOBILE_ROOT, HEADER), 'utf8'), + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TSX + ) + const found: Control[] = [] + function visit(node: ts.Node): void { + if (ts.isJsxElement(node) || ts.isJsxSelfClosingElement(node)) { + const element = ts.isJsxElement(node) ? node.openingElement : node + if (PRESSABLE_TAGS.has(element.tagName.getText())) { + const press = readAttribute(element, 'onPress') + // A Pressable with no handler is decoration; one whose handler is spread in is a control. + if (!press.known || press.value !== '') { + found.push({ + line: source.getLineAndCharacterOfPosition(node.getStart(source)).line + 1, + press, + role: readAttribute(element, 'accessibilityRole'), + label: readAttribute(element, 'accessibilityLabel') + }) + } + } + } + ts.forEachChild(node, visit) + } + visit(source) + return found +} + +function show(read: Read): string { + if (!read.known) { + return 'spread' + } + return read.value || 'none' +} + +function describeControl(control: Control): string { + return `${HEADER}:${control.line} press=${show(control.press)} role=${show(control.role)} label=${show( + control.label + )}` +} + +const CONTROLS = headerControls() + +/** Every handler this header presses more than once, with the names its sites give it. */ +function namesByHandler(): Map> { + const groups = new Map>() + for (const control of CONTROLS) { + if (!control.press.known) { + continue + } + const names = groups.get(control.press.value) ?? new Set() + names.add(show(control.label)) + groups.set(control.press.value, names) + } + return groups +} + +describe('host header controls carry a role and a name in both toolbars', () => { + it('finds each shared control in both toolbars, so the naming rule has pairs to compare', () => { + expect( + SHARED_CONTROLS.filter( + (press) => + CONTROLS.filter((control) => control.press.known && control.press.value === press) + .length !== 2 + ) + ).toEqual([]) + }) + + it('gives every pressable control the button role', () => { + expect( + CONTROLS.filter((control) => !control.role.known || control.role.value !== 'button').map( + describeControl + ) + ).toEqual([]) + }) + + it('names every pressable control', () => { + expect( + CONTROLS.filter((control) => !control.label.known || control.label.value === '').map( + describeControl + ) + ).toEqual([]) + }) + + it('names a control the same way wherever this header renders it', () => { + const disagreeing = [...namesByHandler()] + .filter(([, names]) => names.size > 1) + .map(([press, names]) => `${press} -> ${[...names].sort().join(' | ')}`) + expect(disagreeing).toEqual([]) + }) +}) diff --git a/mobile/src/host-screen/host-screen-header.tsx b/mobile/src/host-screen/host-screen-header.tsx index fe6a3b0ab17..3408e1d575c 100644 --- a/mobile/src/host-screen/host-screen-header.tsx +++ b/mobile/src/host-screen/host-screen-header.tsx @@ -79,6 +79,8 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl void forceReconnectHost(hostId!)} + accessibilityRole="button" + accessibilityLabel="Reconnect" hitSlop={8} > Reconnect @@ -267,6 +269,8 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl 0 && styles.filterChipActive]} onPress={() => state.setShowFilterModal(true)} + accessibilityRole="button" + accessibilityLabel={`Filter workspaces${settings.activeFilterCount > 0 ? `, ${settings.activeFilterCount} active` : ''}`} > - state.setShowSortPicker(true)}> + state.setShowSortPicker(true)} + accessibilityRole="button" + accessibilityLabel={`Sort by ${settings.selectedSortLabel}`} + > {settings.selectedSortLabel} - state.setShowGroupPicker(true)}> + state.setShowGroupPicker(true)} + accessibilityRole="button" + accessibilityLabel="Group workspaces" + > {state.groupMode === 'none' @@ -310,6 +324,8 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl actions.navigateFromHostList(`/h/${encodeURIComponent(hostId)}/accounts`) } disabled={connState !== 'connected'} + accessibilityRole="button" + accessibilityLabel="Accounts" > actions.navigateFromHostList(`/h/${encodeURIComponent(hostId)}/tasks`)} disabled={connState !== 'connected'} + accessibilityRole="button" + accessibilityLabel="Tasks" > - state.setShowSearch((s) => !s)}> + state.setShowSearch((s) => !s)} + accessibilityRole="button" + accessibilityLabel={state.showSearch ? 'Close search' : 'Search workspaces'} + > {state.showSearch ? ( ) : ( 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 65c6d241064..c63f836d905 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 @@ -2,6 +2,12 @@ import { readFileSync, readdirSync } from 'node:fs' import { join } from 'node:path' import ts from 'typescript' import { describe, expect, it } from 'vitest' +import { + PRESSABLE_TAGS, + readAttribute, + spreadsProps, + type Read +} from './pressable-control-source-reader' /** * A Back control the page serves is reachable by name or not at all. Inside the shell there is no @@ -54,16 +60,11 @@ const PAGE_SERVED_SCREENS = [ /** The rule reads whole trees, so a Back added beside a screen is ruled as well as the screen's. */ const screenTree = (screen: string): string => screen.slice(0, screen.lastIndexOf('/')) -const PRESSABLE_TAGS = new Set(['Pressable', 'TouchableOpacity']) /** `router.back()`, `goBack()`, `onBack()`; the leading class keeps `callback(` and `rollback(` out. */ const BACK_CALL = /(?:^|[^A-Za-z0-9_$])(?:back|goBack|onBack)\s*\(/ const BACK_HANDLER = /^(?:back|[A-Za-z0-9_$]*Back)$/ const IDENTIFIER = /^[A-Za-z_$][A-Za-z0-9_$]*$/ -/** A spread hides the props this rule reads, so it answers "unknown" rather than "absent". */ -type Read = { known: true; value: string } | { known: false } -const UNKNOWN: Read = { known: false } - type BackControl = { path: string; line: number; role: Read; label: Read } function componentFiles(tree: string): string[] { @@ -79,32 +80,6 @@ function componentFiles(tree: string): string[] { return found } -function spreadsProps(element: ts.JsxOpeningLikeElement): boolean { - return element.attributes.properties.some((property) => ts.isJsxSpreadAttribute(property)) -} - -function readAttribute(element: ts.JsxOpeningLikeElement, name: string): Read { - if (spreadsProps(element)) { - return UNKNOWN - } - for (const property of element.attributes.properties) { - if (ts.isJsxAttribute(property) && property.name.getText() === name) { - const initializer = property.initializer - if (!initializer) { - return { known: true, value: '' } - } - if (ts.isStringLiteral(initializer)) { - return { known: true, value: initializer.text } - } - if (ts.isJsxExpression(initializer) && initializer.expression) { - return { known: true, value: initializer.expression.getText() } - } - return { known: true, value: initializer.getText() } - } - } - return { known: true, value: '' } -} - /** One hop: `onPress={requestBack}` is read through the declaration `requestBack` names here. */ function declarationText(source: ts.SourceFile, name: string): string { let text = '' diff --git a/mobile/src/mobile-web-shell/pressable-control-source-reader.ts b/mobile/src/mobile-web-shell/pressable-control-source-reader.ts new file mode 100644 index 00000000000..20ff030eb56 --- /dev/null +++ b/mobile/src/mobile-web-shell/pressable-control-source-reader.ts @@ -0,0 +1,58 @@ +import ts from 'typescript' + +/** + * Reads what a Pressable's source says about itself, for the accessibility censuses that judge + * controls without rendering them. Shared so the two censuses cannot drift into disagreeing about + * what an unnamed control looks like: the ways of writing "no name" are the easy thing to miss, and + * a census that misses one is green on exactly the regression it exists to catch. + */ +export const PRESSABLE_TAGS = new Set(['Pressable', 'TouchableOpacity']) + +/** A spread hides the props a census reads, so it answers "unknown" rather than "absent". */ +export type Read = { known: true; value: string } | { known: false } +const UNKNOWN: Read = { known: false } + +/** Formatting differs between call sites, so compare what an expression says, not how it wraps. */ +function normalize(source: string): string { + return source.replace(/\s+/g, ' ').trim() +} + +export function spreadsProps(element: ts.JsxOpeningLikeElement): boolean { + return element.attributes.properties.some((property) => ts.isJsxSpreadAttribute(property)) +} + +/** + * The empty reads are the point: `prop`, `prop=""`, `{''}`, `` {``} `` and `{undefined}` are all a + * control with nothing to announce, and only the first two look empty as source text. + */ +function readExpression(expression: ts.Expression): string { + if (ts.isStringLiteral(expression) || ts.isNoSubstitutionTemplateLiteral(expression)) { + return expression.text + } + if (ts.isIdentifier(expression) && expression.text === 'undefined') { + return '' + } + return normalize(expression.getText()) +} + +export function readAttribute(element: ts.JsxOpeningLikeElement, name: string): Read { + if (spreadsProps(element)) { + return UNKNOWN + } + for (const property of element.attributes.properties) { + if (ts.isJsxAttribute(property) && property.name.getText() === name) { + const initializer = property.initializer + if (!initializer) { + return { known: true, value: '' } + } + if (ts.isStringLiteral(initializer)) { + return { known: true, value: initializer.text } + } + if (ts.isJsxExpression(initializer) && initializer.expression) { + return { known: true, value: readExpression(initializer.expression) } + } + return { known: true, value: normalize(initializer.getText()) } + } + } + return { known: true, value: '' } +}