fix(mobile): name the narrow host header's controls and gate the drawer's hardware back on web (OTA phase C, C2.10) (#21729)

* fix(mobile): name the narrow host header's controls

The header renders two toolbars and the phone sees the narrow one,
whose controls carried neither a role nor a name. 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 from the same
state, so the fix is to say the same thing rather than invent wording:
filter, sort, group, accounts, tasks and the search toggle take their
wide sibling's role and label expression verbatim.

The census names a seventh site the plan did not: the Reconnect button
in the status bar above both toolbars, which is shared rather than
narrow and has no wide sibling. It announces through its Text child
today, so it takes the role and the string it already renders.

No layout, style, handler or order changed; the diff is accessibility
props only.

The census parses the file with the TypeScript API and rules that every
Pressable carrying an onPress has a button role and a name, and that a
control both toolbars render is named the same way in both. It keys the
pairing on the handler, because that is what makes two elements the
same control, and asserts each shared handler is found exactly twice,
so a control deleted from one toolbar cannot leave the naming rule
comparing a group of one with itself. Red first, naming all seven
sites by path and line.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): stop the right drawer arming hardware back on web

React Native Web logs "BackHandler is not supported on web and should
not be used." and returns an inert subscription, so inside the shell's
page every open of this drawer put that line on the console and armed
nothing. The gate is the one mounted-bottom-drawer and the file preview
already carry, with the same comment stating the degradation: there is
no hardware back in a WebView, and the shell owns the one the phone has.

The drawer had no render test. This one mocks react-native, the safe
area, gesture handler and Reanimated the way the bottom drawer's
hand-back test does, and reads the call rather than the console: on iOS
and on Android the handler is registered once for 'hardwareBackPress'
and released when the drawer hides, and on web it is never reached. The
three native cases are the control that keeps the web case honest; they
passed before the fix, which is what makes the single red meaningful.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): type the right drawer test's element helper

The tests-typecheck ratchet was red on the previous commit: the drawer's
props declare `children` as required, so passing it as createElement's
third argument left no overload matching. It is a prop here, and the
helper answers a ReactElement rather than a return type borrowed from
createElement.

Test files sit outside `tsc --noEmit`, so only the ratchet sees this;
it is the gate that exists because a type-level pin in an unchecked
test proves nothing.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): write the right drawer test in JSX

Lint was red on the previous commit and I ran it in the same command as
the commit, so it landed: passing `children` as a prop to satisfy the
type checker is exactly what react(no-children-prop) refuses. The
canonical form settles both, so the test is JSX in a .tsx file and the
drawer takes its body as a child again. The StyleSheet mock's generic
needs the trailing comma a .tsx file requires.

Re-proved in this form: with the web gate removed the web case fails
and the three native cases still pass.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): derive the header's naming groups, and read a spread as unknown

Round 1, four folds.

The drawer's comment claimed the console line was observed inside the
shell's page. It was not: the drawer's one caller is the review screen,
whose route C4 serves, so no page closure reaches it today. The gate is
pre-emptive and now says so in its own words rather than borrowing the
bottom drawer's sentence.

The naming rule iterated a hand-written list, so it only ever compared
the six controls both toolbars render. Giving the two
`actions.openFloatingWorkspace` sites different labels left the census
green. The groups are derived from the discovered controls now, keyed
by the handler text, so any handler this header presses from more than
one place is compared and the failure prints both names. The declared
list stays as the precondition it always was: each of the six is found
exactly twice, which is what keeps the derived rule from holding
vacuously over a file with no repeated handler.

The scan read `Pressable` only and dropped any control whose `onPress`
read as empty text, which is what a spread reads as. It reads
`TouchableOpacity` too now, and a spread answers unknown rather than
absent: a control whose handler or whose accessibility props arrive
through one is kept, fails both rules, and prints `spread` rather than
`none`, so it can never be mistaken for a control the scan judged.

Red first on all four: the reviewer's disagreeing-label mutation, a
spread over the a11y props, a spread over the handler, an unlabelled
TouchableOpacity, and a shared control deleted from one toolbar.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): read a braced-empty label as unnamed, from one shared reader

Round 2, two folds.

