diff --git a/apps/desktop/src/routes/editor/CanvasElementsOverlay.tsx b/apps/desktop/src/routes/editor/CanvasElementsOverlay.tsx index 3b017e92ce4..d10b29b7db3 100644 --- a/apps/desktop/src/routes/editor/CanvasElementsOverlay.tsx +++ b/apps/desktop/src/routes/editor/CanvasElementsOverlay.tsx @@ -652,6 +652,7 @@ export function CanvasElementsOverlay(props: { size: Size }) { if (!rect) return; e.preventDefault(); + e.stopPropagation(); const px = e.shiftKey ? 10 : 1; const x = clamp( rect.x + (dir[0] * px) / props.size.width, diff --git a/apps/desktop/src/routes/editor/Player.tsx b/apps/desktop/src/routes/editor/Player.tsx index 3ef4105cf0f..753601c2e38 100644 --- a/apps/desktop/src/routes/editor/Player.tsx +++ b/apps/desktop/src/routes/editor/Player.tsx @@ -309,16 +309,59 @@ export function PlayerContent(props: { compactness?: number }) { ); } - // Register keyboard shortcuts in one place + const seekToStart = async () => { + const pending = requestHandoffPlayback(false, 0); + if (pending) { + editorState.timeline.transform.setPosition(0); + await pending; + return; + } + await commands.stopPlayback(); + setEditorState("playing", false); + setEditorState("playbackTime", 0); + setEditorState("previewTime", null); + editorState.timeline.transform.setPosition(0); + if (!handoffPlaybackPending()) { + await commands.seekTo(0); + } + }; + + const seekToEnd = async () => { + const total = totalDuration(); + if (!Number.isFinite(total) || total < 0) return; + const pending = requestHandoffPlayback(false, total); + if (pending) { + await pending; + return; + } + await commands.stopPlayback(); + setEditorState("playing", false); + setEditorState("playbackTime", total); + setEditorState("previewTime", null); + if (!handoffPlaybackPending()) { + await commands.seekTo(Math.floor(total * FPS)); + } + }; + + const hasActiveOverlayNudge = () => { + if (editorState.canvasSelection) return true; + const selType = editorState.timeline.selection?.type; + return selType === "text" || selType === "image"; + }; + useEditorShortcuts(() => { - const el = document.activeElement; + const el = document.activeElement as HTMLElement | null; if (!el) return true; const tagName = el.tagName.toLowerCase(); - const isContentEditable = el.getAttribute("contenteditable") === "true"; + const role = el.getAttribute("role"); return !( tagName === "input" || tagName === "textarea" || - isContentEditable + tagName === "select" || + el.isContentEditable || + role === "slider" || + role === "listbox" || + role === "menu" ); }, [ { @@ -361,6 +404,28 @@ export function PlayerContent(props: { compactness?: number }) { await handlePlayPauseClick(); }, }, + { + combo: "ArrowUp", + handler: () => { + if (hasActiveOverlayNudge()) return; + void seekToStart(); + }, + }, + { + combo: "Home", + handler: () => void seekToStart(), + }, + { + combo: "ArrowDown", + handler: () => { + if (hasActiveOverlayNudge()) return; + void seekToEnd(); + }, + }, + { + combo: "End", + handler: () => void seekToEnd(), + }, ]); return ( @@ -448,18 +513,7 @@ export function PlayerContent(props: { compactness?: number }) { @@ -479,16 +533,7 @@ export function PlayerContent(props: { compactness?: number }) { diff --git a/apps/desktop/src/routes/editor/TextOverlay.tsx b/apps/desktop/src/routes/editor/TextOverlay.tsx index 359b285eb6f..54948f5bf27 100644 --- a/apps/desktop/src/routes/editor/TextOverlay.tsx +++ b/apps/desktop/src/routes/editor/TextOverlay.tsx @@ -213,6 +213,7 @@ export function TextOverlay(props: TextOverlayProps) { if (!dir) return; e.preventDefault(); + e.stopPropagation(); const px = e.shiftKey ? 10 : 1; updateSegmentByIndex(index, (s) => { s.center.x = clamp( diff --git a/apps/desktop/src/routes/editor/use-editor-shortcuts.test.ts b/apps/desktop/src/routes/editor/use-editor-shortcuts.test.ts new file mode 100644 index 00000000000..c3d02238101 --- /dev/null +++ b/apps/desktop/src/routes/editor/use-editor-shortcuts.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { normalizeCombo } from "./useEditorShortcuts"; + +function createKeyboardEvent( + code: string, + options: { + metaKey?: boolean; + ctrlKey?: boolean; + altKey?: boolean; + shiftKey?: boolean; + } = {}, +): KeyboardEvent { + return { + code, + metaKey: options.metaKey ?? false, + ctrlKey: options.ctrlKey ?? false, + altKey: options.altKey ?? false, + shiftKey: options.shiftKey ?? false, + } as KeyboardEvent; +} + +describe("useEditorShortcuts: normalizeCombo", () => { + it("normalizes navigation and boundary keys without modifiers", () => { + expect(normalizeCombo(createKeyboardEvent("ArrowUp"))).toBe("ArrowUp"); + expect(normalizeCombo(createKeyboardEvent("ArrowDown"))).toBe("ArrowDown"); + expect(normalizeCombo(createKeyboardEvent("Home"))).toBe("Home"); + expect(normalizeCombo(createKeyboardEvent("End"))).toBe("End"); + expect(normalizeCombo(createKeyboardEvent("Space"))).toBe("Space"); + }); + + it("strips Key prefix for standard letters", () => { + expect(normalizeCombo(createKeyboardEvent("KeyS"))).toBe("S"); + expect(normalizeCombo(createKeyboardEvent("KeyC"))).toBe("C"); + }); + + it("normalizes Mod modifier and special symbols", () => { + expect(normalizeCombo(createKeyboardEvent("Equal", { metaKey: true }))).toBe("Mod+="); + expect(normalizeCombo(createKeyboardEvent("Minus", { ctrlKey: true }))).toBe("Mod+-"); + expect(normalizeCombo(createKeyboardEvent("KeyS", { metaKey: true }))).toBe("Mod+S"); + }); + + it("preserves Shift and Alt modifiers to avoid collision with bare navigation keys", () => { + expect(normalizeCombo(createKeyboardEvent("ArrowUp", { shiftKey: true }))).toBe("Shift+ArrowUp"); + expect(normalizeCombo(createKeyboardEvent("ArrowDown", { altKey: true }))).toBe("Alt+ArrowDown"); + expect(normalizeCombo(createKeyboardEvent("Home", { ctrlKey: true, shiftKey: true }))).toBe("Mod+Shift+Home"); + expect(normalizeCombo(createKeyboardEvent("KeyS", { altKey: true }))).toBe("Alt+S"); + }); +}); diff --git a/apps/desktop/src/routes/editor/useEditorShortcuts.ts b/apps/desktop/src/routes/editor/useEditorShortcuts.ts index 073ff0fbb17..2c3e310b191 100644 --- a/apps/desktop/src/routes/editor/useEditorShortcuts.ts +++ b/apps/desktop/src/routes/editor/useEditorShortcuts.ts @@ -1,17 +1,19 @@ import { createEventListener } from "@solid-primitives/event-listener"; export type ShortcutBinding = { - combo: string; // e.g. "Mod+=", "Mod+-", "Space", "S", "C" + combo: string; handler: (e: KeyboardEvent) => void | Promise; - preventDefault?: boolean; // default: true - when?: () => boolean; // optional enablement gate + preventDefault?: boolean; + when?: () => boolean; }; -const isMod = (e: KeyboardEvent) => e.metaKey || e.ctrlKey; // treat Cmd/Ctrl as Mod +const isMod = (e: KeyboardEvent) => e.metaKey || e.ctrlKey; -function normalizeCombo(e: KeyboardEvent): string { +export function normalizeCombo(e: KeyboardEvent): string { const parts: string[] = []; if (isMod(e)) parts.push("Mod"); + if (e.altKey) parts.push("Alt"); + if (e.shiftKey) parts.push("Shift"); let key: string; switch (e.code) { @@ -38,7 +40,6 @@ export function useEditorShortcuts( ); createEventListener(document, "keydown", async (e: KeyboardEvent) => { - // Basic guards if (!getScopeActive()) return; if (e.repeat) return; diff --git a/apps/web/__tests__/unit/timeline-keyboard.test.ts b/apps/web/__tests__/unit/timeline-keyboard.test.ts new file mode 100644 index 00000000000..8ea3d9939f0 --- /dev/null +++ b/apps/web/__tests__/unit/timeline-keyboard.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "vitest"; +import { + isIgnoredTimelineKeyboardTarget, + resolveTimelineKeyAction, +} from "@/app/s/[videoId]/_components/timeline/TimelineView"; + +describe("resolveTimelineKeyAction", () => { + it("seeks to start (0) on bare ArrowUp and Home when not focused on slider", () => { + expect(resolveTimelineKeyAction("ArrowUp", false, 120, false, false)).toEqual({ + type: "seekTo", + time: 0, + }); + expect(resolveTimelineKeyAction("Home", false, 120, false, false)).toEqual({ + type: "seekTo", + time: 0, + }); + }); + + it("seeks to end on bare ArrowDown and End when not focused on slider", () => { + expect(resolveTimelineKeyAction("ArrowDown", false, 120, false, false)).toEqual({ + type: "seekTo", + time: 120, + }); + expect(resolveTimelineKeyAction("End", false, 120, false, false)).toEqual({ + type: "seekTo", + time: 120, + }); + }); + + it("steps incrementally on ArrowUp/Down/Left/Right when focused on ARIA slider per accessibility guidelines", () => { + // ArrowUp increments slider + expect(resolveTimelineKeyAction("ArrowUp", false, 120, false, true)).toEqual({ + type: "seekDelta", + delta: 5, + }); + // ArrowDown decrements slider + expect(resolveTimelineKeyAction("ArrowDown", false, 120, false, true)).toEqual({ + type: "seekDelta", + delta: -5, + }); + // ArrowRight increments slider + expect(resolveTimelineKeyAction("ArrowRight", false, 120, false, true)).toEqual({ + type: "seekDelta", + delta: 5, + }); + // ArrowLeft decrements slider + expect(resolveTimelineKeyAction("ArrowLeft", false, 120, false, true)).toEqual({ + type: "seekDelta", + delta: -5, + }); + // Home/End on slider still jump to extremes + expect(resolveTimelineKeyAction("Home", false, 120, false, true)).toEqual({ + type: "seekTo", + time: 0, + }); + expect(resolveTimelineKeyAction("End", false, 120, false, true)).toEqual({ + type: "seekTo", + time: 120, + }); + }); + + it("handles non-finite or negative durations safely", () => { + expect(resolveTimelineKeyAction("ArrowDown", false, Number.NaN, false, false)).toEqual({ + type: "seekTo", + time: 0, + }); + expect(resolveTimelineKeyAction("End", false, -10, false, false)).toEqual({ + type: "seekTo", + time: 0, + }); + }); + + it("seeks delta on ArrowLeft and ArrowRight", () => { + expect(resolveTimelineKeyAction("ArrowLeft", false, 120, false)).toEqual({ + type: "seekDelta", + delta: -5, + }); + expect(resolveTimelineKeyAction("ArrowRight", false, 120, false)).toEqual({ + type: "seekDelta", + delta: 5, + }); + }); + + it("toggles play on Space unless focused over a branch node button", () => { + expect(resolveTimelineKeyAction(" ", false, 120, false)).toEqual({ + type: "togglePlay", + }); + expect(resolveTimelineKeyAction("Spacebar", false, 120, false)).toEqual({ + type: "togglePlay", + }); + expect(resolveTimelineKeyAction(" ", false, 120, true)).toBeNull(); + }); + + it("strictly ignores actions when any modifier is pressed", () => { + expect(resolveTimelineKeyAction("ArrowUp", true, 120, false)).toBeNull(); + expect(resolveTimelineKeyAction("ArrowDown", true, 120, false)).toBeNull(); + expect(resolveTimelineKeyAction("Home", true, 120, false)).toBeNull(); + expect(resolveTimelineKeyAction("End", true, 120, false)).toBeNull(); + expect(resolveTimelineKeyAction("ArrowLeft", true, 120, false)).toBeNull(); + expect(resolveTimelineKeyAction("ArrowRight", true, 120, false)).toBeNull(); + expect(resolveTimelineKeyAction(" ", true, 120, false)).toBeNull(); + }); +}); + +describe("isIgnoredTimelineKeyboardTarget", () => { + it("returns false for null target or generic divs", () => { + expect(isIgnoredTimelineKeyboardTarget(null)).toBe(false); + const div = document.createElement("div"); + expect(isIgnoredTimelineKeyboardTarget(div)).toBe(false); + }); + + it("returns true for input, textarea, select, contenteditable, and listbox/menu roles", () => { + const input = document.createElement("input"); + expect(isIgnoredTimelineKeyboardTarget(input)).toBe(true); + + const textarea = document.createElement("textarea"); + expect(isIgnoredTimelineKeyboardTarget(textarea)).toBe(true); + + const select = document.createElement("select"); + expect(isIgnoredTimelineKeyboardTarget(select)).toBe(true); + + const editable = document.createElement("div"); + editable.contentEditable = "true"; + expect(isIgnoredTimelineKeyboardTarget(editable)).toBe(true); + + const listbox = document.createElement("div"); + listbox.setAttribute("role", "listbox"); + expect(isIgnoredTimelineKeyboardTarget(listbox)).toBe(true); + + const menu = document.createElement("div"); + menu.setAttribute("role", "menu"); + expect(isIgnoredTimelineKeyboardTarget(menu)).toBe(true); + }); +}); diff --git a/apps/web/app/s/[videoId]/_components/timeline/TimelineView.tsx b/apps/web/app/s/[videoId]/_components/timeline/TimelineView.tsx index bbd40bcd27f..27ba95dab40 100644 --- a/apps/web/app/s/[videoId]/_components/timeline/TimelineView.tsx +++ b/apps/web/app/s/[videoId]/_components/timeline/TimelineView.tsx @@ -59,6 +59,82 @@ const EMPTY_CHAPTERS: TimelineChapter[] = []; const WHEEL_ZOOM_RATE = 0.012; const WHEEL_ZOOM_MAX = 1.6; +export type TimelineKeyAction = + | { type: "seekDelta"; delta: number } + | { type: "seekTo"; time: number } + | { type: "togglePlay" } + | null; + +export function isIgnoredTimelineKeyboardTarget( + target: HTMLElement | null, +): boolean { + if (!target) return false; + const tagName = target.tagName?.toLowerCase(); + const role = target.getAttribute?.("role"); + return ( + tagName === "input" || + tagName === "textarea" || + tagName === "select" || + target.isContentEditable || + role === "listbox" || + role === "menu" + ); +} + +export function resolveTimelineKeyAction( + key: string, + hasModifier: boolean, + duration: number, + isNodeButton: boolean, + isSlider = false, +): TimelineKeyAction { + if (hasModifier) return null; + + if (isSlider) { + if (key === "ArrowRight" || key === "ArrowUp") { + return { type: "seekDelta", delta: KEYBOARD_SEEK_STEP }; + } + if (key === "ArrowLeft" || key === "ArrowDown") { + return { type: "seekDelta", delta: -KEYBOARD_SEEK_STEP }; + } + if (key === "Home") { + return { type: "seekTo", time: 0 }; + } + if (key === "End") { + const safeDuration = + Number.isFinite(duration) && duration >= 0 ? duration : 0; + return { type: "seekTo", time: safeDuration }; + } + if (key === " " || key === "Spacebar") { + return { type: "togglePlay" }; + } + return null; + } + + if (key === "ArrowLeft" || key === "ArrowRight") { + return { + type: "seekDelta", + delta: key === "ArrowLeft" ? -KEYBOARD_SEEK_STEP : KEYBOARD_SEEK_STEP, + }; + } + + if (key === "ArrowUp" || key === "Home") { + return { type: "seekTo", time: 0 }; + } + + if (key === "ArrowDown" || key === "End") { + const safeDuration = + Number.isFinite(duration) && duration >= 0 ? duration : 0; + return { type: "seekTo", time: safeDuration }; + } + + if ((key === " " || key === "Spacebar") && !isNodeButton) { + return { type: "togglePlay" }; + } + + return null; +} + export interface TimelineViewProps { comments: CommentType[]; videoId: Video.VideoId; @@ -409,27 +485,38 @@ function TimelineBand({ const handleKeyDown = useCallback( (event: React.KeyboardEvent) => { const target = event.target as HTMLElement | null; - if ( - target instanceof HTMLInputElement || - target instanceof HTMLTextAreaElement - ) { + if (isIgnoredTimelineKeyboardTarget(target)) { return; } - if (event.key === "ArrowLeft" || event.key === "ArrowRight") { - event.preventDefault(); - const delta = - event.key === "ArrowLeft" ? -KEYBOARD_SEEK_STEP : KEYBOARD_SEEK_STEP; - playback.seek(playback.getCurrentTime() + delta); - return; - } + const role = target?.getAttribute?.("role"); + const isSlider = + role === "slider" || Boolean(target?.closest?.("[data-timeline-rail]")); + const hasModifier = + event.shiftKey || event.altKey || event.ctrlKey || event.metaKey; + const isNodeButton = Boolean(target?.closest("[data-timeline-node]")); + const action = resolveTimelineKeyAction( + event.key, + hasModifier, + playback.getDuration(), + isNodeButton, + isSlider, + ); + + if (!action) return; - // Space over a branch node belongs to that button, not to playback. - if (event.key === " " || event.key === "Spacebar") { - if (target?.closest("[data-timeline-node]")) return; - event.preventDefault(); - if (playback.getPlaying()) playback.pause(); - else playback.play(); + event.preventDefault(); + switch (action.type) { + case "seekDelta": + playback.seek(playback.getCurrentTime() + action.delta); + break; + case "seekTo": + playback.seek(action.time); + break; + case "togglePlay": + if (playback.getPlaying()) playback.pause(); + else playback.play(); + break; } }, [playback],