Skip to content

fix(mobile): hardware keyboard shortcuts work as soon as their screen opens - #29679

Open
saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:fix/mobile-keyboard-command-registrations
Open

saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:fix/mobile-keyboard-command-registrations

Conversation

@saphid

@saphid saphid commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Problem

On iPad and Android with a hardware keyboard, shortcuts that a screen registers don't work when that screen first opens. Examples are ⌘F (Find), ⌘\ (toggle sidebar), ⌘1–9 (jump to thread) and ⇧⌘H (next machine). For example, ⌘F does nothing on Home right after launch. The shortcut only starts working after you navigate somewhere else, and then the new screen's own shortcuts are missing instead.

HardwareKeyboardCommandProvider builds enabledCommands in a useMemo that lists registrationVersion as a dependency but never reads it. Since #20967, Metro runs the React Compiler on mobile. The compiler memoizes on the values a callback reads and drops unread entries from the dependency list, so the compiled memo only recomputes when pathname, navigation or the active thread change. Screens register their commands in effects that run after the provider renders, so the list sent to native is always one registration behind. Native iOS (T3KeyboardCommandsView.keyCommands) and Android (dispatchKeyEvent) both ignore any command that isn't in enabledCommands.

Change

This uses the same fix as #28169 (useThreadListV2's observeInbox(threads, _expiredGraceVersion)). A small registeredCommands(_registrationVersion) helper reads the version, so the compiled memo depends on it. The react/memo-dependencies lint disable is no longer needed, so I removed it. After the change, the compiled cache check is $[13]!==activeThreadRef||$[14]!==navigation||$[15]!==pathname||$[16]!==registrationVersion.

Scope and approval

This is a very small, focused fix for an obvious bug, so there's no prior issue. The provider already subscribes to the registry for this purpose, and the lint-disable comment states the intent: "registrationVersion invalidates the snapshot of the external keyboard-command registry". The React Compiler silently dropped that behavior. I found it while checking which merged fixes have siblings with the same root cause; #28169 is the same defect in another memo.

Overlap search: open #22135 reduces how often thread-jump shortcuts re-register. It changes hardwareKeyboardCommands.ts and threadKeyboardShortcuts.ts, not this memo, so the two don't conflict. I found no PR that touches enabledCommands.

Verification

  • New test in the mobile react vitest project, which compiles sources with the release React Compiler. It renders the provider with a child that registers focusSearch. On main it fails: AssertionError: expected [ 'newTask', 'commandPalette' ] to include 'focusSearch'. With the fix it passes.
  • vp test run src/features/keyboard/HardwareKeyboardCommandProvider.test.tsx src/features/keyboard/hardwareKeyboardCommands.test.ts src/reactCompilerCoverage.test.ts (apps/mobile): 3 files, 1015 tests passed.
  • vp run check:changed passed: format, lint, knip, typecheck (2 packages), tests and the React compiler check.
  • Not checked: a real iPad or Android device with a hardware keyboard.

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on high. Sol verdict: approved.

Created with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

… opens

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a587f51

Macroscope's review found this PR approvable — Existing hardware keyboard shortcuts refresh when their screen registers them, without changing command actions or screen filters. The change is narrowly scoped and removes a lint suppression rather than adding one.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8648aea and a587f51.

Files selected for processing: 3
  • apps/mobile/src/features/keyboard/HardwareKeyboardCommandProvider.test.tsx
  • apps/mobile/src/features/keyboard/HardwareKeyboardCommandProvider.tsx
  • apps/mobile/vite.config.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

The provider retrieves registered hardware keyboard commands using the registration version. A new test checks that focusSearch reaches the native command component, and the React test project includes that test.

Changes: Hardware keyboard command registration

Layer / File(s) Summary
Versioned command registration and test
apps/mobile/src/features/keyboard/HardwareKeyboardCommandProvider.tsx, apps/mobile/src/features/keyboard/HardwareKeyboardCommandProvider.test.tsx, apps/mobile/vite.config.ts
The provider retrieves registered commands using the registration version. The test checks that the native command component receives focusSearch, and the React test project includes the test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to a587f

The change addresses screen-specific shortcut registration, with no actionable merge-blocking risk identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the change and the symptom it fixes: hardware keyboard shortcuts on mobile now work as soon as their screen opens. The changeset adds a registrationVersion-dependent helper in Hardware…
Description check Passed The description follows the repository template. It has Problem, Change, Scope and approval, and Verification sections. The Problem section gives the symptom, reproduction examples, and root cause. Th…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the change and the symptom it fixes: hardware keyboard shortcuts on mobile now work as soon as their screen opens. The changeset adds a registrationVersion-dependent helper in HardwareKeyboardCommandProvider.tsx, which is the cause the title describes. The title is specific and follows the conventional commit form, so it clearly summarizes the main change.

Full details: Description check

Explanation

The description follows the repository template. It has Problem, Change, Scope and approval, and Verification sections. The Problem section gives the symptom, reproduction examples, and root cause. The Change section explains the helper and the lint-disable removal. The Scope section justifies a small fix without a prior issue and links related PR #28169 and #22135. The Verification section lists the new test, the failing-then-passing result, the commands run, and what was not checked on real hardware. The description does not include a model and harness line in the template's required form, but it does name Claude Opus 5.5 and Claude Code. Its Scope section also cites a PR number from the author's own repository state, which a reviewer should confirm. Overall the description is mostly complete.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant