Skip to content

feat(shortcuts): add Cmd+1-9 to jump to task by sidebar position#97

Merged
johannesjo merged 3 commits intojohannesjo:mainfrom
brooksc:task/command-shortcut-8959d8
May 6, 2026
Merged

feat(shortcuts): add Cmd+1-9 to jump to task by sidebar position#97
johannesjo merged 3 commits intojohannesjo:mainfrom
brooksc:task/command-shortcut-8959d8

Conversation

@brooksc
Copy link
Copy Markdown
Contributor

@brooksc brooksc commented May 2, 2026

Summary

  • Adds Cmd+1 through Cmd+9 keyboard shortcuts to jump directly to the Nth task in the sidebar
  • Shortcuts use the visual sidebar order (project-grouped, active before collapsed) so the number matches what the user sees
  • Shortcuts are marked global: true so they fire even when a terminal has focus

Implementation

  • src/store/navigation.ts — new jumpToTask(index) function using computeSidebarTaskOrder()
  • src/lib/keybindings/defaults.ts — 9 new bindings (app.nav.jump-to-task-1app.nav.jump-to-task-9)
  • src/store/store.ts — exports jumpToTask through the store barrel
  • src/App.tsx — wires the 9 action strings to handlers

Test plan

  • Open app with 3+ tasks and press Cmd+2 — second task in sidebar becomes active
  • Press Cmd+1 — first task becomes active
  • Press Cmd+9 with fewer than 9 tasks — no crash, no change
  • Inside a terminal, press Cmd+2 — still switches task (global shortcut)
  • npm test — all 312 tests pass (5 new tests in src/store/navigation.test.ts)

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@johannesjo
Copy link
Copy Markdown
Owner

johannesjo commented May 5, 2026

Thank you very much for this!

Code Review

Reviewed end-to-end (binding wiring, matchers, Linux behavior, tests). Two critical findings before this can merge — the headline one is that the shortcut never fires on any platform.

Critical — shortcut never fires

src/lib/keybindings/defaults.ts uses key: `Digit${i + 1}` , but both matchers compare against KeyboardEvent.key, not KeyboardEvent.code:

  • src/lib/keybindings/match.ts:13e.key.toLowerCase() \!== binding.key.toLowerCase()
  • src/lib/shortcuts.ts:27 — same comparison

For the digit row, e.key === "1"; "Digit1" is the code. So "1" === "digit1" is always false and the binding never matches. The existing app.reset-zoom correctly uses key: '0' (defaults.ts:260) — same convention applies here.

Fix: key: `${i + 1}` .

Critical — test does not exercise the binding

src/store/navigation.test.ts mocks ./core and ./sidebar-order and calls jumpToTask() directly. It proves the trivial order[index] lookup but never touches matchesKeyEvent / matches / the App.tsx handler map. That's why the Digit1 bug slipped through.

Suggestion: add one integration test that dispatches a synthetic KeyboardEvent("keydown", { key: "1", ctrlKey: true }) after registerFromRegistry(DEFAULT_BINDINGS, handlers) and asserts the handler is called.

Important — Linux + non-US layouts

Once key is fixed to "1""9", the binding still misses on layouts where the digit row requires Shift (AZERTY, several EU layouts). The existing Cmd+0 reset-zoom binding documents this and registers a shift: true variant in src/lib/shortcuts.ts:87-94. Same treatment needed here — either (a) register shift variants for each digit, or (b) match on e.code for layout-independence (larger change; would ironically have justified the original Digit1 value).

On Linux, cmdOrCtrl resolves to Ctrl (match.ts:19-20), so this intercepts Ctrl+1Ctrl+9 globally — including inside xterm.js (TerminalView.tsx:237). Follows the precedent of Ctrl+0, but the test plan only covers Cmd+1/2/9 and should mention Linux explicitly.

Minor

  • src/store/sidebar-order.ts:40-49 includes collapsed tasks in the order — Cmd+1 may activate a task hidden inside a collapsed project, which is surprising. Worth confirming intent and adding a test for the collapsed case.
  • src/App.tsx:456-458 — spreading nine generated entries into actionHandlers works but reads differently from surrounding static entries; nine explicit entries (or a single jumpToTask action that parses a numeric arg) would be more consistent.
  • defaults.ts:79-89 — nine near-identical "Jump to task N" descriptions will look noisy in any keybindings UI.

Verdict