Only a bare `""` or an omitted attribute read as unnamed, so
`accessibilityLabel={undefined}`, `{''}` and an empty template all left
a control with nothing to announce and the census green. Reproduced on
both Tasks sites with each of the three shapes before the fix. The
reader unwraps a braced expression now: a string or a no-substitution
template answers its own text, and the identifier `undefined` answers
empty, so all three read as unnamed.

That reader was a near-verbatim copy in both censuses, which is how one
of them could have gained this rule and the other kept the hole. It
lives in one module under mobile-web-shell now, named for what it reads
and typechecked by mobile tsc rather than by the ratchet alone. Both
censuses import it and neither changed an assertion; their diffs are
the deleted copies and the import.

Red first, five mutations: the three empty shapes on both Tasks sites
here, and `{undefined}` and `{''}` on the tasks Back, which the page
census now catches too and did not before.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo Hong
2026-09-19 18:33:20 -04:00
committed by GitHub
parent 93d245e358
commit cef4416115
6 changed files with 354 additions and 35 deletions
+5 -1
View File
@@ -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', () => {
@@ -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<string, unknown>) => options[native.platform.os] ?? options.default
}
},
Pressable: 'Pressable',
StyleSheet: { create: <T,>(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<string, unknown> = {}
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 (
<RightDrawer visible={visible} onClose={() => {}}>
<DrawerBody />
</RightDrawer>
)
}
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())
})
})
@@ -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<string, Set<string>> {
const groups = new Map<string, Set<string>>()
for (const control of CONTROLS) {
if (!control.press.known) {
continue
}
const names = groups.get(control.press.value) ?? new Set<string>()
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([])
})
})
+26 -3
View File
@@ -79,6 +79,8 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl
<Pressable
style={styles.reconnectButton}
onPress={() => void forceReconnectHost(hostId!)}
accessibilityRole="button"
accessibilityLabel="Reconnect"
hitSlop={8}
>
<Text style={styles.reconnectButtonText}>Reconnect</Text>
@@ -267,6 +269,8 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl
<Pressable
style={[styles.filterChip, settings.activeFilterCount > 0 && styles.filterChipActive]}
onPress={() => state.setShowFilterModal(true)}
accessibilityRole="button"
accessibilityLabel={`Filter workspaces${settings.activeFilterCount > 0 ? `, ${settings.activeFilterCount} active` : ''}`}
>
<Filter
size={12}
@@ -282,14 +286,24 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl
</Text>
</Pressable>
<Pressable style={styles.modeButton} onPress={() => state.setShowSortPicker(true)}>
<Pressable
style={styles.modeButton}
onPress={() => state.setShowSortPicker(true)}
accessibilityRole="button"
accessibilityLabel={`Sort by ${settings.selectedSortLabel}`}
>
<SlidersHorizontal size={14} color={colors.textSecondary} />
<Text style={styles.sortLabel} numberOfLines={1}>
{settings.selectedSortLabel}
</Text>
</Pressable>
<Pressable style={styles.modeButton} onPress={() => state.setShowGroupPicker(true)}>
<Pressable
style={styles.modeButton}
onPress={() => state.setShowGroupPicker(true)}
accessibilityRole="button"
accessibilityLabel="Group workspaces"
>
<Layers size={14} color={colors.textSecondary} />
<Text style={styles.sortLabel} numberOfLines={1}>
{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"
>
<UserCircle
size={16}
@@ -321,6 +337,8 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl
style={styles.searchToggle}
onPress={() => actions.navigateFromHostList(`/h/${encodeURIComponent(hostId)}/tasks`)}
disabled={connState !== 'connected'}
accessibilityRole="button"
accessibilityLabel="Tasks"
>
<List
size={16}
@@ -328,7 +346,12 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl
/>
</Pressable>
<Pressable style={styles.searchToggle} onPress={() => state.setShowSearch((s) => !s)}>
<Pressable
style={styles.searchToggle}
onPress={() => state.setShowSearch((s) => !s)}
accessibilityRole="button"
accessibilityLabel={state.showSearch ? 'Close search' : 'Search workspaces'}
>
{state.showSearch ? (
<X size={16} color={colors.textSecondary} />
) : (
@@ -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 = ''
@@ -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: '' }
}