From 8595b40ac452339c30cdcaf46d116d88bd094e1b Mon Sep 17 00:00:00 2001 From: whackur Date: Tue, 1 Sep 2026 16:37:50 +0900 Subject: [PATCH 01/17] feat(input): cycle projects with Ctrl+Shift+arrows Stepping between project tabs had no chord of its own: the F-keys jump to a tab by number, which does not reach past ten and says nothing about where the neighbours are. The step is sent as a direction, not an index. The handler holds one project, so the wrap needs the tab count and which tab is in front, and both are only known where the tab list is. Switching stays a request: the tab moves when the daemon rebroadcasts the set. --- src/application/input/dispatch.rs | 12 +++++ src/application/session_link.rs | 25 +++++++++++ src/application/session_link_tests.rs | 65 ++++++++++++++++++++++++++- src/application/tests/workspace.rs | 20 +++++++++ src/input/mod.rs | 6 +++ src/input/routing.rs | 6 +++ src/input/tests/common.rs | 4 ++ src/input/tests/key_map_tests.rs | 37 ++++++++++++++- 8 files changed, 172 insertions(+), 3 deletions(-) diff --git a/src/application/input/dispatch.rs b/src/application/input/dispatch.rs index 8ecd4383..e5a19f96 100644 --- a/src/application/input/dispatch.rs +++ b/src/application/input/dispatch.rs @@ -26,6 +26,14 @@ pub(crate) enum KeyOutcome { #[derive(Debug, PartialEq, Eq)] pub(crate) enum ProjectRequest { Switch(usize), + /// Step one tab forward or backward, wrapping over tab order. + /// + /// A direction rather than a resolved index because the sender holds one + /// project: the wrap needs the tab count and which tab is in front, and + /// both are only known where the tab list is. + Cycle { + forward: bool, + }, Close, Open(String), OpenDialog, @@ -139,6 +147,10 @@ pub(super) fn handle_global_action(app: &mut App, action: Action) -> Option Some(KeyOutcome::Project(ProjectRequest::OpenDialog)), Action::CloseProject => Some(KeyOutcome::Project(ProjectRequest::Close)), Action::SwitchProject(idx) => Some(KeyOutcome::Project(ProjectRequest::Switch(idx))), + Action::PrevProject => Some(KeyOutcome::Project(ProjectRequest::Cycle { + forward: false, + })), + Action::NextProject => Some(KeyOutcome::Project(ProjectRequest::Cycle { forward: true })), Action::ToggleFullscreen => { match app.focus { Focus::DiffViewer => app.toggle_diff_fullscreen(), diff --git a/src/application/session_link.rs b/src/application/session_link.rs index 6e094701..04084bd5 100644 --- a/src/application/session_link.rs +++ b/src/application/session_link.rs @@ -90,6 +90,12 @@ impl SessionLink { None => return, } } + // Resolved to an id here rather than to an index sent onward, + // because the wrap and the tab it lands on are the same question. + ProjectRequest::Cycle { forward } => match cycle_target(ws, forward) { + Some(id) => self.client.focus_repo(&id), + None => return, + }, // The dialog is this client's own; only what it confirms is a // request. ProjectRequest::OpenDialog => { @@ -150,6 +156,25 @@ fn focus_repo(ws: &mut Workspace, repo: &str) -> bool { } } +/// The catalog id of the tab one step from the front, wrapping over tab order. +/// +/// `None` for every case with nothing to ask about: no tabs, a single tab — +/// where either direction lands back on the one already in front — and a tab +/// the daemon has not named yet, the same early-out closing has. Takes +/// `&Workspace` because stepping is a request; the tab moves when the daemon +/// rebroadcasts the set, never here. +fn cycle_target(ws: &Workspace, forward: bool) -> Option { + let len = ws.projects().len(); + if len <= 1 { + return None; + } + // Backward as a forward step of `len - 1` so the arithmetic stays in + // unsigned space and wrapping past zero needs no special case. + let step = if forward { 1 } else { len - 1 }; + let target = (ws.active_index() + step) % len; + ws.projects()[target].repository_id().map(str::to_string) +} + /// Raise a terminal refusal on the tab it came from, not the active one: the /// client subscribes to every open repository, so the refusal may be about a /// tab the user is not looking at. A repository with no tab yet falls back to diff --git a/src/application/session_link_tests.rs b/src/application/session_link_tests.rs index 4c5e5eb4..ca18a640 100644 --- a/src/application/session_link_tests.rs +++ b/src/application/session_link_tests.rs @@ -1,4 +1,4 @@ -use super::{focus_repo, notify_repo}; +use super::{cycle_target, focus_repo, notify_repo}; use crate::app::App; use crate::app::tests::app_with_files; use crate::workspace::Workspace; @@ -93,3 +93,66 @@ fn a_refusal_for_a_repository_with_no_tab_is_still_shown() { assert!(ws.projects()[0].notice.is_some()); } + +#[test] +fn stepping_forward_from_the_last_tab_wraps_to_the_first() { + let mut ws = workspace_on(&["/a", "/b", "/c"]); + ws.switch(2); + + assert_eq!(cycle_target(&ws, true), Some(id_of("/a"))); +} + +#[test] +fn stepping_backward_from_the_first_tab_wraps_to_the_last() { + let mut ws = workspace_on(&["/a", "/b", "/c"]); + ws.switch(0); + + assert_eq!(cycle_target(&ws, false), Some(id_of("/c"))); +} + +#[test] +fn stepping_between_two_tabs_alternates_in_both_directions() { + let mut ws = workspace_on(&["/a", "/b"]); + ws.switch(0); + + assert_eq!(cycle_target(&ws, true), Some(id_of("/b"))); + assert_eq!(cycle_target(&ws, false), Some(id_of("/b"))); +} + +#[test] +fn stepping_with_fewer_than_two_tabs_asks_for_nothing() { + // One tab is already the destination in either direction, and an empty + // workspace has none — asking the daemon to focus what is in front would + // be a broadcast that changes nothing for every client. + assert_eq!(cycle_target(&workspace_on(&[]), true), None); + assert_eq!(cycle_target(&workspace_on(&["/a"]), true), None); + assert_eq!(cycle_target(&workspace_on(&["/a"]), false), None); +} + +#[test] +fn stepping_onto_a_tab_the_session_has_not_named_yet_asks_for_nothing() { + // A client names a repository by catalog id, so a tab still waiting for + // one cannot be asked for — the same early-out as closing an unnamed tab. + let mut ws = Workspace::new(KeyEvent::new(KeyCode::Char('f'), KeyModifiers::CONTROL)); + assert!(ws.add(project_at("/a"))); + assert!(ws.add(project_at("/b"))); + ws.set_repo_id("/a", &id_of("/a")); + + ws.switch(0); + assert_eq!(cycle_target(&ws, true), None); + ws.switch(1); + assert_eq!(cycle_target(&ws, true), Some(id_of("/a"))); +} + +#[test] +fn resolving_a_step_leaves_the_front_tab_where_it_is() { + // Switching is a request: the tab moves only when the daemon rebroadcasts + // the set, so resolving the target must not move it optimistically. + let mut ws = workspace_on(&["/a", "/b", "/c"]); + ws.switch(1); + + let _ = cycle_target(&ws, true); + let _ = cycle_target(&ws, false); + + assert_eq!(ws.active_index(), 1); +} diff --git a/src/application/tests/workspace.rs b/src/application/tests/workspace.rs index 0a2a29de..118df44f 100644 --- a/src/application/tests/workspace.rs +++ b/src/application/tests/workspace.rs @@ -129,3 +129,23 @@ fn dialog_rejects_command_modifier_chars() { assert!(ws.repo_input.buf.is_empty()); } + +#[test] +fn ctrl_shift_arrows_ask_the_workspace_to_step_between_projects() { + let mut app = app_with_files(vec!["a.rs"]); + let both = KeyModifiers::CONTROL | KeyModifiers::SHIFT; + + let next = handle_key(&mut app, press(KeyCode::Right, both)); + let prev = handle_key(&mut app, press(KeyCode::Left, both)); + + // Direction, not a target index: the wrap needs the tab count and which + // tab is in front, and the handler holds one project. + assert_eq!( + next, + KeyOutcome::Project(ProjectRequest::Cycle { forward: true }) + ); + assert_eq!( + prev, + KeyOutcome::Project(ProjectRequest::Cycle { forward: false }) + ); +} diff --git a/src/input/mod.rs b/src/input/mod.rs index e5697612..007ee474 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -26,6 +26,12 @@ pub enum Action { SwapPanePrompt, /// Focus the project tab at this index. Out-of-range indices are inert. SwitchProject(usize), + /// Step one project tab towards the front of the list, wrapping. The + /// relative counterpart to the F-key jumps, for a session with more tabs + /// than the user wants to count. + PrevProject, + /// Step one project tab away from the front of the list, wrapping. + NextProject, /// Open the repo-path dialog to add a project tab. OpenProject, /// Close the active project tab. Refused when it is the only one. diff --git a/src/input/routing.rs b/src/input/routing.rs index 64e5d2c3..85af4cb3 100644 --- a/src/input/routing.rs +++ b/src/input/routing.rs @@ -12,11 +12,17 @@ pub fn map_key(event: KeyEvent) -> Action { // F-keys / arrows must carry no modifier at all — including // Super/Hyper/Meta, so e.g. Super+F3 passes straight through. let shift_only = event.modifiers == KeyModifiers::SHIFT; + let ctrl_shift = event.modifiers == KeyModifiers::CONTROL | KeyModifiers::SHIFT; let no_mods = event.modifiers.is_empty(); match event.code { KeyCode::Left if shift_only => Action::CycleBackward, KeyCode::Right if shift_only => Action::CycleForward, + // One modifier deeper than pane cycling, on the same keys: the arrows + // step through panes, and adding Ctrl widens the step to project tabs. + // Exact equality keeps these from swallowing the shift-only arms. + KeyCode::Left if ctrl_shift => Action::PrevProject, + KeyCode::Right if ctrl_shift => Action::NextProject, KeyCode::Up if shift_only => Action::TermScrollLineUp, KeyCode::Down if shift_only => Action::TermScrollLineDown, KeyCode::PageUp if shift_only => Action::TermScrollUp, diff --git a/src/input/tests/common.rs b/src/input/tests/common.rs index 25997777..60f222ef 100644 --- a/src/input/tests/common.rs +++ b/src/input/tests/common.rs @@ -7,3 +7,7 @@ pub(super) fn key(code: KeyCode) -> KeyEvent { pub(super) fn ctrl(code: KeyCode) -> KeyEvent { KeyEvent::new(code, KeyModifiers::CONTROL) } + +pub(super) fn ctrl_shift(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::CONTROL | KeyModifiers::SHIFT) +} diff --git a/src/input/tests/key_map_tests.rs b/src/input/tests/key_map_tests.rs index dd8815a2..433c55bf 100644 --- a/src/input/tests/key_map_tests.rs +++ b/src/input/tests/key_map_tests.rs @@ -4,7 +4,7 @@ //! rule — a follow-up ignores modifiers, while these must match them exactly or //! a chord meant for the pane below becomes an app command. -use super::common::{ctrl, key}; +use super::common::{ctrl, ctrl_shift, key}; use super::*; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; @@ -29,8 +29,18 @@ fn reserved_keys_require_exact_modifiers() { // Shift-only arrows are reserved. assert_eq!(with(KeyCode::Left, M::SHIFT), Action::CycleBackward); // Extra modifiers fall through to the PTY. - assert_eq!(with(KeyCode::Left, M::SHIFT | M::CONTROL), Action::None); + assert_eq!(with(KeyCode::Left, M::SHIFT | M::ALT), Action::None); assert_eq!(with(KeyCode::Right, M::SHIFT | M::ALT), Action::None); + // Ctrl+Shift+arrow is reserved too, and on that pair alone: a third + // modifier still belongs to whatever is running in the pane. + assert_eq!( + with(KeyCode::Left, M::SHIFT | M::CONTROL), + Action::PrevProject + ); + assert_eq!( + with(KeyCode::Right, M::SHIFT | M::CONTROL | M::SUPER), + Action::None + ); // F-keys are reserved only without modifiers. assert_eq!(with(KeyCode::F(3), M::NONE), Action::SwitchProject(2)); assert_eq!(with(KeyCode::F(3), M::ALT), Action::None); @@ -100,3 +110,26 @@ fn f_keys_select_project_tabs_regardless_of_layout() { assert_eq!(map_key(key(KeyCode::F(1))), Action::SwitchProject(0)); assert_eq!(map_key(key(KeyCode::F(8))), Action::SwitchProject(7)); } + +#[test] +fn ctrl_shift_arrows_step_between_project_tabs() { + // The F-keys jump to a tab by number; stepping needs a chord of its own, + // and Ctrl+Shift+arrow is safe for the same reason Shift+arrow is — it + // carries modifiers, so it can never be prompt text. + assert_eq!(map_key(ctrl_shift(KeyCode::Left)), Action::PrevProject); + assert_eq!(map_key(ctrl_shift(KeyCode::Right)), Action::NextProject); + // The shift-only arrows keep pane focus cycling: the two chords share the + // arrow keys and must stay distinct. + assert_eq!( + map_key(KeyEvent::new(KeyCode::Left, KeyModifiers::SHIFT)), + Action::CycleBackward + ); + assert_eq!( + map_key(KeyEvent::new(KeyCode::Right, KeyModifiers::SHIFT)), + Action::CycleForward + ); + // Ctrl+Shift on any other reserved key is not a project step. + assert_eq!(map_key(ctrl_shift(KeyCode::Up)), Action::None); + assert_eq!(map_key(ctrl_shift(KeyCode::PageDown)), Action::None); + assert_eq!(map_key(ctrl_shift(KeyCode::F(3))), Action::None); +} From ebf8e3d7c293b9f5e4f3baebfe4fba41cefa8014 Mon Sep 17 00:00:00 2001 From: whackur Date: Tue, 1 Sep 2026 16:44:23 +0900 Subject: [PATCH 02/17] docs: record the project-cycling chord --- docs/keybindings.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/keybindings.md b/docs/keybindings.md index a363c482..8781badc 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -27,6 +27,7 @@ The prefix waits indefinitely for one follow-up. `Esc` or `Ctrl+C` cancels it. A ## Global keys - `F1`–`F10` switch project tabs 1–10. Modified function keys pass through to the terminal. +- `Ctrl+Shift+Left` / `Ctrl+Shift+Right` switch to the previous or next project, wrapping at both ends of the tab order. With one project open they do nothing. - `Shift+Left` / `Shift+Right` cycle focus through the file list, diff viewer, and terminal. - `Shift+Up` / `Shift+Down` scroll the active terminal three lines. - `Shift+PageUp` / `Shift+PageDown` scroll the active terminal one page. Input remains live while scrolled. From 164569a372a79399dd7410de06e2e43007abafb5 Mon Sep 17 00:00:00 2001 From: whackur Date: Tue, 1 Sep 2026 16:45:53 +0900 Subject: [PATCH 03/17] feat(viewer): cycle projects with Ctrl+Shift+arrows The browser had no way to reach a neighbouring project from the keyboard. The TUI answers with bare F-keys, which the browser and the OS have already claimed, so the web takes the same chord the TUI now uses for stepping instead of copying its key table. The switch goes through the existing selectRepo, so pane clear, active repo persistence and the per-project view restore are the same as a click. The listener captures on document: xterm reads keys from its own textarea below it, so stopping there is what keeps a claimed chord from reaching the PTY, while an unclaimed one is left completely untouched. --- viewer-ui/src/hooks/useAppViewModel.ts | 12 ++ viewer-ui/src/hooks/useGlobalKeydown.test.ts | 115 +++++++++++++++ viewer-ui/src/hooks/useGlobalKeydown.ts | 49 +++++++ .../useProjectCycleShortcut.guards.test.ts | 116 +++++++++++++++ .../hooks/useProjectCycleShortcut.harness.ts | 54 +++++++ .../src/hooks/useProjectCycleShortcut.test.ts | 66 +++++++++ .../src/hooks/useProjectCycleShortcut.ts | 138 ++++++++++++++++++ viewer-ui/src/lib/projectCycle.test.ts | 54 +++++++ viewer-ui/src/lib/projectCycle.ts | 25 ++++ 9 files changed, 629 insertions(+) create mode 100644 viewer-ui/src/hooks/useGlobalKeydown.test.ts create mode 100644 viewer-ui/src/hooks/useGlobalKeydown.ts create mode 100644 viewer-ui/src/hooks/useProjectCycleShortcut.guards.test.ts create mode 100644 viewer-ui/src/hooks/useProjectCycleShortcut.harness.ts create mode 100644 viewer-ui/src/hooks/useProjectCycleShortcut.test.ts create mode 100644 viewer-ui/src/hooks/useProjectCycleShortcut.ts create mode 100644 viewer-ui/src/lib/projectCycle.test.ts create mode 100644 viewer-ui/src/lib/projectCycle.ts diff --git a/viewer-ui/src/hooks/useAppViewModel.ts b/viewer-ui/src/hooks/useAppViewModel.ts index 57c80e54..cfc0793f 100644 --- a/viewer-ui/src/hooks/useAppViewModel.ts +++ b/viewer-ui/src/hooks/useAppViewModel.ts @@ -3,6 +3,7 @@ import { isUnauthorized } from "../api"; import { appRows } from "../layout/appLayout"; import { toast } from "../lib/toast"; import { useClone } from "./useClone"; +import { useProjectCycleShortcut } from "./useProjectCycleShortcut"; import { useProjectTabs } from "./useProjectTabs"; import { useRepoActions } from "./useRepoActions"; import { useRepoWorkspace } from "./useRepoWorkspace"; @@ -81,6 +82,17 @@ export function useAppViewModel() { }, [tabs.repo, tabs.setRepo, workspace.clearPane], ); + // Mounted here because this is where the tab order and the one selection + // path meet. Going through `selectRepo` rather than writing the active + // project itself is what keeps a shortcut switch identical to a tab click — + // pane clear, per-project view restore, and the single write-back in + // `useRepoPoll` that the `adoptedRef` invariant depends on. + useProjectCycleShortcut({ + repos: tabs.repos, + repo: tabs.repo, + selectRepo, + enabled: authed === true, + }); const openPicker = useCallback(() => setPickerOpen(true), []); const closePicker = useCallback(() => setPickerOpen(false), []); // Re-bootstrap after login instead of mounting repository state retained diff --git a/viewer-ui/src/hooks/useGlobalKeydown.test.ts b/viewer-ui/src/hooks/useGlobalKeydown.test.ts new file mode 100644 index 00000000..8bbc1467 --- /dev/null +++ b/viewer-ui/src/hooks/useGlobalKeydown.test.ts @@ -0,0 +1,115 @@ +// @vitest-environment happy-dom + +import { cleanup, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useGlobalKeydown } from "./useGlobalKeydown"; + +/** A descendant listening in the bubble phase, standing in for xterm. */ +function child() { + const el = document.createElement("textarea"); + document.body.appendChild(el); + const seen = vi.fn(); + el.addEventListener("keydown", seen); + return { el, seen }; +} + +function press(el: Element, key = "ArrowLeft") { + const event = new KeyboardEvent("keydown", { + key, + bubbles: true, + cancelable: true, + }); + el.dispatchEvent(event); + return event; +} + +// Vitest runs without globals, so RTL cannot auto-register its cleanup. +afterEach(() => { + cleanup(); + document.body.innerHTML = ""; +}); + +describe("useGlobalKeydown", () => { + it("document의_keydown마다_핸들러가_불린다", () => { + const handler = vi.fn(() => false); + renderHook(() => useGlobalKeydown(handler)); + + press(document.body); + + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("true를_반환하면_자식_리스너까지_막고_기본_동작도_막는다", () => { + const { el, seen } = child(); + renderHook(() => useGlobalKeydown(() => true)); + + const event = press(el); + + expect(seen).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); + }); + + it("false를_반환하면_이벤트를_건드리지_않는다", () => { + const { el, seen } = child(); + renderHook(() => useGlobalKeydown(() => false)); + + const event = press(el); + + expect(seen).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(false); + }); + + it("언마운트하면_리스너가_사라진다", () => { + const handler = vi.fn(() => false); + const { unmount } = renderHook(() => useGlobalKeydown(handler)); + + unmount(); + press(document.body); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("enabled가_false면_아무것도_등록하지_않는다", () => { + const handler = vi.fn(() => true); + const { el, seen } = child(); + renderHook(() => useGlobalKeydown(handler, false)); + + const event = press(el); + + expect(handler).not.toHaveBeenCalled(); + expect(seen).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(false); + }); + + it("enabled가_false로_바뀌면_리스너를_뗀다", () => { + const handler = vi.fn(() => false); + const { rerender } = renderHook( + ({ on }: { on: boolean }) => useGlobalKeydown(handler, on), + { initialProps: { on: true } }, + ); + + rerender({ on: false }); + press(document.body); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("핸들러가_바뀌어도_다시_구독하지_않고_최신_것이_불린다", () => { + const first = vi.fn(() => false); + const second = vi.fn(() => false); + const add = vi.spyOn(document, "addEventListener"); + const { rerender } = renderHook( + ({ h }: { h: () => boolean }) => useGlobalKeydown(h), + { initialProps: { h: first } }, + ); + const subscribed = add.mock.calls.length; + + rerender({ h: second }); + press(document.body); + + expect(add.mock.calls.length).toBe(subscribed); + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + add.mockRestore(); + }); +}); diff --git a/viewer-ui/src/hooks/useGlobalKeydown.ts b/viewer-ui/src/hooks/useGlobalKeydown.ts new file mode 100644 index 00000000..885ade23 --- /dev/null +++ b/viewer-ui/src/hooks/useGlobalKeydown.ts @@ -0,0 +1,49 @@ +import { useEffect, useRef } from "react"; + +/** Return true to consume the key; false leaves the event untouched. */ +export type GlobalKeydownHandler = (event: KeyboardEvent) => boolean; + +/** + * One capture-phase keydown listener on `document`, shared by every page-level + * shortcut. + * + * Why capture on `document` and not a React `onKeyDown`: xterm reads keys from + * a `keydown` listener on its own hidden `