Fix radio group keyboard navigation - #992
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The mobile keyboard and focus path is unavailable in the shipped React Native stable runtime, so the mobile half of this fix does not work and removes the previous native focus traversal from unselected radios.
Reviewed changes the full initial PR at 42a97313, including all 28 changed files and the surrounding radio-group callers and installed React Native implementation.
- Shared navigation — centralizes wrapped Arrow, Home, and End index calculation in
@orbit/shared. - Web radio groups — reuses the existing registry for segmented controls, select-check rows, goal-type controls, and preference pickers while preserving DOM-order navigation.
- Mobile radio groups — adds a registry and roving tab stops for segmented controls, select-check rows, goal-type controls, selection fields, preference pickers, and time-picker columns.
- Regression coverage — adds focused web and mobile tests for selection, focus movement, disabled-option skipping, wrapping, and pointer or touch behavior.
GPT Sol ｜ 𝕏
Round 5 is
|
There was a problem hiding this comment.
Caution
The generated Android app now performs React Native's one-shot feature-flag override twice during startup, so the application will terminate in MainApplication.onCreate before React Native initializes.
Reviewed changes since the prior Pullfrog review at 42a97313, focusing on round 5 8cdef1b4:
- Enabled native event gates — injected the Android key-event and imperative-focus overrides through the Expo config plugin.
- Corrected roving focus timing — made the destination tabbable before focusing it and tightened the React Native host mock to reject
tabIndex={-1}. - Aligned Android coverage — removed fabricated Home and End events from mobile tests while retaining the shared web behavior.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
Caution
The generated Android startup now installs two React Native feature-flag providers and throws before the app renders.
Reviewed changes since the prior Pullfrog review at 42a97313, including the new Android runtime adapter and the resulting mobile focus sequence.
- Enabled Android interfaces — Added a generated
MainApplicationoverride for React Native key events and imperative focus. - Deferred mobile focus — Made the arrow-key destination tabbable before moving focus after render.
- Scoped Android keys — Removed unsupported Home and End expectations from mobile coverage while retaining all arrow directions.
- Strengthened regressions — Made the host mock reject focus on
tabIndex={-1}and added config-plugin output coverage.
GPT Sol ｜ 𝕏
Resolves apps/mobile/components/ui/motion-pressable.tsx as the union of both sides: 988 added the optional `active` prop for the dragged Progresso card and this branch added `forwardRef` so a radio row can move focus imperatively. Both are kept and the shared body already used both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review at 8cdef1b4, including the startup correction and subsequent merge-forward:
- Replaced the conflicting override — Moved custom key and focus flags after
loadReactNative(this)and used a fresh accessor so React Native's one-shot release provider remains intact. - Preserved release-level behavior — Delegated every existing flag to the selected experimental, canary, or stable provider while changing only key events and imperative focus.
- Strengthened startup coverage — Rejected the former
override(...)call and asserted the generated provider selection and lifecycle ordering. - Resolved the merge conflict — Retained both
MotionPressable's base-branchactivestate and this PR's forwarded native ref.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
Important
The mobile registry can navigate out of visual order after filtering, so the searchable timezone picker may select the wrong adjacent option.
Reviewed changes the full PR at 144254cb, including all 30 changed files and the surrounding radio-group callers and installed React Native implementation.
- Shared navigation — centralizes wrapped arrow, Home, and End index calculation in
@orbit/shared. - Web radio groups — extends the existing roving registry to goal types, preference pickers, select rows, and segmented controls while preserving DOM-order navigation.
- Mobile radio groups — adds roving tab stops, disabled-item skipping, selection movement, and post-render focus across the corresponding Android controls.
- Android runtime adapter — configures React Native key events and imperative focus after the release-level provider initializes, while delegating all other feature flags.
- Regression coverage — exercises web and mobile selection, focus, wrapping, disabled options, touch or pointer behavior, host focusability, and generated Android startup ordering.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review at 144254cb, focusing on the rendered-order correction:
- Preserved rendered order — Required each mobile radio item to supply its current array index and sorted enabled registry entries by that position, so remounts no longer change arrow-key order.
- Updated every mobile caller — Passed rendered positions through goal types, preference pickers, segmented controls, selection rows, time fields, and navigation test fixtures.
- Covered the remount regression — Added a searchable-timezone test that narrows and restores the list before verifying ArrowDown selects the visually adjacent option.
GPT Sol ｜ 𝕏
|
@pullfrog review |
There was a problem hiding this comment.
Important
Android's native directional focus traversal still runs after the new JavaScript handler, so repeated arrow navigation can escape the radio group instead of advancing selection reliably.
Reviewed changes at a559477e, covering the complete radio-navigation implementation and its Android runtime integration:
- Shared navigation — Centralizes wrapped Arrow, Home, and End index calculation in
@orbit/shared. - Web radio groups — Extends roving focus and selection to segmented controls, select rows, goal types, and preference pickers while preserving DOM order.
- Mobile radio groups — Adds indexed item registration, disabled-item skipping, roving tab stops, and post-render focus across the corresponding Android controls and time columns.
- Android runtime adapter — Enables key events and imperative focus after React Native installs its release-level provider while delegating all other flags.
- Regression coverage — Exercises selection, focus, wrapping, filtering, disabled options, touch or pointer behavior, and generated startup ordering.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
Important
Android forward focus now changes values on entry and inserts an invisible stop on exit; these regressions must be corrected before merge.
Reviewed changes since the prior Pullfrog review at a559477e, covering the native-focus rewrite and its follow-up typing change:
-
Replaced synthetic key handling — Removed mobile
onKeyDown,preventDefault(), roving tab stops, and imperative focus in favor of Android focus traversal and focus-driven selection. -
Added native focus routing — Wired every enabled option to adjacent native handles for directional wrapping and to a shared post-group target for forward traversal.
-
Reworked mobile coverage — Assigned distinct host handles in the React Native mock and changed focused suites to assert focusability, native targets, and
onFocusselection. -
Merged the latest base — Brought
redesign/mainforward without a radio behavior change, then typed the Android focus props at the final head.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
Important
The hidden focus stop and production feature override are fixed, but the existing forward-entry thread remains unresolved: default Tab focus can still select an unchecked radio and close picker sheets. __setFocusImpl also remains as the test-only cleanup from the feature-flag thread.
Reviewed changes since the prior Pullfrog review at 522fb746, focusing on 0cbcd183:
-
Removed the invisible focus stop — Deleted the focusable sentinel and all
nextFocusForwardrouting while preserving directional wrapping among visible enabled radios. -
Removed unused Android feature overrides — Deleted the generated
MainApplicationfeature-flag injection and its generated-source regression now that native focus routing consumes neither flag. -
Aligned the host fixtures — Made
findNodeHandlereject synthetic objects without a native tag and updated keyboard-aware scroll tests to mount a real mockedTextInputhost.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
Important
The Android entry redirect depends on a generic focus command disabled in the shipped stable runtime, so hardware-keyboard entry still lands on an unchecked radio.
Reviewed changes since the prior Pullfrog review at 0cbcd183, covering the focus-entry correction and its regression coverage:
- Distinguished entry from movement — Tracked focus within each mobile radio group so initial entry does not select an unchecked landing item while directional movement still selects its destination.
- Redirected entry to selection — Attempted to move initial focus to the checked enabled item and left focus in place when no item is checked.
- Reset group focus state — Deferred blur cleanup so focus transfers within a group remain movement while a completed exit makes the next focus an entry.
- Strengthened mobile coverage — Added entry, re-entry, no-selection, and time-column regressions and initialized existing focus-selection tests from an in-group item.
GPT Sol ｜ 𝕏
There was a problem hiding this comment.
Important
The web correction introduces no new issue, but this PR remains blocked by the open Android focus-entry thread from the prior review.
Reviewed changes since the prior Pullfrog review at bb3ae15a, focusing on the web duplicate-selection correction:
- Guarded selected destinations — Focused the keyboard-navigation destination while invoking its selection handler only when that radio is not already selected.
- Covered endpoint keys — Added an exact regression proving End on the already-selected last radio retains focus without firing
onChangeagain.
GPT Sol ｜ 𝕏
|
@pullfrog review |
|
Independent review at head Codex and Pullfrog share one exhausted OpenAI allowance until 2026-09-22, so a separate Claude agent reviewed this and the orchestrator verified the load-bearing citations against installed source. Stating the substitution rather than hiding it: this is not a Pullfrog verdict. First, a correction to a correction this repository published yesterdayA review on this pull request cited Both read P1-1: every arrow key press in the Timezone and Week-start pickers writes to the server
Open Preferences, open Timezone, attach a hardware keyboard, press ArrowDown ten times to reach your zone. Ten Web is the same code path, not an adapter difference: This is new in this pull request, on both platforms. Before it the web Both new tests assert the defect as correct. The mobile test asserts The shape that is already right in this codebase is Red first: focus three timezone rows in sequence and assert Secondary effect of the same wiring: P1-2: every radio group becomes an arrow-key trap on Android
The pull request's own test asserts the trap: This is the platform-adapter defect named in the earlier rounds, one level up. The group already makes every option individually focusable, which IS the platform-correct answer. Overriding all four D-pad directions on top of it imports the web wrap convention into the native adapter, and on the web wrap is right only because Tab exits the group. A D-pad has no Tab. Leave Red first: assert that no option in the hours column carries a P2-1: the value now changes while the sheet is still dismissingThe pull request replaces
Red first: make the P2-2: deleting
|
…roid Arrow keys inside the Timezone and Week-start pickers wrote one server mutation per keypress, because selection followed focus straight into onTimeZoneChange and onWeekStartChange. The pickers now hold a draft and persist it from onCommit, the shape time-field.tsx already used, so the value changes once and after the sheet has finished closing. Mobile radio groups no longer set nextFocusDown, nextFocusLeft, nextFocusRight or nextFocusUp. Those are Android focus overrides, so a per-group modulo wrap trapped every arrow key inside one time column and inside the segmented control. Every enabled option stays focusable and the platform's own focus search runs. Web keeps its roving tabIndex. A press on the row that focus already selected now commits without selecting again, so a D-pad centre press cannot write twice. The mobile-only index prop is gone: entry focus redirects to the checked option, and when none is checked it stays where the platform put it, so no caller has to count its own children. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A press on a row that is already selected now commits without calling onSelect again, so the first MoveParentOverlay case could no longer observe onSelectOption. It rendered with selectedMoveParentId null, which makes the root row the checked one, and then clicked the root row. The case renders with the tree destination checked instead, so the root row is unchecked and the press that follows selects it. That is what the case is named for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Run failed. View the logs →
|
1 similar comment
|
Run failed. View the logs →
|
|
Independent re-review at head Codex and Pullfrog share one exhausted OpenAI allowance until 2026-09-22, so a separate Claude agent reviewed this against the worktree carrying this head. Stating the substitution rather than hiding it: this is not a Pullfrog verdict. One finding is REJECTED, and the rejection matters more than the findingThe review claimed that Zero matches for One real correction does come out of it: Android DOES map This is the second citation tonight that looked authoritative and was not. Read the file. P1-1 is clean, end to end, both platformsMobile: The two tests that asserted the defect as correct now assert the fix rather than having been deleted: the web pair asserts P1-2 is clean, and its consequence holdsZero mobile production hits for The test that asserted the trap now asserts its absence, in the right shape. The other three ordered confirmations are cleanThe value no longer changes mid-dismiss: Both non-reddenable tests are gone and their replacements are real: the web one now clicks the ENABLED row and asserts the disabled one P2: the config plugin's test only exercises the already-patched file, never the pristine one a release build produces
If the Expo template changes that indentation or the P3s, all small, all in the same round
Verified clean, with the line that settles eachThe focus mechanism is sound rather than invented:
Every other new test was checked by naming the product change that reddens it, across the mobile One pre-existing divergence worth a follow-up ticket rather than a block: the web time picker is The |
The plugin test fed one fixture that already carried the legacy override block, so the pristine Expo template path, the one every CI and EAS build starts from because .gitignore:86 ignores apps/mobile/android/, was never executed. A second fixture now runs that path and asserts all five imports land after the anchor. Removing the Provider import reddens the new test and leaves the old one green, which is the gap it closes. The plugin used to drop the ReactNativeFeatureFlagsDefaults import unconditionally while removing its block only on an exact match, so a tree an older revision patched lost an import its surviving block still needed. The import now goes only once nothing constructs the class. Import detection matches a whole line, so the longer ReactNativeFeatureFlagsProvider no longer stands in for the shorter ReactNativeFeatureFlags. dangerouslyForceOverride returns the flags read before the call and its KDoc at ReactNativeFeatureFlags.kt:631-645 says to log it. The generated onCreate now keeps that value and warns when it is not null. The move-parent eyebrow and the mobile no-results text moved out of the radiogroup on both platforms. role=radiogroup owns radio children, so a text node inside it made TalkBack and NVDA count a fifth item. PickerOptions took its draft from a prop once and never resynced, so a cold start where the profile resolves after the first render left every row unchecked. An effect on the incoming value now moves the draft with it. The unused disabled prop is gone from both select-check RadioRows; no production caller passed it. The disabled-skipping coverage moved to the move-parent suites, where the list RadioRow carries a real disabled destination. The mobile focusable-target test now counts focusable hosts against options, and the dead onBlur call in the re-entry test is deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The design review of 398103a found two DESIGN.md breaks in the eyebrow this branch just moved above the move-parent group. DESIGN.md:720 defines the eyebrow role as Geist Mono 12/500 +0.08em uppercase on fg-3. The web copy used var(--font-sans), so the same string rendered in two families across platforms while mobile already used GeistMono_500Medium. It now uses var(--font-mono), which is what components/ui/section-label.tsx has always used for the same role. DESIGN.md:351 requires the gap between two groups to be at least twice the gap within one. The mobile eyebrow sat 4px above a group whose rows are 4px apart, so a label meant to head the whole list read as one more row. Its bottom margin is now 8. The web eyebrow also carried margin 2px 0 0 from its old position inside the group. The parent column already supplies an 8px gap, so the margin is gone and the gap is the only spacing mechanism. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Run failed. View the logs →
|
1 similar comment
|
Run failed. View the logs →
|
|
Run failed. View the logs →
|
1 similar comment
|
Run failed. View the logs →
|
The record on
|
| file | 2026-09-17 onward | this worktree, 2026-09-16 |
|---|---|---|
ReactNativeFeatureFlagsDefaults.kt |
6910 bytes, :86 and :92 both false |
6908 bytes, both true |
.../events/KeyEvent.kt |
6547 bytes, no Home, no End anywhere, KEY_NAME_MAP ending on the four DPAD codes at :149-152 |
6779 bytes, MOVE_HOME and MOVE_END at :114,115 and :143,144 |
So every citation in this chain was honest. A review said false; a comment "corrected" it to true; a third read confirmed false; this round reported true with Home and End. Each read the installed source, as code standard 8 requires. The standard assumes one installed source per repository, and that assumption was false here.
What it means for this pull request, and it is good news
Against the bytes this branch merges into, enableImperativeFocus defaults false. So the config plugin is the MECHANISM the focus behaviour depends on, not a redundant pin, and the pull request's premise holds. The no Home, no End constraint also holds against those bytes, so a mobile test must not fabricate either key.
Filed as #601, which owns establishing why the installs diverge and giving a session a way to check its own tree before citing it: https://github.com/thomasluizon/orbit-tickets/issues/601
Two things this branch owes because of it
- Reinstall this worktree before the next round, so its suites run against the same react-native every other worktree has. Do it deliberately and re-run the branch's suites afterwards, because it changes what the tests see.
- Correct the pull request body, which currently records the
truereading and theHome/Endmapping as facts about react-native 0.86.3. State the divergence instead, and say which bytes the claim is made against.
Round 3 itself is accepted
Two commits, 398103ae carrying the two P2s and six P3s across 14 files with each new assertion shown red first, and 29998d8d fixing two DESIGN.md breaks a design review found in the eyebrow the round had just moved: the web copy sat in the sans family where DESIGN.md:720 says Geist Mono, and the mobile label sat 4px above a group whose rows are 4px apart, against the 2x rule at DESIGN.md:351. Catching a design break inside your own fix, in the same round, is the right instinct. Root type-check forced green, root lint 0 errors, suppressions flat, 131 tests across 17 radio suites.
Its last observation is also right and already true: pull request 1024 merged as 0f96925b, four minutes after the order called it unmerged, so Surface Manifest Drift is live on redesign/main and this branch goes red on committed surface no longer exists in the tree: m-overlay-ui-selection-field. The deferred manifest round can run now, and it carries the reinstall above.
…rash The branch deletes apps/mobile/components/ui/selection-field.tsx, so the committed inventory still named a surface the tree no longer has and the new Surface Manifest Drift job went red on it. Regenerate the manifest, move m-overlay-ui-selection-field into the deleted section with the decision that removed it, and drop the MOBILE_OVERLAY_BASES entry that named the same file. With that entry gone the web and mobile base lists are the same value, so one OVERLAY_BASES serves both call sites instead of two aliases for one list. The determinism assertion the whole drift gate rests on passed on a dead process: the generator writes nothing on a non-zero exit, so a crashed second run left the first run's bytes on disk and the byte comparison agreed with itself. Both derivations now assert their exit code before the comparison. Also close what 1024's review left owed on these two files: de-duplicate trackedFiles, because git ls-files prints an unmerged path once per index stage; narrow the chat-order comment to the real condition, a character between 0x2F and 0x5C after the shared prefix; say that an untracked file is excluded for being untracked rather than for being ignored, because git ls-files reads the index; and record that the android build-tree exclusion now only guards a force-added artifact. The fixture wrapper moves to confirm-sheet. The order expected it to stay green, and it does not: with selection-field gone from the base list the fixture caller drops out of the inventory, and a fixture naming a file no tree has tests nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Run failed. View the logs →
|
1 similar comment
|
Run failed. View the logs →
|
Independent review, head
|
The time picker announced itself two different ways. Web used listbox and option with aria-selected, while Android used radiogroup and radio. Web now uses the shared RadioGroup and useRadioGroupItem, so both platforms announce a radio group. The mobile option also set accessibilityState.selected next to checked; only checked makes a node checkable, so the extra key is gone. The absence guard on the mobile time column could not fail on the axis that broke. It asked whether a nextFocus handle belonged to the same column, so a handle pointing at the next column passed. It now asserts every nextFocus prop is undefined, on both columns. ReleaseLevel was missing from the config plugin import list, and only a hand-written fixture that supplied the import kept the generated Kotlin compiling. The plugin now adds the import itself, and a template without it proves that. withReactNativeImperativeFocus applies to debug builds too, so it moves out of the release-build plugin into plugins/with-react-native-imperative-focus.js with its own tests and registration. A mobile RadioGroup with no FocusProvenanceView ancestor now throws. Without the provider every focus reads a null previous target, which reads as entry, so the group would redirect focus on every move. The web language picker dropped a lookup that returned its own input. The prop is now SupportedLocale, narrowed once in usePreferenceControls, which is the shape the mobile twin already had. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…enance usePreviousFocusTarget() returns null outside FocusProvenanceView, and a null previous target reads the same as "focus arrived from outside the group". The group therefore armed a redirect to the checked row on every focus event, not only on entry. RadioGroup now publishes a null onFocus in that case, so useRadioGroupItem takes the standalone path and selects the focused option. A group under the provider keeps the entry redirect it needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Run failed. View the logs →
|
1 similar comment
|
Run failed. View the logs →
|
…sRef prop The radio conversion replaced the browser's default focus ring on the web pickers, so a keyboard user lost the only cue that told them where they were. Each converted control now carries an explicit focus-visible outline in --primary. MotionPressable's focusRef prop is gone: the split focus plugin made it redundant, and its one caller now forwards an ordinary ref. The surface manifest is regenerated at this head. #543
|
Run failed. View the logs →
|
1 similar comment
|
Run failed. View the logs →
|
|
Independent review, head
|
| run | result |
|---|---|
| this head's test, pass-through shim (control) | Test Files 1 passed (1), Tests 11 passed (11) |
| this head's test, defect restored | 1 failed / 10 passed, AssertionError: expected false to be true at :290 |
the previous head 24b2f7f6's test, same defect restored |
PASSES |
That last row is the whole point. The old expect(hours.filter((hour) => hourHandles.has(hour.props[direction]))).toEqual([]) stayed green with the D-pad trap present, because adjacent-column handles are never in hourHandles. The new assertion reddens on it. The guard is real.
Earlier findings
| # | finding | status |
|---|---|---|
| P1 | vacuous absence assertion, horizontal axis | fixed, proved red at :290 and green with the defect removed |
| P1 | web time picker is listbox/option/aria-selected |
fixed, apps/web/components/ui/time-field.tsx:62-113 is RadioGroup plus useRadioGroupItem, role="radio", aria-checked, and the scroll-into-view query moved to [role="radio"][aria-checked="true"] |
| P1 | mobile accessibilityState={{ selected }} on a radio |
fixed and swept. All five accessibilityRole="radio" sites in apps/mobile use checked: radio-row.tsx:221,227, segmented-control.tsx:35, select-check.tsx:114, time-field.tsx:79, goal-type-selector.tsx:70. Zero radios left on selected. Confirmed against installed source: ReactAccessibilityDelegate.kt:632-640 sets isCheckable and isChecked only for STATE_CHECKED |
| P2 | ReleaseLevel absent from FEATURE_FLAGS_IMPORTS |
fixed, with-react-native-imperative-focus.js:9, plus TEMPLATE_WITHOUT_RELEASE_LEVEL which strips it so only the plugin can supply it. ReleaseLevel.kt is package com.facebook.react.common with exactly EXPERIMENTAL, CANARY, STABLE, so the generated when is exhaustive |
| P3 | usePreviousFocusTarget() null ambiguity |
fixed, and better than ordered: a6b71b43 made it throw, c77458a3 replaced that with onFocus = getPreviousFocusTarget ? handleFocus : null plus a standalone fallback at radio-row.tsx:117,165-172, with a test |
| P3 | focus plugin inside a release-build plugin | fixed, split into its own file and suite, registered through withLocalPlugin. with-android-release-build-fixes.js and its test are byte-identical to redesign/main |
| P3 | pointless LANGUAGE_OPTIONS.find(...) |
fixed, deleted; selectedLanguage is now SupportedLocale, narrowed once in use-preference-controls.ts:39 |
| standing | zero production nextFocus |
held, grep -rn "nextFocus" apps/ returns only test files and a test type helper |
| standing | drafts commit only on press | held on both platforms, with red-capable tests |
| standing | no node_modules edits |
held. Every cited react-native file has an mtime within 3 seconds of the package extraction, KeyEvent.kt is 6547 bytes with zero MOVE_HOME/MOVE_END, and ReactNativeFeatureFlagsDefaults.kt:86,92 both read = false. #601's symptom is gone from this tree |
The salvage commit: nothing was left behind
74b891a3 was written by tools/salvage-worker.mjs after a ceiling kill, and it excluded apps/web/components/ui/radio-row.tsx as pure CRLF churn. That exclusion is safe: the focus ring the commit adds elsewhere already existed in that file on redesign/main (git show 0f96925b:apps/web/components/ui/radio-row.tsx line 177), so nothing was dropped. useRadioGroupItem is exported and consumed by select-check.tsx and time-field.tsx, so the file's real content is intact.
The commit is coherent on its own: MotionPressable loses a focusRef prop that was always identical to ref, its one caller now passes ref, three converted web controls gain an explicit focus ring, one stale Rubik doc comment is corrected, and the manifest is regenerated.
#608 and #609: the split is right
Neither belongs here. #608 (two RadioRow implementations, the unselected glyph at 1.5px --hairline-strong against DESIGN.md:778's 2px --track-empty, goal-type not on SegmentedControl) is entirely pre-existing on redesign/main and needs token and visual changes this ticket's scope excludes. #609's two controls declare accessibilityRole="button" with accessibilityState={{ selected }}, which is correct for a selectable button and not the false promise this pull request chased, which exists only on a radio. Both are untouched by this diff.
New findings, none blocking
- P2, the pull request body is stale and the round's mandated evidence is missing. The body ends at Round 15 and its
### Still open, and named so it is not lostblock still asserts "The web time picker isrole="listbox"withrole="option"while mobile isradiogroupwithradio", whicha6b71b43fixed. A squash merge would carry a false statement about the merged code into the history. Being fixed before the merge, with this review's red/green table going into## Test evidence. - P2, a shared util with zero coverage in its own workspace.
packages/shared/src/utils/radio-navigation.ts:1-16is tested only fromapps/web.npx turbo run test -- --coveragereports shared at 95.3 / 89.07 / 96.36 / 97.39 against thresholds of 95 / 88 / 96 / 97, and the file itself at0 | 0 | 0 | 0, uncovered6-15. It passes today with 0.30 points of statement margin, and the next pull request adding a few uncovered shared statements tips that gate red with the blame landing elsewhere. It also breaks D84. Fix: move the test topackages/shared/src/__tests__/radio-navigation.test.ts; it already imports from@orbit/shared/utils, so the body needs no change. Filed rather than held, because it changes no behaviour. - P2, nothing asserts
app.config.jsregisters the focus plugin. Drop"./plugins/with-react-native-imperative-focus"from thewithLocalPluginchain andenableImperativeFocus()staysfalse,ReactViewManager.kt:472-476gateshandleFocusbehind it, andradio-row.tsx:110'sfocus()becomes a silent no-op on every Android radio group while every test stays green, because every test drives focus through the mock. The registration is correct at this head. Pre-existing forwith-android-release-build-fixestoo; this pull request doubles the exposure rather than creating it. Filed. - P3, an undeclared spacing change on the mobile timezone picker.
preferences-sections.tsx:113-118againstpreferences-styles.ts:50-52:timeZoneOptionssetsgap: 8and the rows are now wrapped in theRadioGroupview, so they sit flush. It is the right direction, web was already flush andDESIGN.md:776makes the hairline the only owner of row separation, so it is being named in the body rather than restored. - P3, the salvage commit message overstates its own fix. It says the conversion "replaced the browser's default focus ring", but all three touched controls were already
<button>elements andglobals.csscarries no rule stripping their outline. Nothing was lost; the commit adds an accent ring where the browser default showed. The change is right, the claim is not.
Checked and found clean
Every declared radio group on both platforms routes through the one primitive, symmetrically: six mobile and six web. The eyebrow fix holds on both, and the web eyebrow moving to var(--font-mono) brings it onto DESIGN.md:720 and into parity with mobile's GeistMono_500Medium. The entry-redirect state machine cannot loop or write: armedRedirect is cleared at the top of every handleFocus, and even if the arm check missed, the redirect target is the checked item so if (!focusedItem.selected) is false. Mobile Sheet uses plain Views rather than RN Modal, so the root onFocusCapture genuinely covers the pickers. No new i18n keys. Zero className in the mobile diff.
Commands run
git -C <worktree> rev-parse HEAD -> 74b891a3a7d7c4d012929e138ae019f3800eceb2
git status --porcelain -> empty, before and after the probe
node tools/surface-manifest.mjs --check -> matches this tree, 185 surfaces, 796 cells, exit 0
node tools/redesign-coverage.mjs exit 0
GITHUB_BASE_REF=redesign/main node tools/check-suppressions-ratchet.mjs
-> web 14 -> 14 ok, mobile 16 -> 16 ok, exit 0
node tools/test-tools.mjs -> ORBIT TOOLS GATE OK, 1811 assertions, exit 0
node .claude/hooks/test-hooks.mjs -> ORBIT HOOKS OK, exit 0
npx turbo run type-check --concurrency=1 --force -> 3 successful, 3 total
npx turbo run lint --concurrency=1 -> 3 successful, 0 errors
npx turbo run test --concurrency=1 --force -- --coverage -> 4 successful, 4 total
eslint-rules 392, shared 2081, mobile 2688, web 2969
mobile focused, 9 files -> 9 files passed, 93 tests passed
web focused, 7 files -> 7 files passed, 37 tests passed
One recorded artifact, not a finding: a first run of the tools harness launched while the coverage suite was running reported two create-worktree.mjs lock-reclaim failures. That file is untouched by this pull request and both cases pass in the clean re-run above.
1 participant
Summary
Platform evidence
ReactViewManager.kt:204-211sets Android's four directional focus ids fromnextFocusUp,nextFocusDown,nextFocusLeft, andnextFocusRight.BaseViewConfig.android.js:348-352registers those four props andnextFocusForward.View.js:82-84mapstabIndexto Android focusability, so enabled mobile options use native focus and explicit directional targets.KeyEvent.kt:132-153does not emit Home or End. Those keys remain web only.Test evidence
Before the original fix, existing web focused tests passed: 2 files and 10 tests.
Before the original fix, existing mobile focused tests passed: 3 files and 17 tests.
The strengthened web segmented-control test then failed because ArrowDown did not select
completed.The strengthened mobile segmented-control test then failed because tab indexes were unset.
Round 5 unchanged mobile segmented-control test passed with the defect present: 1 file and 3 tests. The old mock allowed focus on
tabIndex={-1}.After correcting the mock, the same mobile test failed as intended: 1 failed and 2 passed. Focus remained undefined for
segment-completed-unselected-enabled.The new Android startup regression failed as intended: 1 failed and 6 passed. The plugin registered no
mainApplicationmod.Final web focused tests passed: 4 files and 15 tests.
Final mobile focused tests passed: 7 files and 32 tests.
Earlier prebuild evidence generated the former override before
super.onCreate(); round 3 replaces that unsafe startup order.Round 3 unchanged
npm test --workspace @orbit/mobile -- __tests__/plugins/with-android-release-build-fixes.test.tspassed 1 file and 7 tests with the startup crash still present.The strengthened round 3 regression then failed as intended: 1 failed and 6 passed because
ReactNativeFeatureFlags.override(still appeared once.After the fix, the same focused command passed 1 file and 7 tests.
Round 3
npm run type-checkpassed all 3 workspace tasks.Round 3
npm run lintpassed all 3 workspace tasks with 7 existing warnings.npm run type-checkpassed all 3 workspace tasks.npm run lintpassed all 3 workspace tasks with 7 existing warnings.npm testpassed shared, ESLint rules, and all 2,572 mobile tests in the original round.All 2,918 web assertions passed in the original round, but the command exited 1 after
progress.test.tsx:210timed out closing Chrome after 30 seconds.The teardown-only timeout occurred during the parallel full-suite run and is outside this change. Closes thomasluizon/orbit-tickets#543
Round 10 unchanged focused tests passed with both defects present: mobile 3 files and 25 tests; web 1 file and 5 tests.
Round 10 strengthened mobile coverage failed as intended: 4 failed and 23 passed. The tracked plugin registered no mainApplication mod, the checked-entry focus observer saw zero calls, and the host mock did not model focusable View focus. One initial wrap assertion also exposed a test-authoring mistake and was corrected to inspect native hosts.
Round 10 strengthened web coverage failed as intended: 1 failed and 5 passed. ArrowDown from Top level left focus on Top level because it was outside the group.
After the two fixes, the focused commands passed: mobile 3 files and 27 tests; web 1 file and 6 tests.
The review-found no-selection fallback regression failed against 8eefbea: 1 failed and 8 passed because redirected focus selected first. At d0631c3, the same suite passed all 9 tests.
Final radio coverage passed: mobile 9 files and 77 tests; web 5 files and 21 tests.
Root npm run type-check passed all 3 workspace tasks.
Root npm run lint passed all 3 workspace tasks with 9 existing unrelated warnings and no errors.
Round 11 unchanged tests passed before the new assertions: mobile 1 file and 9 tests; web 2 files and 10 tests.
Round 11 mobile red proof failed 1 test and passed 8 because immediate re-entry incorrectly selected the first radio.
Round 11 web red proof failed 2 tests and passed 9 because Move Parent was unnamed and TimeField had multiple tab stops.
Round 11 focused green passed 8 mobile files and 70 tests, plus 6 web files and 26 tests.
Final
npm testpassed all 4 tasks: mobile 2,671 tests, web 2,960 tests, and ESLint rules 392 tests.Final
npm run type-checkpassed all 3 workspace tasks.Final
npm run lintpassed all 3 workspace tasks with 9 existing unrelated warnings and no errors.Final suppression ratchets passed: web 20 to 20 and mobile 16 to 16.
Round 13 unchanged
move-parent-overlay.test.tsxfailed against42bda168with the one unfinished edit reverted: 1 failed and 5 passed,expected "vi.fn()" to be called with arguments: [ null ],Number of calls: 0.Round 13 with that edit finished, the same suite passed 1 file and 6 tests.
Round 13 focused web radio suites passed 4 files and 27 tests: move-parent-overlay, preference-picker-sheet, select-check and time-field.
Round 13 focused mobile radio suites passed 5 files and 46 tests: select-check, time-field, list-primitives, preference-picker-sheet and move-parent-dialog.
Round 13
npx turbo run type-check --concurrency=1 --forcepassed 3 of 3 tasks, exit 0. The type-check was forced.Round 13
npx turbo run lint --concurrency=1passed 3 of 3 tasks, exit 0, with 2 pre-existing web warnings and no errors.Round 13
npx turbo run test --concurrency=1passed 4 of 4 tasks onf104e11a: eslint-rules 392 tests, shared 2,081 tests, mobile 2,679 tests, web 2,967 tests. This is the full suite run that round 12 recorded as missing.Round 13
GITHUB_BASE_REF=redesign/main node tools/check-suppressions-ratchet.mjspassed: web 14 to 14 and mobile 16 to 16.Review harness
DESIGN.md breaks in the relocated move-parent eyebrow and both are fixed in
29998d8d, detailed underRound 14
Manual steps
Round 11: focus provenance and complete web parity
Heads
44f96483andb027b223.44f96483mergesorigin/redesign/main. The suppressions conflict took the base file wholesale andnpm run lint:prune --workspace=@orbit/webpreserved the 20 to 20 web ratchet. The Move Parent conflict keeps the base 4px spacing and this branch's single radio group.The mobile blur timer could not distinguish a directional move from a real leave followed by immediate re-entry.
b027b223removes the timer. The root focus capture now records actual focus provenance, and each radio group decides entry versus movement from the preceding native target. Checked-entry redirects remain selection-neutral.The installed React Native interface was checked directly.
ViewPropTypes.js:116,118declaresonBlurCaptureandonFocusCapture.BaseViewManager.java:751-759registers captured and bubbled focus phases.FocusEvent.kt:20-23andBlurEvent.kt:20-23emit onlytarget, so a blur event cannot identify the next target and cannot classify the transition by itself.Web Move Parent now gives its radio group the same localized destination name as mobile. Web TimeField now has one selected tab stop per listbox and supports wrapped Arrow keys plus Home and End. This fixes the unthreaded concurrent Pullfrog finding rather than merely recording it.
Both requested review threads were replied to with
fixed in b027b223and resolved. The architecture map was regenerated for inspection and its generated artifacts were not committed, per repository policy.Round 3: the override no longer runs twice, and the merge-forward Head
144254cb. Two commits since the last review.109d93c7fixes a startup crash this pull request would otherwise have shipped Pullfrog was right and the chain is proven from installed source, not from the review body: 1.node_modules/@react-native/gradle-plugin/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateEntryPointTask.kt:96generatesloadReactNative, which callsDefaultNewArchitectureEntryPoint.load()wheneverBuildConfig.IS_NEW_ARCHITECTURE_ENABLEDis true. 2.node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultNewArchitectureEntryPoint.kt:88-99callsReactNativeFeatureFlags.override(...)for every one of the three release levels. 3.node_modules/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp:1688-1693throwsstd::runtime_error("Feature flags cannot be overridden more than once")oncewasOverridden_is true. The old plugin injected itsoverridebeforesuper.onCreate(), so React Native's own call was the second one and the process died before it rendered. The override now runs AFTERloadReactNative(this)and goes throughReactNativeFeatureFlags.dangerouslyForceOverride, declared atReactNativeFeatureFlags.kt:640-645. That path cannot hit the once-only guard:ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp:406-416builds a FRESH accessor, overrides that one, and swaps it in. The provider delegates to whichever release-level providerDefaultNewArchitectureEntryPoint.releaseLevelnames, so React Native's own overrides are preserved rather than replaced, and onlyenableKeyEventsandenableImperativeFocusare changed. No try/catch was added: a defensive branch around a crash this code causes is banned by rootCLAUDE.mdcode standard 1. The other two threads are the same finding.radio-group.tsxneeded no change:ReactNativeFeatureFlagsOverrides_RNOSS_Stable_Android.ktisReactNativeNewArchitectureFeatureFlagsDefaults()with nothing added, and the plugin pinsenableImperativeFocusthroughdangerouslyForceOverride. Correction, made at 8ddcca0: this sentence originally claimedReactNativeFeatureFlagsDefaults.kt:86,92leave both flagsfalse. Read again from the installed tree, line 86 isoverride fun enableImperativeFocus(): Boolean = trueand line 92 isoverride fun enableKeyEvents(): Boolean = true, so both defaults are already on in React Native 0.86.3 and the original citation was false. TheInstalled interface evidenceline under Round 7 states the flags correctly and stands unchanged.144254cbis a merge-forward and carries one conflict resolutionredesign/mainmoved 16 commits ahead while this branch was open. The only conflict wasapps/mobile/components/ui/motion-pressable.tsx, and it is the union of two changes, neither of which is wrong:activeprop, so a dragged Progresso goal card reads as pressed; - this branch wrapped the component inforwardRef, so a radio row can move focus imperatively. Both are kept. The shared body already referencedrefandactive, so only the signature needed resolving.local/react19-apibansforwardRefon web only and says so in its own header, deliberately, because React Native's renderer version is not the React package version, so the wrapper is correct here. Nothing else was changed in the merge-forward.git diff origin/redesign/main...HEAD --statreports 30 files, which is this branch's own change and nothing more.Test evidence
npx turbo run type-check --forcefrom the repository ROOT: 3 successful, 3 total, 0 cached. Forced, because a cachedFULL TURBOon a file just changed proves nothing.npm test -w @orbit/mobile -- __tests__/screens/progress-screen.test.tsx __tests__/components/ui/segmented-control.test.tsx __tests__/components/ui/select-check.test.tsx __tests__/components/ui/selection-field.test.tsx __tests__/components/ui/time-field.test.tsx __tests__/components/habits/goal-type-selector.test.tsx __tests__/components/profile/preference-picker-sheet.test.tsx __tests__/plugins/with-android-release-build-fixes.test.ts: 8 files, 115 tests, all passed. That set is every consumer ofMotionPressableand every surface the radio work touches, run after the merge.MainApplication.ktfail against the old plugin and pass against the new one.What only a device can confirm That the two flags actually take effect at runtime on a release build. Nothing in this repository builds
apps/mobile/android/, which is generated and untracked. Thomas tests the whole redesign once from an APK he generates himself.Round 4: rendered radio order survives filtering Head
3f274fea. The mobile registry now stores each requiredindexand sorts enabled items by that current rendered position. Every mobile caller supplies its array index, so remounted timezone rows return to their visible position before keyboard navigation runs. Disabled-item filtering remains unchanged. Web needs no matching code change.apps/web/components/ui/radio-row.tsxorders registered buttons withcompareDocumentPosition, which follows current DOM order after a remount.apps/web/components/ui/radio-navigation.tsonly calculates the next index from that supplied order.origin/redesign/mainremained at0600332bbefore push, so no merge-forward was needed.Test evidence
144254cb, the unchangednpm test -w @orbit/mobile -- __tests__/components/profile/preference-picker-sheet.test.tsxpassed 1 file and 1 test. It did not exercise filter, unmount, and remount ordering.Delta/Keepinstead of the rendered adjacent rowCharlie/Three.3f274fea, the same command passed 1 file and 2 tests.goal-type-selector.test.tsx,preference-picker-sheet.test.tsx,segmented-control.test.tsx,select-check.test.tsx,selection-field.test.tsx, andtime-field.test.tsx.goal-type-selector.test.tsx,preference-picker-sheet.test.tsx,radio-navigation.test.ts,segmented-control.test.tsx, andselect-check.test.tsx.npx turbo run type-check --forcefrom the repository root passed 3 tasks, with 0 cached. The forced run verified every requiredindexcall site.npm run lintpassed 3 tasks with 7 existing warnings and no errors. No suppression count changed.Round 5b: Android follows native focus Head
522fb746. This result is superseded by Round 6 below because its shared forward target was an invisible focus stop. The Android radio group no longer handlesonKeyDownor callspreventDefault(). Every enabled option is focusable, disabled options are not, and selection followsonFocus. Each option receivesnextFocusUp,nextFocusDown,nextFocusLeft, andnextFocusRighthandles for the adjacent enabled option, including wrap at both ends. All enabled options share onenextFocusForwardtarget after the group. The registry still sorts by the required renderedindex, so filtered and remounted rows keep visual navigation order. The test host mock now assigns distinct native handles and only permits imperative focus for focusable hosts. Web needs no code change. Its DOM keyboard handler can cancel key events and its registry follows current DOM order. AndroidnextFocus*andfocusableare platform adapter props, so the existing parity position remains.386fd3c7merges the latestorigin/redesign/mainbefore final verification. The merge carried no radio behavior change and had no conflicts.Test evidence
onKeyDowndirectly and could not exerciseReactRootView.dispatchKeyEvent.focusableprop, the radio group had no forward target, and time options had noonFocushandler.npx turbo run type-check --forcefrom the repository root passed 3 tasks with 0 cached.npm run lintpassed 3 tasks with 8 existing warnings and no errors. No suppression count changed.Manual steps
## Manual stepssection above.Round 6: visible native focus targets only Head
0cbcd183. The invisible forward target and everynextFocusForwardassignment are removed. Every enabled option remains focusable, disabled options remain excluded, the four directional ids still wrap and skip disabled options, and selection still follows focus. Forward traversal now matches native Android radio behavior. It enters at the first focusable option and visits the visible enabled options in order. Moving with arrows also selects, so reaching the checked option is direct and predictable. The release feature flags are also removed. A production search found no mobileonKeyDownconsumer and no genericView.focus()call. The only remaining.focus()calls targetTextInputrefs ininput.tsxandotp-input.tsx, which use the TextInput native path and do not consume those flags. The redUnit Testscheck had three failures inkeyboard-aware-scroll-view.test.tsx. Its synthetic input fixture was not created by the host mock, so the correctedfindNodeHandlereturned null beforemeasureInWindow. The fixture now mounts the mockedTextInput, receives its real test host handle, and keeps the original behavior assertions.Test evidence
select-check.test.tsxfailed as intended: 1 failed and 2 passed. The rendered group contained one empty focusable host.measureInWindowwas never reached.npx turbo run type-check --forcepassed 3 tasks with 0 cached.npm run lintpassed 3 tasks with 8 existing warnings and no errors. No suppression count changed.origin/redesign/mainremained at43bc28ad, already contained by this branch, so no merge-forward was needed.Round 7: focus entry preserves selection
Head
bb3ae15a.Mobile now distinguishes entering a radio group from directional movement inside it. Initial focus does not select an unchecked option. It redirects to the checked enabled option through the stored native
View.focus()method. A group with no checked option leaves focus on Android's first enabled landing target. Focus movement between items still selects the destination. A zero-delay blur timer clears group focus only when another item does not focus first.Web needs no change.
apps/web/components/ui/radio-row.tsx:111-114givestabIndex={0}to the selected enabled option, or the first enabled option when none is selected.Installed interface evidence
ViewPropTypes.d.ts:93,100declares realonBlurandonFocusView props.ReactNativeTypes.d.ts:106,252declaresfocus(): voidand exportsHostInstance = ReactNativeElement.View.d.ts:22makes a View ref aHostInstance.ReactViewManager.kt:445,472-474maps the focus command torequestFocusFromJS().ReactNativeFeatureFlagsDefaults.kt:86,92enables imperative focus and key events by default in the installed React Native 0.86.3.apps/mobile/android/, ignored by.gitignore:86. No tracked plugin ships an override.Test evidence
npm test -w @orbit/mobile -- __tests__/components/ui/select-check.test.tsx __tests__/components/ui/time-field.test.tsxpassed 2 files and 11 tests. Existing tests did not distinguish entry from movement.npm run type-checkpassed all 3 workspace tasks.npm run lintpassed all 3 workspace tasks with 8 existing warnings and no errors.Device verification boundary
Only a hardware-keyboard device run can confirm Android's real forward entry and directional focus traversal. No repository test claims that platform behavior passed.
Round 8: the D76 step 6 sweep, and the web defect it found
Head
69e43526.The
Redesign Review Harnessgate failed atbb3ae15abecause this pull request changes 14 UIfiles on
redesign/main. The sweep was then run against the complete diff.interface-reviewandbetter-interfacewere fetched by listing every blob under their directoriesin
jakubkrehel/skillsthrough the Trees API, not throughnpx ui-skills get, which does not servethem. The inventory was checked first:
--jq .truncatedprintedfalseover 89 entries, so thelisting did not under-fetch.
It found one defect, on web.
apps/web/components/ui/radio-row.tsx:125called the destination'sselection handler on every directional move, including a move that lands on the option already
selected. Pressing End on the last radio when it is already checked fired
onChangeagain. Mobilealready guarded this through its
onFocuscheck; web did not.69e43526fixes it withif (!nextItem.selected), which is the same rule stated once on each platform.Test evidence for
69e43526The worker that produced this commit was killed for low memory after committing and before
pushing. None of its numbers are quoted. The runs below were produced by the orchestrator, from a
clean tree at that commit.
Red proved after the fact, by reverting only the source file:
Restored with
git checkout 69e43526 -- apps/web/components/ui/radio-row.tsx, after whichgit status --porcelainprinted nothing.Green, the four web radio suites at
69e43526:covering
select-check.test.tsx,segmented-control.test.tsx,goal-type-selector.test.tsxandpreference-picker-sheet.test.tsx.Round 9: one mobile radio primitive, and the callers that were left behind
Head
fb520fb1.The review was right that this pull request had added a SECOND mobile radio implementation instead
of extending the existing one. That is a parity break rather than a style preference: a caller got
keyboard navigation depending on which module it happened to import, while web had only one.
apps/mobile/components/ui/radio-group.tsxis deleted, 204 lines, and its behaviour lives inapps/mobile/components/ui/radio-row.tsx, which is the survivor because it already had the callers.A grep for
components/ui/radio-groupacrossapps/mobilereturns nothing, so no importer was leftpointing at a file that no longer exists.
Migrated callers:
apps/mobile/app/support.tsx,apps/mobile/components/habit-list/move-parent-dialog.tsx,apps/mobile/components/habits/create-goal-from-habit/goal-type-selector.tsxandapps/mobile/components/profile/preferences-sections.tsx. The first two are exactly the ones thefinding named as stranded on the no-navigation implementation.
The imperative focus chain, re-read rather than asserted back
The review says
element.focus()is a no-op for generic View hosts in the shipped stableconfiguration. Every step was re-read in the installed tree under
node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/:internal/featureflags/ReactNativeFeatureFlagsOverrides_RNOSS_Stable_Android.kt:10declaresclass ReactNativeFeatureFlagsOverrides_RNOSS_Stable_Android() : ReactNativeNewArchitectureFeatureFlagsDefaults(),and a grep of that file for
enableImperativeFocusorenableKeyEventsreturns zero hits.internal/featureflags/ReactNativeNewArchitectureFeatureFlagsDefaults.kt:23declares the class itextends, and the same grep there also returns zero hits.
internal/featureflags/ReactNativeFeatureFlagsDefaults.kt:86,override fun enableImperativeFocus(): Boolean = true, and:92forenableKeyEvents.views/view/ReactViewManager.kt:473-474runsroot.requestFocusFromJS()insideif (ReactNativeFeatureFlags.enableImperativeFocus()).views/view/ReactViewGroup.kt:429-435is that method:super.requestFocus(FOCUS_DOWN, null)whenisAttachedToWindow, otherwise it defers withfocusOnAttach = true.On that chain the flag is true by inheritance and the command reaches
View.requestFocus, whichsucceeds for a focusable attached view. This group makes every enabled option focusable, so the
redirect has a real target.
An earlier round of this pull request wrote the opposite into this same body, claiming
ReactNativeFeatureFlagsDefaults.kt:86,92leave both flags false. That claim is wrong against theinstalled tree and is corrected here rather than left to contradict itself.
Whether
requestFocusactually lands on a real device is the one thing no repository test canexecute. It is recorded under
## Manual stepsand is not claimed as passing.Test evidence for
fb520fb1Green, the nine suites covering every migrated caller and every radio surface: 9 files, 74 tests.
Red proved after the fact by reverting only
apps/mobile/components/ui/radio-row.tsxtofb520fb1~1, which is the migration itself:Those four are precisely the parity the finding reported missing. Restored with
git checkout fb520fb1 -- apps/mobile/components/ui/radio-row.tsx, after whichgit status --porcelainprinted nothing.The worker that produced this commit was killed for low memory after committing and before pushing.
None of its numbers are quoted; every run above is the orchestrator's from a clean tree.
The two red checks on the previous head
Redesign Review Harnessfailed because this body had lost every newline, so `Review harness` sat
mid-paragraph and was not a heading. The block had been there since round 5. The body is reflowed
and
check-review-harness.mjsnow passes locally against it.Buildfailed atRun npm ciwith anull conclusion after 45 minutes, which is a hang rather than a diff failure, and was re-run.
SonarCloud is red and does not gate
redesign/main.Round 10: checked entry focus and Move Parent parity
Heads 8eefbea and d0631c3.
Android keeps every enabled native radio focusable and stores each rendered host ref. First entry now requests focus on the checked enabled item, or the first enabled item when nothing is checked. The entry redirect records the destination before requesting focus, so neither the original landing event nor the redirected focus event changes selection. Directional focus inside the group still selects the new destination, and no sentinel or nextFocusForward target returns.
The tracked Expo plugin again edits generated MainApplication.kt, after loadReactNative(this). It delegates to the installed release-level provider and force-overrides only enableImperativeFocus; enableKeyEvents is untouched. The plugin test uses the generated Kotlin anchors, rejects the deleted two-flag legacy override, and proves idempotent output. The installed Kotlin native default currently returns true while the JavaScript fallback returns false; the explicit tracked override makes the Android release contract independent of that inherited default.
Web Move Parent now places Top level and every tree destination inside one RadioGroup, matching mobile wrap and disabled-item behavior. This closes a cross-platform parity gap the original ticket assigned to ORB-67. The touched web layout also replaces off-scale 10px and 6px gaps with the existing 8px scale step, allowing the spacing suppression baseline to shrink by two.
No repository test executes Android framework focus traversal. A release APK with a hardware keyboard must still confirm that the imperative focus command lands on the checked item and that native arrows wrap correctly.
Round 12: an arrow key no longer closes the picker it is navigating
Head
8ddcca0e.The blocking defect.
moveSelectioncalled the destination row's registered handler, and forthe four preference pickers that handler was
selectAndClose. One ArrowDown in Preferences, Timezone therefore ran
closeSheet, dismissed the sheet and committed the adjacent timezone. The rovingtabIndexthis pull request introduced had already made every unchecked rowtabIndex={-1}, so thearrow keys were the only remaining keyboard route and every option except the adjacent one was
unreachable. This was a regression this pull request introduced, not a pre-existing fault.
Where the fix lives. Selection and commit are now separate channels on the group, not on the
row, so no call site can re-couple them.
RadioGrouptakesonCommitanduseRadioGroupItemreturns
onActivate. A directional move calls only the destination row's value handler.onCommitruns on an explicit activation: on web that is the native button click a pointer press, Enter and
Space all raise, and on mobile it is
onPress.selectAndCloseis deleted from both picker sheetsand replaced by one
commitSelectionpassed to eachRadioGroup. A row has no close-capablechannel any more, so a fifth caller cannot reinherit the defect.
Red first, against the code at
78c38628The three suites that asserted the closing behaviour as correct were corrected to require that the
sheet survives a directional key and closes only on an explicit commit. Against the unfixed code:
onClose,closePickerandsheetCounteach prove the same thing from a different angle: onedirectional key dismissed the sheet.
The four review findings
fallbackFocusedHandleRefdeleted. It duplicated the provenance the root layout'sFocusProvenanceViewalready owns and survived only because most suites skipped the provider.The six suites that drive focus now mount the real provider and record a real focus capture
before each
onFocus, through the newapps/mobile/__tests__/support/focus-provenance.tsx.Suites that merely render a radio group are left alone deliberately:
usePreviousFocusTargetstill reads the context rather than throwing, because making a leaf primitive hard require a root
provider would force eight screen suites to mount a provider they have no stake in. With the
duplicate tracker gone, a suite that skips the provider can no longer pass a focus movement
assertion, which is the protection that matters.
redirectTargetIdRefscoped. It is nowarmedRedirectRef, holding the target id and thenative tag focus was on when the redirect was armed. It is cleared on every focus event and
suppresses selection only when both the id and the originating tag match, so a redirect that
never lands expires instead of swallowing a later key.
apps/web/components/ui/radio-navigation.tsdeleted. It was a one line re-export with noproduction importer. Its only consumer,
apps/web/__tests__/components/ui/radio-navigation.test.ts,now imports
getRadioNavigationIndexfrom@orbit/shared/utils.the installed file reads
trueat both lines, so Round 7's wording was already right and Round3's was the false one.
The judgement call:
selection-field.tsxis deletedapps/mobile/components/ui/selection-field.tsxhad no production caller anywhere inapps/mobile;its only importer was its own test. There is no web counterpart, so it is not a platform adapter,
and the redesign queue that might have consumed it is finished. Code standard 2 says delete unused
code immediately and standard 9 says build the simplest thing that meets current requirements, so
keeping a component alive for a caller that does not exist, and adding a
disabledoption to it,fails both. It is deleted with its test.
Its test was the only mobile coverage of
RadioGroupplusSheetpluscloseSheettogether, sothat coverage is replaced where the rule actually lives rather than dropped:
select-check.test.tsxnow asserts on both platforms that a focus move changes the value without committing, and that a
press commits. The mobile and web picker sheet suites keep the end to end proof.
Test evidence for
8ddcca0enpx turbo run test --forcepassed 4 of 4 tasks: eslint-rules 392 tests, shared 2,041, mobile2,674, web 2,968. That run predates three later edits made to clear the pre-commit lint gate: nine
unnecessary non null assertions removed from three mobile test files, one banned line comment
removed from
radio-row.tsx, and the test helper's tree parameter widened.npx turbo run type-check --forcepassed 3 of 3 tasks on the pushed bytes, exit 0. Its first runcaught seven real
TS2345errors from that helper's over narrow tree type.web radio suites passed 7 files and 33 tests.
root-allowlist gates. It rejected the first commit attempt with 10 errors and passed on the
second, so
8ddcca0eexists only because those gates are green.Not re-run on the pushed bytes: the full
npx turbo run test --force. The three edits after thegreen full run are a comment deletion, test-only assertion removals and a test-helper type widening,
and the affected suites plus a forced type-check were re-run green, but the complete suite has not
been executed against
8ddcca0eitself.Round 13: the unfinished test edit, and the first full suite run on the pushed bytes
Head
f104e11a.Provenance of every number in this section. The previous worker was killed at the 45 minute
ceiling with one file open and nothing pushed. None of its figures are quoted. Every result in this
section comes from a command run in this session against this worktree, captured to a file rather
than read through a pipe. The sections for rounds 1 to 12 keep the numbers those rounds recorded.
What was left open.
apps/web/__tests__/components/habits/move-parent-overlay.test.tsxwas theonly dirty file. It threads a
selectedMoveParentIdargument through therenderOverlayhelper andpasses
'parent'in the first case. That edit is correct, and it is now finished and committed.The reason it is needed is in
fdb88336itself.useRadioGroupItem'sonActivatenow readsif (!selected) onSelect?.(), so a press on the row that is already checked commits withoutselecting again. The case rendered with
selectedMoveParentIdnull, andMoveParentOverlaymarksthe root row selected when
rootOption.id === selectedMoveParentId, which is null equals null. Sothe case pressed the checked row and then asserted the handler fired. Rendering with the tree
destination checked leaves the root row unchecked, which is what a case named for a selectable root
row should assert.
Red first, against
42bda168with the edit reverted:With the edit restored, the same command passed 1 file and 6 tests.
The two P1s of
fdb88336, verified against the tree rather than against its commit messageA preference is written once, on a press. Both platforms hold the picker value in a local
PickerOptionsdraft.apps/mobile/components/profile/preferences-sections.tsxandapps/web/app/(app)/preferences/_components/preference-picker-sheet.tsxeach keep adraftplus adraftRef, passselectDraftas the row handler, and passcommitDraftas the group'sonCommit.That is the shape
time-field.tsxalready used.onTimeZoneChange,onWeekStartChange,onLanguageChangeandonThemeModeChangeare reachable only throughcommitSelection, which runsthem inside
closePickeron mobile andcloseSheeton web, so the value also changes after thesheet is gone rather than during its dismissal.
Which surfaces commit on focus, in one list:
moves a draft only.
time-field.tsxhour, minute and meridiem columns movepickerDrafton focus and commit inapplyDraft. That was already true before this round.because selection is the whole interaction there and no separate commit step exists.
nextFocus*is unset on mobile.grep -rn "nextFocus" apps/ --include=*.ts --include=*.tsxreturns no production hit. The only matches are three test files asserting the absence and the
navigation-render.tsxhelper's prop type.useRadioGroupItemreturnsfocusableandonFocusandnothing else, so Android's own geometric focus search runs and an arrow key can leave the hours
column for the minutes column, the meridiem column and the Done button. The mobile-only
indexpropis gone from every caller with it.
Web keeps its roving
tabIndexthroughgetTabIndexinapps/web/components/ui/radio-row.tsx.That is the named platform adapter on this pull request:
focusableandnextFocus*are Androidprops while
tabIndexandonKeyDownare DOM ones, so the mechanism differs and the behaviour doesnot.
The branch diff carries no unreviewed work
42bda168is the merge-forward the killed worker took, and no second one was added.git diff origin/redesign/main...HEAD --statnames 41 files. Every one is a radio primitive, one ofits callers, a test, the Expo plugin, the mobile focus provenance view or the shared navigation
helper. No generated baseline, no
DESIGN.md, no canvas file and no manifest appears.Still deferred
tools/surface-manifest.mjs:103,tools/__tests__/surface-manifest.mjs:38-39and the four entriesin
.claude/manifests/surfaces.jsonstill namecomponents/ui/selection-field, which this branchdeletes. Pull request 1024 regenerates that manifest and adds the
Surface Manifest Driftjob, andit is not merged. This branch rebases onto it and regenerates in one small round afterwards, with
the ids moving into the
deletedsection oftools/redesign-groups.json. A generated baseline isresolved by regenerating it, never by hand.
Round 14: the template the plugin really patches, and the group that owned a text node
Head
29998d8d. Two commits. The first carries the two P2s and six P3s across 14 files; the second carries the design review fixes. Every number below comes from a commandrun in this session against this worktree and captured to a file.
P2-1: the config plugin test now runs the pristine path
apps/mobile/__tests__/plugins/with-android-release-build-fixes.test.tshad one fixture and italready carried the legacy override block, so the path a release build actually takes was never
executed. Root
.gitignore:86ignoresapps/mobile/android/, so every CI and EAS build starts fromthe untouched Expo template.
PRISTINE_MAIN_APPLICATION_TEMPLATEis that fixture with the two feature flag imports and the legacyblock removed, and it asserts
not.toContain('ReactNativeFeatureFlags')before the plugin runs. Thenew case feeds it to
resolveMainApplicationand asserts all five imports land, that the plainReactNativeFeatureFlagsimport appears exactly once as a whole line, that all three release levelsare present, that
enableKeyEventsis not enabled, and that the override sits betweenloadReactNative(this)andApplicationLifecycleDispatcher.onApplicationCreate(this).Proof that the new case notices what the old one could not: removing
ReactNativeFeatureFlagsProviderfromFEATURE_FLAGS_IMPORTSreddens only the pristine case.A second case reindents the load anchor to two spaces and asserts the plugin throws
MainApplication.kt is missing the React Native load anchorrather than skipping the insert. Thatcase passes against the current plugin, so it records that the guard already fires; it is coverage,
not a fix. Deeper indentation is not a valid break for it, because
loadReactNative(this)is asubstring of a more indented line and the guard still matches.
P3-4: the legacy import outlives a block this plugin cannot match
The plugin dropped
import ...ReactNativeFeatureFlagsDefaultsunconditionally while removing itsblock only on an exact string match. An
android/tree an older revision patched therefore lost animport its surviving block still needed, which is a Kotlin unresolved reference on a gitignored
directory. The import now goes only once nothing constructs the class.
Import detection also changed.
FEATURE_FLAGS_IMPORTS[0]is a prefix of the longerReactNativeFeatureFlagsProviderimport, so a substring test let the longer one stand in for theshorter one.
hasImportLinematches a whole trimmed line instead, which is CRLF safe.Red first, against the plugin at
f104e11a:P3-5: the return value of
dangerouslyForceOverrideis loggednode_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt:640declares
public fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String?,and its KDoc at
:631-635says the value holds the flags read before the call and that it shouldstill be logged somehow. The generated
onCreatenow keeps it infeatureFlagsReadBeforeOverrideand warns throughandroid.util.Log.wwhen it is not null, so afuture React Native that reads
enableImperativeFocusduring load fails loudly. The fully qualifiedcall needs no extra import anchor.
Red first:
× logs the flags React Native read before the override instead of discarding them 11ms.P2-2: the radiogroup owns radio children only
apps/mobile/components/habit-list/move-parent-dialog.tsxandapps/web/components/habits/habit-list/move-parent-overlay.tsxrender thehabits.moveParent.destinationseyebrow above the group rather than inside it, and mobile moves thenoSearchResultstext out as well.role="radiogroup"andaccessibilityRole="radiogroup"expectowned
radiochildren, so a text node inside made TalkBack and NVDA count an item that is not adestination. The eyebrow string stays the group's accessible name on both platforms, so the two
still name the group identically. The change is symmetric and is not a parity break either way.
P3-3: a picker checks its current row once the profile resolves
PickerOptionsseededdraftanddraftReffromselectedonce and never resynced, so a firstrender with no profile left every row unchecked after the value arrived. The draft now starts null
and the rendered value is
draft ?? selected, withcommitDraftfalling back the same way. Focusstill wins over the incoming value, so an in progress choice is never clobbered.
An effect that reset the draft was written first and rejected by the gate:
react-hooks/set-state-in-effectatpreferences-sections.tsx:47andpreference-picker-sheet.tsx:33. The fallback needs no effect and no render time setState.Red first on both platforms, against
f104e11a:P3-2: the unused
disabledprop is gone from both select-check rowsapps/mobile/components/ui/select-check.tsxandapps/web/components/ui/select-check.tsxexported aRadioRowwith adisabledprop that no production caller passed. The only importers arepreferences-sections.tsxandpreference-picker-sheet.tsx, and neither passes it. The prop isdeleted along with the mobile
styles.disabledentry and the web disabled class variants. There isno upcoming caller to name.
The disabled skipping coverage that fixture carried moved to the two suites where a disabled option
is real production data.
apps/web/__tests__/components/habits/move-parent-overlay.test.tsxassertsthat a disabled destination never takes a tab stop and that ArrowDown from the root lands past it on
the next enabled destination.
apps/mobile/__tests__/components/habit-list/move-parent-dialog.test.tsxasserts the disabled destination is not focusable, has no press handler, and is skipped when focus
moves. Both use
components/ui/radio-row, which carriesdisabledfrom the shared contract and iswhat
move-parentrenders.P3-1: the focusable target test counts instead of searching for nothing
apps/mobile/__tests__/components/ui/select-check.test.tsxlooked for a childless host withfocusable === true, which nothing inRadioGroup,RadioRoworRadioGlyphcan render. It nowasserts that the focusable host count equals the option count, so it reddens both if a sentinel
returns and if a row loses
focusable. Proved by settingfocusable: falseinuseRadioGroupItem:P3-6: the dead blur call is deleted
second.props.onBlur?.()in the re-entry case was always a no op, becauseuseRadioGroupItemreturns no
onBlur. It is removed and the case still passes for the right reason: theoutsidefocus is what resets provenance.
A correction to the record, and both halves of it differ from the tree
The round order asked me to record two readings. I read both in this worktree, in installed
react-native@0.86.3, and both come out the other way. Nothing in the order was built on them, sono code changes, but the record should carry what the files say.
Android does map Home and End.
node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/KeyEvent.ktlines
114and115readAndroidKeyEvent.KEYCODE_MOVE_HOME to "Home"andAndroidKeyEvent.KEYCODE_MOVE_END to "End", and the same pair appears again at:143and:144.grep -c "MOVE_HOME\|MOVE_END"on that file returns 4, not 0.KEYCODE_TAB to "Tab"sits at:111and
:140.Home and End are still web only, and the true reason is the one the order gives: mobile consumes no
key events at all.
grep -rn "onKeyDown" apps/mobile --include=*.tsx --include=*.tsoutside__tests__returns 0 hits, so the adapter is web roving tabindex plus ARIA keys against mobileplatform focus search.
Both feature flags default to true, not false.
.../internal/featureflags/ReactNativeFeatureFlagsDefaults.kt:86readsoverride fun enableImperativeFocus(): Boolean = trueand:92readsoverride fun enableKeyEvents(): Boolean = true. The release provider inherits both:ReactNativeFeatureFlagsOverrides_RNOSS_Stable_Android.kt:10-11extendsReactNativeNewArchitectureFeatureFlagsDefaults(), and grepping either class for those two namesreturns 0 hits.
The JavaScript side reads the native value rather than carrying its own default.
src/private/featureflags/ReactNativeFeatureFlags.js:331iscreateNativeFlagGetter('enableImperativeFocus', false), andcreateNativeFlagGetterinReactNativeFeatureFlagsBase.jscallsNativeReactNativeFeatureFlags?.enableImperativeFocus?.()and uses its second argument only when that native module is unavailable. So
ReactNativeElement.focus()atReactNativeElement.js:155-161reachesViewCommands.focuson thisversion without any override.
What that means for this branch, stated plainly: the config plugin's
dangerouslyForceOverride(enableImperativeFocus = true)is not the mechanism that makes entry focuswork on
0.86.3, because the flag is already true by inheritance. It pins an experimental flag theentry redirect depends on, so it stays and it is now covered by the pristine template case. It is
not load bearing today, and this section is the honest version of the claim earlier rounds made.
Test evidence
Every run below was captured to a file. No result is read through a pipe.
Test Files 4 passed (4),Tests 41 passed (41)Test Files 3 passed (3),Tests 24 passed (24)Test Files 9 passed (9),Tests 87 passed (87)Test Files 8 passed (8),Tests 44 passed (44)npx turbo run type-check --force --concurrency=1from the rootTasks: 3 successful, 3 total,Cached: 0 cachednpm run lintfrom the rootTasks: 3 successful, 3 total, 0 errors, 4 pre-existing warningsGITHUB_BASE_REF=redesign/main node tools/check-suppressions-ratchet.mjsThe type-check was forced, so the parameter and prop deletions were checked against every call site
rather than against a cache.
The deferred manifest round can now run, and this pull request will go red until it does
Pull request 1024 is MERGED. It landed on
redesign/mainas0f96925bat headcc5e9c8a, aboutfour minutes after the round order recorded it as unmerged, and it added the
Surface Manifest Driftjob to
.github/workflows/guards.yml. That job runsnode tools/surface-manifest.mjs --checkon anypull request touching
apps/web/orapps/mobile/, which this one does.Measured rather than predicted. Running the base copy of the checker against this worktree with the
base manifest in place exits 1:
m-overlay-ui-selection-fieldis this branch's deletion. Theblock-chat-*entries it also listscome from this branch sitting one commit behind the base and will resolve in the merge forward, so
the exact counts after that merge will differ.
This round did not take it, because the round order deferred it twice in writing and named it as its
own small round: merge
redesign/mainforward, regenerate the manifest, delete the dead"components/ui/selection-field"entry fromMOBILE_OVERLAY_BASESattools/surface-manifest.mjs:103,and move
m-overlay-ui-selection-fieldinto thedeletedsection oftools/redesign-groups.jsonwith the decision that removed it. That round can run now.
One follow-up to record rather than fix
CLOSED in round 16, recorded here as it stood at the time. This round noted that the web time
picker was
role="listbox"withrole="option"while mobile wasaccessibilityRole="radiogroup"with
radio, and deferred it. Round 16 converted web ontoRadioGroupwithrole="radio"andaria-checkedinstead of filing it, because this pull request already edited both files and theparity rule put it here.
The design review of this round, and the two breaks it found
design-reviewerwas re-run against398103ae's diff alone, because moving the eyebrow is the oneuser visible change in it. It found two real DESIGN.md breaks in the element this round relocated,
both confirmed against the spec before the fix and both fixed in
29998d8d.The web eyebrow was in the wrong type family.
DESIGN.md:720defines the eyebrow role as GeistMono 12/500, 1.20 line height, +0.08em uppercase on
fg-3.eyebrowStyleinmove-parent-overlay.tsxusedvar(--font-sans), while the mobile copy already usedGeistMono_500Medium, so the same relocated string rendered in two families across platforms. It nowuses
var(--font-mono), whichapps/web/components/ui/section-label.tsx:6has always used for thesame role. The review called this a parity break introduced by the move; the wrong family predates
the move, and the move is what made the two copies comparable. Either way it is a break and it is
fixed.
The mobile eyebrow sat one rhythm unit above its own group.
DESIGN.md:351requires the gapbetween two groups to be at least twice the gap within one.
styles.eyebrow.marginBottomwas 4 andstyles.moveOptionsContent.gapis 4, so a label meant to head the whole destination list read as onemore row. The bottom margin is now 8. The web side already cleared this: its column supplies
gap: 8against an internalgap: 4.The review's third note, a leftover
margin: '2px 0 0'on the web eyebrow from its old nestedposition, is fixed in the same commit. The column gap is now the only spacing mechanism there.
It recorded no finding on the accessibility naming itself, on the eyebrow reading correctly as a
label over the whole group including the root row, on the
disableddeletion, or on thedraft ?? selectedfallback.Assumptions
MAIN_APPLICATION_TEMPLATEwith the plugin's own additionsremoved, rather than a copy of
expo-template-bare-minimum. That package is not installed in thisworktree and
apps/mobile/android/on this machine is a stale tree an older plugin revisionalready patched, so neither could serve as the source. The round order specified this derivation.
disabledprop takes the select-check suites' disabled cases with it, so theequivalent coverage moved to the two move-parent suites rather than being dropped. The rejected
alternative was keeping a prop with no production caller purely to host a test.
key. The rejected alternatives were an effect, which the lint gate blocks, and a key on
selected,which would discard focus inside the group every time the value arrives.
Round 15: the manifest catch-up, and the node_modules edit behind the react-native dispute
Head
24b2f7f6. Every figure in this section comes from a run made in this worktree after thereinstall. No number is carried over from an earlier round.
The reinstall did not repair the tree, so I measured instead
npm installreportedremoved 11 packages, and audited 1680 packages in 7sand leftreact-nativeuntouched, because the lockfile entry already matched what was on disk. BothReactNativeFeatureFlagsDefaults.kt:86and the 6779 byteKeyEvent.ktstill read the way thisbranch had recorded them.
So I scanned every file under
node_modules/react-nativefor an mtime later than the package's ownextraction. Exactly two came back:
Nothing else in the package moved. Those two files were hand edited three hours after install, which
is the
patch-packageround this ticket reverted in git on 2026-09-16 and never reverted insidenode_modules. The rootpackage-lock.jsonandnode_modules/.package-lock.jsonagree on onereact-nativeentry,0.86.3, with one integrity hash, so npm had no reason to re-extract.rm -rf node_modules/react-native && npm installreportsadded 16 packages, and audited 1680 packages in 25sand restores the published bytes. The worktree now agrees with the other fivecheckouts on this machine:
ReactNativeFeatureFlagsDefaults.kt:86isoverride fun enableImperativeFocus(): Boolean = false:92isoverride fun enableKeyEvents(): Boolean = falseKeyEvent.ktis 6547 bytes,grep -c "MOVE_HOME\|MOVE_END"returns 0, andKEY_NAME_MAPruns:132to:154, ending onKEYCODE_DPAD_UP,DOWN,LEFTandRIGHTKEYCODE_TAB to "Tab"is real and present in that map#601owns the root cause. Its symptom in this worktree was a hand edit to two files, not adifferent install.
Two corrections to this body, both against the restored bytes
The section titled
A correction to the record, and both halves of it differ from the treeiswithdrawn on both halves, and so is the
Correction, made at 8ddcca0esentence under Round 3.Homeand noEnd. The## Platform evidenceline saying those keys stayweb only was right from the start.
false. That makes the config plugin the mechanism the entry redirectdepends on rather than a redundant pin:
ReactViewManager.kt:472-476gateshandleFocusbehindReactNativeFeatureFlags.enableImperativeFocus(), which is off until the plugin turns it on. Theearlier claim that the plugin was "not load bearing today" is wrong and is withdrawn with it.
The suites re-run on the restored bytes
The reinstall changes what a mobile test sees, so both halves ran again before anything else changed.
No assertion moved.
Test Files 8 passed (8),Tests 80 passed (80)Test Files 7 passed (7),Tests 37 passed (37)The manifest, regenerated rather than hand merged
origin/redesign/maincarries pull request 1024, soSurface Manifest Driftis live and this branchwent red on a surface whose source file it deletes.
node tools/surface-manifest.mjsnow writes 185 surfaces and 796 cells, one fewer surface than thecommitted 186. Comparing the two parsed manifests by id, the regeneration removes exactly
m-overlay-ui-selection-fieldand adds nothing.node tools/surface-manifest.mjs --checkreportsthe committed inventory matches this tree (185 surfaces, 796 cells).m-overlay-ui-selection-fieldmoves out ofOrbit Sobreposicoesintools/redesign-groups.jsoninto
deleted, with the decision that removed it.deletedis the right bucket becausetools/redesign-coverage.mjs:120refuses a deleted id that is still present in the manifest, and itno longer is.
node tools/redesign-coverage.mjsexits 0.The dead
MOBILE_OVERLAY_BASESentry, and what removing it exposed"components/ui/selection-field"is gone fromtools/surface-manifest.mjs, with the comment thatdescribed callers the tree does not have. A grep for
selection-fieldacrossapps/**/*.tsxandapps/**/*.tsreturns zero hits on this branch, because the component and its test are deleted here.With that entry gone the web and mobile base lists hold the same value, so both call sites now read
the single
OVERLAY_BASESrather than two aliases for one list.The round order expected
tools/__tests__/surface-manifest.mjsto stay green through this and itdoes not. Removing the entry turns
FAIL a caller reaching Sheet only through the selection-field wrapper is inventoriedred, becausethe fixture stages a
frequency-fieldwhose only route toSheetis that wrapper, andisOverlaySourceis direct import only. The case tests a real mechanism, so it keeps testing itagainst a wrapper the tree still has: the fixture wrapper is now
confirm-sheet, which is inOVERLAY_BASES. A fixture naming a file no tree has would test nothing.The determinism assertion the drift gate rests on, shown green over a dead process
tools/__tests__/surface-manifest.mjsdiscarded bothrun(...)results around its byte comparison,and the generator writes nothing on a non-zero exit. So a crashed second run left the first run's
bytes on disk and the comparison agreed with itself.
Proved with a
[DEBUG-c7d1]probe at the top ofbuildManifestthat throws on the third derivationonly, so the case module still reaches the comparison:
PASS two runs over one unchanged tree write the same bytesPASS, andFAIL the second determinism run writes a manifestbeside itORBIT TOOLS GATE OKBoth derivations now assert
status === 0before the comparison.grep -rn "DEBUG-c7d1" tools/ .claude/returns nothing.
The five comment and correctness items 1024's review left owed
trackedFilesde-duplicates through aSet.git ls-filesprints an unmerged path once perindex stage, so a regeneration during an unresolved widget conflict wrote that path into
ownedFilestwo or three times. Unreachable in CI, reachable locally.and the machine, and names
chatBlockEntriesas the owner of the cross-platform ordering class itcannot see.
chatBlockEntriesstates the real condition. The order flips only when the character after theshared prefix sorts between
/(0x2F) and the backslash (0x5C), so a siblingchat2.tsxflipsand a sibling
chat-header.tsxnever does.local.propertiescomment says the file is excluded for being untracked, not for beingignored, because
git ls-filesreads the index and never consults ignore rules. The fixture.gitignoreentry changes no outcome and the comment now says so, so nobody later reads the caseas licence to swap
git ls-filesforgit check-ignore.android/(build|.gradle|.cxx)exclusion stays, and its new comment says why: root.gitignoremeans git never lists one of those paths, so the filter now only guards a build artifact somebody
force added. The comment does not cite the fixture, which would be circular.
The fixture staging for
local.propertiesand the widget build trees is otherwise untouched.Round 15 assumptions
origin/redesign/mainforward at0f96925brather than rebasing onto it. Arebase would need a force push, which the delivery boundaries forbid, and this branch already
carries merge commits from earlier rounds.
git merge-treereported no conflict and the mergeneeded no resolution. The rejected alternative was the literal rebase the round order asked for.
WEB_OVERLAY_BASESis deleted alongside the mobile entry rather than kept as an alias forOVERLAY_BASES. Two names for one value is the "just in case" export code standard 2 bans. Therejected alternative was keeping both names so the lists could diverge again later.
confirm-sheetfixture wrapper is named fromOVERLAY_BASESrather than invented, so thecase covers the same mechanism with a module the real tree still has.
Round 15 verification, every run captured to a file
CLAUDE.mdrequires both harnesses after atools/**change and neither runs in CI, so these twolocal runs are the only proof.
node tools/test-tools.mjsORBIT TOOLS GATE OK,Assertions: 1811 | Elapsed: 798.649s, 0 failuresnode .claude/hooks/test-hooks.mjsORBIT HOOKS OK, 455 passing checks, 0 failuresnpx turbo run type-check --force --concurrency=1Tasks: 3 successful, 3 total, forcednpm run lintTasks: 3 successful, 3 total, 0 errors, 9 pre-existing warningsnode tools/surface-manifest.mjs --checkthe committed inventory matches this tree (185 surfaces, 796 cells)node tools/redesign-coverage.mjsGITHUB_BASE_REF=redesign/main node tools/check-suppressions-ratchet.mjs14 -> 14web,16 -> 16mobile, unchangedThe type-check was forced. No result above is read through a pipe.
The branch's own diff against
origin/redesign/mainnames 41 files. This round touches four of them:tools/surface-manifest.mjs,tools/__tests__/surface-manifest.mjs,tools/redesign-groups.jsonand
.claude/manifests/surfaces.json.Still open, and named so it is not lost
CORRECTED in round 16. This block previously said the web time picker was
role="listbox"withrole="option"while mobile wasradiogroupwithradio. That split is closed.a6b71b43moved web onto
RadioGroupplususeRadioGroupItemwithrole="radio"andaria-checked, and thescroll-into-view query moved to
[role="radio"][aria-checked="true"]. Both platforms now declare thesame widget. What remains open is
#608and#609, filed rather than widened into this branch.Round 16: one widget contract, a guard that can actually fail, and the plugin split
Round 15 shipped an absence assertion that could not fail on the axis that mattered. An independent
review at
24b2f7f6found it, and this round closes it along with the widget-contract split above.The absence assertion could not fail on the horizontal axis, and now it can
apps/mobile/__tests__/components/ui/time-field.test.tsx:271-292previously read:hourHandlesholds only the handles inside the SAME column. Restore the exact defect an earlierround shipped,
nextFocusLeftandnextFocusRighton each hour cell pointing at the adjacentminutes column, and those handles are not in
hourHandles, so the filter is empty and the teststays green. Horizontal is the axis that trapped the D-pad, and it was the one axis the
assertion could not see.
It now collects the host nodes of BOTH columns and asserts
props[direction] === undefinedfor allfive
nextFocus*props, withlength > 1guards so.every()cannot be vacuous.One widget contract, on both platforms
Web moved onto
RadioGroupplususeRadioGroupItemwithrole="radio"andaria-checked,matching mobile. And the mobile half was wrong in a way nothing caught:
accessibilityState={{ selected }}on anaccessibilityRole="radio"never makes the node checkable.Confirmed against the installed source at
node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.kt:632-640,where only the
checkedkey setsisCheckableandisCheckedwhileselectedsetsisSelectedalone. So TalkBack announced neither "checked" nor "not checked" on a radio.
All five
accessibilityRole="radio"sites now usechecked:radio-row.tsx:221,227,segmented-control.tsx:35,select-check.tsx:114,time-field.tsx:79,goal-type-selector.tsx:70.Zero radios are left on
selected.The rest of the round
with-react-native-imperative-focus.js:9suppliesimport com.facebook.react.common.ReleaseLevel,and
TEMPLATE_WITHOUT_RELEASE_LEVELstrips it from the fixture so only the plugin can supply it.That removes the fixture-agrees-with-the-guess shape code standard 8 forbids.
with-android-release-build-fixes.jsinto its own file and suite.That file and its test are now byte-identical to
redesign/main.usePreviousFocusTarget()returning null is no longer ambiguous:onFocus = getPreviousFocusTarget ? handleFocus : nullplus a standalone fallback atradio-row.tsx:117,165-172, with a test.LANGUAGE_OPTIONS.find(...)is deleted;selectedLanguageisSupportedLocale,narrowed once in
use-preference-controls.ts:39.MotionPressableloses itsfocusRefprop, which was always identical toref, and its onecaller passes
ref. Three converted web controls gain an explicit focus-visible ring.One visual change, named rather than hidden
preferences-sections.tsx:113-118againstpreferences-styles.ts:50-52:timeZoneOptionssetsgap: 8and the rows are now wrapped in theRadioGroupview, so they sit flush instead of carrying8px of air. This is kept rather than restored: web was already flush, and
DESIGN.md:776makes thehairline the only owner of row separation.
Round 16 test evidence
The absence assertion, proved red and green in both directions. The unchanged test was run
against a shimmed
@/components/ui/radio-rowthrough an alias override in a scratchpad config, withnothing in the repository touched, restoring the exact defect:
nextFocusLeft: 90001,nextFocusRight: 90002, handles deliberately outside the hour column's own tag set.Test Files 1 passed (1),Tests 11 passed (11)1 failed / 10 passed,AssertionError: expected false to be trueattime-field.test.tsx:29024b2f7f6, same defect restoredThe third row is the proof that matters: the old assertion was green with the D-pad trap present.
Gates, all from the worktree at this head:
Round 16 follow-ups, filed rather than widened
#608, twoRadioRowimplementations and the unselected glyph at1.5px --hairline-strongagainst
DESIGN.md:778's2px --track-empty. Entirely pre-existing onredesign/main.#609, the reminder timing and frequency unit pickers. TheiraccessibilityRole="button"withaccessibilityState={{ selected }}is correct usage for a selectable button, so it is a differentsubject from the radio fix above.
packages/shared/src/utils/radio-navigation.tsis tested only fromapps/web, against D84. Sharedcoverage passes today at 95.3 statements against a 95 threshold, which is 0.30 points of margin.
app.config.jsstill registers the focus plugin. Drop it from thewithLocalPluginchain andenableImperativeFocus()stays false, making every Android radiogroup's
focus()a silent no-op while every test stays green.One correction to the salvage commit message
74b891a3says the radio conversion "replaced the browser's default focus ring on the web pickers,so a keyboard user lost the only cue". All three touched controls were already
<button>elementsand
globals.csscarries no rule stripping their outline. Nothing was lost; the commit ADDS anaccent ring where the browser default was showing. The change is right and the claim was not.
Review substitution
Pullfrog could not review this pull request: it and Codex share one OpenAI meter, exhausted until
2026-09-22 07:23. A separate agent reviewed at this exact head, in the worktree carrying it, and its
full review is posted as a pull request comment. That substitution is named here so nobody later
reads this merge as having had the usual reviewer.