diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 1338936..a2cabbf 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -43,7 +43,7 @@ import { SetupScreen } from "./SetupScreen.tsx"; import { StartupErrorScreen } from "./StartupErrorScreen.tsx"; import { HUD_LEFT, hudTopForHeight } from "./Hud.tsx"; import { KeyHints, KEY_HINT_ROWS } from "./KeyHints.tsx"; -import { isPlainShortcut } from "./keys.ts"; +import { handlePlaybackTransportKey, isPlainShortcut } from "./keys.ts"; import { KeymapOverlay } from "./KeymapOverlay.tsx"; import { applyPaletteNavigation } from "./palette-navigation.ts"; import { OVERLAY_PADDING_X, overlayListHeight } from "./Overlay.tsx"; @@ -805,16 +805,12 @@ export function App({ version }: { version: string }) { } const store = usePlayback.getState(); + if (handlePlaybackTransportKey(key, store)) return; + switch (key.name) { case "space": void store.togglePlay(); break; - case "n": - void store.next(); - break; - case "p": - void store.previous(); - break; case "r": if (shouldRetryBootProfile(boot.me, profileRecoveryFailed, boot.client.getCooldown())) { if (profileRecoveryController.current === null) { @@ -833,12 +829,6 @@ export function App({ version }: { version: string }) { case "z": void store.cycleRepeat(); break; - case "right": - void store.seekBy(5_000); - break; - case "left": - void store.seekBy(-5_000); - break; case "up": void store.adjustVolume(5); break; diff --git a/src/ui/keys.ts b/src/ui/keys.ts index b093472..7cb9af4 100644 --- a/src/ui/keys.ts +++ b/src/ui/keys.ts @@ -18,7 +18,17 @@ interface ShortcutKey { hyper?: boolean; } -/** Match a literal letter shortcut without stealing modified terminal/application commands. */ +interface RepeatableShortcutKey extends ShortcutKey { + repeated?: boolean; +} + +export interface PlaybackTransportTarget { + next: () => unknown; + previous: () => unknown; + seekBy: (deltaMs: number) => unknown; +} + +/** Match a literal shortcut without stealing modified terminal/application commands. */ export function isPlainShortcut( key: ShortcutKey, name: string, @@ -35,6 +45,61 @@ export function isPlainShortcut( ); } +/** Ctrl is explicit; every other modifier must remain available to the terminal and focused UI. */ +function isCtrlShortcut(key: ShortcutKey, name: string): boolean { + return ( + key.name === name && + key.ctrl && + !key.shift && + !key.meta && + !key.option && + !key.super && + !key.hyper + ); +} + +type PlaybackTransportCommand = + | { kind: "next" } + | { kind: "previous" } + | { kind: "seek"; deltaMs: -5_000 | 5_000 }; + +/** + * Resolve main-screen transport controls without claiming modified input accidentally. + * + * Track changes are discrete, so a held key must not walk through the queue. Seeking is continuous + * and deliberately accepts terminal repeat events. + */ +function playbackTransportCommand( + key: RepeatableShortcutKey, +): PlaybackTransportCommand | null { + if (key.repeated !== true) { + if (isPlainShortcut(key, "p") || isCtrlShortcut(key, "left")) { + return { kind: "previous" }; + } + if (isPlainShortcut(key, "n") || isCtrlShortcut(key, "right")) { + return { kind: "next" }; + } + } + + if (isPlainShortcut(key, "left")) return { kind: "seek", deltaMs: -5_000 }; + if (isPlainShortcut(key, "right")) return { kind: "seek", deltaMs: 5_000 }; + return null; +} + +/** Dispatch a recognized transport key and report whether the main-screen handler consumed it. */ +export function handlePlaybackTransportKey( + key: RepeatableShortcutKey, + target: PlaybackTransportTarget, +): boolean { + const command = playbackTransportCommand(key); + if (command === null) return false; + + if (command.kind === "next") void target.next(); + else if (command.kind === "previous") void target.previous(); + else void target.seekBy(command.deltaMs); + return true; +} + /** What this keyboard labels the modifier next to the spacebar; the key itself is the same. */ const OPTION_KEY = process.platform === "darwin" ? "opt" : "alt"; @@ -49,8 +114,7 @@ export const KEYMAP: KeyGroup[] = [ label: "PLAYBACK", bindings: [ { key: "space", action: "play / pause" }, - { key: "n", action: "next" }, - { key: "p", action: "previous" }, + { key: "p/n", action: "previous / next" }, { key: "←/→", action: "seek ±5s" }, { key: "↑/↓", action: "volume ±5%" }, { key: "s", action: "shuffle" }, diff --git a/test/keys.test.ts b/test/keys.test.ts index b11bf52..372e58d 100644 --- a/test/keys.test.ts +++ b/test/keys.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { isPlainShortcut } from "../src/ui/keys.ts"; +import { + handlePlaybackTransportKey, + isPlainShortcut, + type PlaybackTransportTarget, +} from "../src/ui/keys.ts"; const plain = { name: "c", @@ -31,3 +35,60 @@ describe("isPlainShortcut", () => { })).toBeFalse(); }); }); + +function transportHarness() { + const calls: Array<"next" | "previous" | number> = []; + const target: PlaybackTransportTarget = { + next: () => calls.push("next"), + previous: () => calls.push("previous"), + seekBy: (deltaMs) => calls.push(deltaMs), + }; + const press = ( + name: string, + modifiers: Partial = {}, + ) => + handlePlaybackTransportKey( + { ...plain, name, repeated: false, ...modifiers }, + target, + ); + return { calls, press }; +} + +describe("playback transport keys", () => { + test("keeps bare arrows on the current track", () => { + const { calls, press } = transportHarness(); + + expect(press("left")).toBeTrue(); + expect(press("right")).toBeTrue(); + expect(calls).toEqual([-5_000, 5_000]); + }); + + test("changes tracks with plain p/n or Ctrl+arrows", () => { + const { calls, press } = transportHarness(); + + expect(press("p")).toBeTrue(); + expect(press("n")).toBeTrue(); + expect(press("left", { ctrl: true })).toBeTrue(); + expect(press("right", { ctrl: true })).toBeTrue(); + expect(calls).toEqual(["previous", "next", "previous", "next"]); + }); + + test("does not steal unsupported modifier combinations", () => { + const { calls, press } = transportHarness(); + + expect(press("n", { ctrl: true })).toBeFalse(); + expect(press("p", { shift: true })).toBeFalse(); + expect(press("right", { shift: true })).toBeFalse(); + expect(press("left", { ctrl: true, meta: true })).toBeFalse(); + expect(calls).toEqual([]); + }); + + test("ignores repeat for track changes but preserves repeated seeking", () => { + const { calls, press } = transportHarness(); + + expect(press("n", { repeated: true })).toBeFalse(); + expect(press("right", { ctrl: true, repeated: true })).toBeFalse(); + expect(press("right", { repeated: true })).toBeTrue(); + expect(calls).toEqual([5_000]); + }); +}); diff --git a/test/playback-keys.test.tsx b/test/playback-keys.test.tsx new file mode 100644 index 0000000..b2996f8 --- /dev/null +++ b/test/playback-keys.test.tsx @@ -0,0 +1,65 @@ +import { createMockKeys, createTestRenderer } from "@opentui/core/testing"; +import { createRoot, useKeyboard } from "@opentui/react"; +import { afterEach, describe, expect, test } from "bun:test"; +import { + handlePlaybackTransportKey, + type PlaybackTransportTarget, +} from "../src/ui/keys.ts"; + +let setup: Awaited> | undefined; + +afterEach(() => { + setup?.renderer.destroy(); + setup = undefined; +}); + +function TransportInput({ target }: { target: PlaybackTransportTarget }) { + useKeyboard((key) => { + handlePlaybackTransportKey(key, target); + }); + return playback; +} + +describe("playback keyboard input", () => { + test("decodes bare and Ctrl-modified arrows as distinct transport commands", async () => { + const calls: Array<"next" | "previous" | number> = []; + const target: PlaybackTransportTarget = { + next: () => calls.push("next"), + previous: () => calls.push("previous"), + seekBy: (deltaMs) => calls.push(deltaMs), + }; + setup = await createTestRenderer({ width: 40, height: 10 }); + const keys = createMockKeys(setup.renderer); + createRoot(setup.renderer).render(); + await Bun.sleep(20); + await setup.renderOnce(); + + keys.pressArrow("left"); + keys.pressArrow("right"); + keys.pressArrow("left", { ctrl: true }); + keys.pressArrow("right", { ctrl: true }); + await Bun.sleep(20); + + expect(calls).toEqual([-5_000, 5_000, "previous", "next"]); + }); + + test("keeps Ctrl+N/P available for mode-specific navigation", async () => { + const calls: string[] = []; + const target: PlaybackTransportTarget = { + next: () => calls.push("next"), + previous: () => calls.push("previous"), + seekBy: () => calls.push("seek"), + }; + setup = await createTestRenderer({ width: 40, height: 10 }); + const keys = createMockKeys(setup.renderer); + createRoot(setup.renderer).render(); + await Bun.sleep(20); + await setup.renderOnce(); + + keys.pressKey("n", { ctrl: true }); + keys.pressKey("p", { ctrl: true }); + await Bun.sleep(20); + + expect(calls).toEqual([]); + }); +});