Needs changes. After the key fix + an integration test + the shift-variant story for non-US layouts, this is a small, low-risk PR.

…integration tests

- Bindings used e.code format ("Digit1") but matchers compare against e.key ("1"),
  so jump-to-task shortcuts never fired on any platform
- Add shift-key variants for each digit so AZERTY/non-US layouts work (mirrors
  the existing Cmd+0 reset-zoom shift variant pattern)
- Add integration tests in shortcuts.test.ts that verify the binding fires on
  key="1" and does not fire on key="Digit1" (regression guard)
- Update defaults.test.ts expected IDs to include jump-to-task and shift variants

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@brooksc
Copy link
Copy Markdown
Contributor Author

brooksc commented May 6, 2026

Changes in response to review

Critical fix — shortcut never fired: Changed key: \Digit${i+1}`tokey: `${i+1}`indefaults.ts. Matchers compare against KeyboardEvent.key(e.g."1"), not KeyboardEvent.code(e.g."Digit1"`), so the binding never matched.

AZERTY / non-US layout support: Added shift variants for each digit (e.g. app.nav.jump-to-task-1-shift with modifiers: { cmdOrCtrl: true, shift: true }), following the same pattern already used for the Cmd+0 reset-zoom shortcut.

Integration tests: Added two tests to shortcuts.test.ts that go through the full registerFromRegistry → initShortcuts → keydown path: one confirming key="1" fires the handler, one confirming key="Digit1" does not (regression guard for the original bug).

Tested and working — Cmd+1 through Cmd+9 now correctly jump to tasks by sidebar position.

…f registry

- jumpToTask now indexes store.taskOrder (left-to-right tile order in the
  main area), so Cmd+N matches what the user sees in the tile layout and
  what Cmd+Left/Right cycles through. Previously it indexed
  computeSidebarTaskOrder which regroups by project, causing Cmd+N to land
  on a different task than the visually Nth tile when projects are
  interleaved. This also incidentally fixes the collapsed-task surprise:
  taskOrder excludes collapsed tasks, so Cmd+N can no longer activate a
  task hidden inside a collapsed project group.
- Move the AZERTY shift variants from defaults.ts to a new
  registerJumpToTaskShortcuts() in shortcuts.ts, mirroring the Cmd+0
  reset-zoom precedent. This keeps the keybindings UI from showing 9
  duplicate "Jump to task N" rows.
- Add an integration test for the AZERTY shift path (key="1", shift=true)
  and a unit test pinning that jumpToTask ignores collapsedTaskOrder.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@johannesjo
Copy link
Copy Markdown
Owner

johannesjo commented May 6, 2026

Thank you very much!

Pushed 8738eba directly to the branch — two changes, both UX-focused:

1. Cmd+N now indexes store.taskOrder, not the project-grouped sidebar order.

The previous jumpToTask walked computeSidebarTaskOrder(), which regroups tasks by project (per-project active, then per-project collapsed, then orphans). When projects are interleaved in taskOrder, that order diverges from the L→R tile layout in the main area and from the order Cmd+Left/Right cycles through. So pressing Cmd+2 could activate a different task than the visually second tile, which is hard to predict and inconsistent with the rest of the navigation model.

Now all three navigation paths agree: Cmd+1..9, Cmd+Left/Right, and tile order are the same sequence. As a side effect, taskOrder excludes collapsed tasks, so Cmd+N can no longer activate a task hidden inside a collapsed project group (the Minor #1 from the earlier review).

2. AZERTY shift variants moved out of the keybindings registry.

The shift variants in defaults.ts made the Keyboard Shortcuts dialog show 18 jump-to-task rows (9 with Cmd+N, 9 duplicate "Jump to task N" rows with Cmd+Shift+N). They now live in a new registerJumpToTaskShortcuts() in shortcuts.ts, mirroring how the Cmd+0 reset-zoom shift variant is registered imperatively rather than via the registry. The UI is back to 9 rows.

Tests: added one unit test pinning jumpToTask ignores collapsedTaskOrder, and one integration test for the AZERTY path (key="1", shiftKey: true). All 316 tests pass; lint/typecheck/prettier clean.

Not included: the sidebar 19 indicator polish (touches Sidebar.tsx rendering — more a polish change than a correctness fix, happy to do as a follow-up).

@johannesjo
Copy link
Copy Markdown
Owner

Thank you! <3

@johannesjo johannesjo merged commit 9932aa4 into johannesjo:main May 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants