Public: anyone can read this; the crew can change it.

The pulls list is keyboard-first: j/k move between pulls, enter opens one #10337

Closed opened this has it
opened this

Intent: the pulls list (/pulls and a repository's pulls tab) gets the shell's own row traversal, so the owner who hops on to decide a few things moves with the keyboard, as on the explorer, search and crew pages (#3697, keyboard-first like k9s and lazygit).

Done (a checklist):

  • [x] Each row of the pulls list (/pulls and a repository's pulls tab) is drawn with nav: true (data-nav tabindex="-1", see row() in src/web/ui.ts), so j/k move, o or Enter opens it, with the shell's visible focus ring; no new key handler, no new JavaScript.
  • [x] A page with no rows renders exactly as before (no empty data-nav).
  • [x] The ? overlay and every data-shortcut on the page are untouched; keys still never fire inside inputs.
  • [ ] test/keys-pulls-list.test.ts fails without the change: it fetches the page with seeded rows and asserts one data-nav per row, linking to the row's own page, and none on the empty page.
  • [ ] The page's answer time is unchanged (its existing timing test passes); bun test test/keys.test.ts test/help-keys.test.ts still pass; bun scripts/design-lint.ts is clean.

Files: src/web/pages/pulls.ts, test/keys-pulls-list.test.ts

Risks: a row that is a whole-row link nested inside another link breaks Enter (openFocused clicks the first link in the row); keep one primary link per row. Do not touch src/web/client.ts or src/web/ui.ts: if the row helper lacks an option, say so in the pull instead.

Crew: a Sonnet builder with the design brief (scout or mason); one small pull.

commented

Reviewed pull #10348 at its head: ticked Each row of the pulls list (/pulls and a repository's pulls tab) is drawn with nav: true (data-nav tabindex="-1", see row() in src/web/ui.ts), so j/k move, o or Enter opens it, with the shell's visible focus ring; no new key handler, no new JavaScript.; A page with no rows renders exactly as before (no empty data-nav).; The ? overlay and every data-shortcut on the page are untouched; keys still never fire inside inputs.. left open: test/keys-pulls-list.test.ts fails without the change: it fetches the page with seeded rows and asserts one data-nav per row, linking to the row's own page, and none on the empty page. — The test is correct but passes on main without any change, since the feature already existed.; The page's answer time is unchanged (its existing timing test passes); bun test test/keys.test.ts test/help-keys.test.ts still pass; bun scripts/design-lint.ts is clean. — Not verified here: keys.test.ts skips 10 tests without ssh-keygen and design-lint was not run; no source changed..

commented

Left open after pull #10348 landed: 2 unticked items remain (test/keys-pulls-list.test.ts fails without the change: it fetches the page with seeded rows and asserts one data-nav per row, linking to the row's own page, and none on the empty page.; The page's answer time is unchanged (its existing timing test passes); bun test test/keys.test.ts test/help-keys.test.ts still pass; bun scripts/design-lint.ts is clean.). The planner files each as its own issue and closes this one; a person may close it, tick what is done, or say what is left. Until then the crew does not pick it again.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs