diff --git a/packages/freecut-editor/README.md b/packages/freecut-editor/README.md index 4480ec512..4e485e835 100644 --- a/packages/freecut-editor/README.md +++ b/packages/freecut-editor/README.md @@ -69,6 +69,40 @@ shortcut editor, including J/K/L transport. UI changes call `setSettings`, and host or agent changes can flow back through `subscribe`, so embedded shortcut configuration never becomes a UI-only setting. +As of 0.3.13, Delete/Backspace and cut shortcuts work when the timeline clip +itself has keyboard focus. Editable fields, nested controls, ordinary buttons, +and clips inside dialogs retain shortcut protection. C cuts the hovered clip at +the pointer frame; with the pointer away, it cuts exactly one selected clip at +the playhead. No selection, multiple selections, and a playhead at either clip +endpoint produce no fallback cut. + +Host-backed undo/redo in 0.3.13 is opt-in through `EditorHost.history`: + +```ts +history: { + undo: () => restoreSavedHistory('undo'), + redo: () => restoreSavedHistory('redo'), +} +``` + +Cmd/Ctrl+Z and Cmd/Ctrl+Shift+Z invoke those callbacks. The host owns history, +request serialization, conflict handling and persistence, and publishes the +resulting authoritative snapshot through its existing `subscribe` port. The +surface never rolls back its local temporal store in host mode. Without the +history port, host-mode undo/redo stays disabled; standalone history continues +to use the local store. Input fields, nested controls and dialogs keep their +native keyboard behavior. + +CodePress requires its durable-history implementation (quantfive/codepress#6428) +and the companion history-port wiring in addition to the 0.3.13 package update; +a package upgrade alone cannot enable saved undo in a host without history. + +This release incorporates the source equivalents of the focused-clip and +selected-playhead shortcut hunks in CodePress's 0.3.12 vendor patch +(quantfive/codepress#7001). Once CodePress pins this published version, remove +those two shortcut hunks while preserving unrelated vendor fixes, regenerate +the patch hash, and verify the installed package through the real host. + As of 0.3.12, host-mode timeline clips use durable forward attachment chains by default. A detached clip is an explicit ripple break and can be reattached from its context menu. The host-mode Delete action and Delete/Backspace shortcuts submit @@ -137,5 +171,5 @@ Consumers install the exact published version and keep it pinned in their lockfile: ```bash -npm install @quantfive/freecut-editor-surface@0.3.12 +npm install @quantfive/freecut-editor-surface@0.3.13 ``` diff --git a/packages/freecut-editor/consumer-smoke.test.tsx b/packages/freecut-editor/consumer-smoke.test.tsx index 819b180b7..21a16c6bd 100644 --- a/packages/freecut-editor/consumer-smoke.test.tsx +++ b/packages/freecut-editor/consumer-smoke.test.tsx @@ -15,6 +15,7 @@ import { type EditorTranscriptPort, type EmbeddedEditorSnapshot, type HostEditPredicate, + type EditorHistoryPort, type HostNotice, type HostTimelineEditPort, } from '@quantfive/freecut-editor-surface' @@ -146,9 +147,11 @@ describe('published FreeCut browser entry', () => { } const requestTranscription = vi.fn>() const timelinePort: HostTimelineEditPort = { requestRippleDelete: vi.fn() } + const historyPort: EditorHistoryPort = { undo: vi.fn(), redo: vi.fn() } expect(notice.detail?.failedPredicates).toEqual(['sourceRange']) expect(requestTranscription).toBeTypeOf('function') expect(timelinePort.requestRippleDelete).toBeTypeOf('function') + expect(historyPort.undo).toBeTypeOf('function') const typedMetadataSnapshot: EmbeddedEditorSnapshot = { ...snapshot, diff --git a/packages/freecut-editor/package.json b/packages/freecut-editor/package.json index 19e9df070..41c3dbf9e 100644 --- a/packages/freecut-editor/package.json +++ b/packages/freecut-editor/package.json @@ -1,6 +1,6 @@ { "name": "@quantfive/freecut-editor-surface", - "version": "0.3.12", + "version": "0.3.13", "description": "The host-backed FreeCut browser editor surface.", "license": "MIT", "repository": { diff --git a/packages/freecut-editor/src/index.d.ts b/packages/freecut-editor/src/index.d.ts index ca1afedcd..9b017f483 100644 --- a/packages/freecut-editor/src/index.d.ts +++ b/packages/freecut-editor/src/index.d.ts @@ -456,6 +456,11 @@ export interface EditorShortcutPort { subscribe?(listener: (settings: HostShortcutSettings) => void): () => void } +export interface EditorHistoryPort { + undo(): Promise | void + redo(): Promise | void +} + export declare function createHostShortcutSettings( overrides?: HotkeyOverrideMap, ): HostShortcutSettings @@ -469,6 +474,7 @@ export interface EditorHost { submitEdit(batch: EditCommandBatch): Promise | HostEditResult subscribe?(listener: (snapshot: EmbeddedEditorSnapshot) => void): () => void shortcuts?: EditorShortcutPort + history?: EditorHistoryPort transcript?: EditorTranscriptPort navigation?: EditorHostNavigation notify?(notice: HostNotice): void diff --git a/packages/freecut-editor/src/index.ts b/packages/freecut-editor/src/index.ts index 2b9d8556e..30133f3ab 100644 --- a/packages/freecut-editor/src/index.ts +++ b/packages/freecut-editor/src/index.ts @@ -25,6 +25,7 @@ export type { EditorCapability, EditorCapabilityMap, EditorHost, + EditorHistoryPort, EditorHostNavigation, EditorShortcutPort, EmbeddedEditorAsset, diff --git a/src/config/hotkeys-dom-guard.test.ts b/src/config/hotkeys-dom-guard.test.ts index 2538c2f03..cfdd7a238 100644 --- a/src/config/hotkeys-dom-guard.test.ts +++ b/src/config/hotkeys-dom-guard.test.ts @@ -127,6 +127,46 @@ describe('global shortcut DOM guards', () => { }) }) + it('allows shortcuts when the exact timeline item root owns focus', () => { + expect( + dispatchFrom( + '
Clip
', + '#clip', + 'k', + ), + ).toEqual({ captureSawEvent: true, defaultPrevented: true }) + }) + + it('keeps nested controls inside a timeline item protected', () => { + expect( + dispatchFrom( + '
', + '#control', + 'k', + ), + ).toEqual({ captureSawEvent: true, defaultPrevented: false }) + }) + + it('keeps a focused timeline item inside a dialog protected', () => { + expect( + dispatchFrom( + '
Clip
', + '#clip', + 'k', + ), + ).toEqual({ captureSawEvent: true, defaultPrevented: false }) + }) + + it('keeps a contenteditable timeline item protected', () => { + expect( + dispatchFrom( + '
Clip
', + '#clip', + 'k', + ), + ).toEqual({ captureSawEvent: true, defaultPrevented: false }) + }) + it('preserves explicit canvas opt-in inside a native dialog', () => { expect( dispatchFrom( diff --git a/src/config/hotkeys.ts b/src/config/hotkeys.ts index 282eeab58..d9c520624 100644 --- a/src/config/hotkeys.ts +++ b/src/config/hotkeys.ts @@ -1261,16 +1261,12 @@ export function shouldIgnoreGlobalHotkey(event: KeyboardEvent): boolean { if (typeof Element === 'undefined' || !(target instanceof Element)) return false if (target.closest(GLOBAL_HOTKEY_OPT_IN)) return false if (isContentEditableTarget(target)) return true - // A Razor click leaves focus on the semantic clip root. Preserve the - // interactive surface's normal transport/activation behavior, but let - // history shortcuts reach the timeline controller so one click-cut can be - // undone without first moving focus away from the clip. - const isHistoryShortcut = - (event.metaKey || event.ctrlKey) && - !event.altKey && - event.key.toLowerCase() === 'z' && - target.closest('[data-timeline-item]') !== null - if (isHistoryShortcut) return false + if ( + target.matches('[data-timeline-item][data-item-id][role="button"]') && + !target.closest(DIALOG_SELECTOR) + ) { + return false + } if (target.closest(INTERACTIVE_CONTROL_SELECTOR)) return true return target.closest(DIALOG_SELECTOR) !== null } diff --git a/src/features/editor/components/editor.tsx b/src/features/editor/components/editor.tsx index bf4924977..c22c7a4cd 100644 --- a/src/features/editor/components/editor.tsx +++ b/src/features/editor/components/editor.tsx @@ -440,8 +440,8 @@ const TimelineShortcutsController = memo(function TimelineShortcutsController() }) // Host mode mounts only the host-safe shortcut slice (playback, tools, -// delete, zoom/snap) — undo/redo, ripple delete, clipboard, markers, and -// nudges would mutate local stores without crossing the host bridge. +// delete, zoom/snap, and optional host-owned undo/redo) — ripple delete, +// clipboard, markers, and nudges remain outside the host bridge. const HostTimelineShortcutsController = memo(function HostTimelineShortcutsController() { useHostTimelineShortcuts() return null diff --git a/src/features/editor/host/contract.ts b/src/features/editor/host/contract.ts index 747e4ad85..d291e3d65 100644 --- a/src/features/editor/host/contract.ts +++ b/src/features/editor/host/contract.ts @@ -347,6 +347,16 @@ export interface EditorShortcutPort { subscribe?(listener: (settings: HostShortcutSettings) => void): () => void } +/** + * Optional host-owned history boundary. The host persists and serializes the + * authoritative history; after applying an action it pushes the resulting + * snapshot through EditorHost.subscribe when the surface needs an update. + */ +export interface EditorHistoryPort { + undo(): Promise | void + redo(): Promise | void +} + export function createHostShortcutSettings( overrides: HotkeyOverrideMap = {}, ): HostShortcutSettings { @@ -374,6 +384,8 @@ export interface EditorHost { subscribe?(listener: (snapshot: EmbeddedEditorSnapshot) => void): () => void /** Optional host/agent round-trip for user-configurable keyboard shortcuts. */ shortcuts?: EditorShortcutPort + /** Optional host-owned undo/redo boundary for host-mode timeline history. */ + history?: EditorHistoryPort /** Optional application-issued transcript read/preview boundary. */ transcript?: EditorTranscriptPort navigation?: EditorHostNavigation diff --git a/src/features/editor/host/index.ts b/src/features/editor/host/index.ts index 35996ef5f..ad2645826 100644 --- a/src/features/editor/host/index.ts +++ b/src/features/editor/host/index.ts @@ -26,6 +26,7 @@ export type { EditorCapability, EditorCapabilityMap, EditorHost, + EditorHistoryPort, EditorHostNavigation, EditorShortcutPort, EmbeddedEditorAsset, diff --git a/src/features/timeline/components/timeline-item/timeline-item-accessibility.test.tsx b/src/features/timeline/components/timeline-item/timeline-item-accessibility.test.tsx index 93769be00..545c88e98 100644 --- a/src/features/timeline/components/timeline-item/timeline-item-accessibility.test.tsx +++ b/src/features/timeline/components/timeline-item/timeline-item-accessibility.test.tsx @@ -165,7 +165,7 @@ describe('TimelineItem keyboard accessibility', () => { it.each([ ['Enter', 'Enter'], [' ', 'Space'], - ])('activates exactly once with %s after global capture declines it', (key, code) => { + ])('activates exactly once with %s while the clip root owns focus', (key, code) => { const selectItems = vi.spyOn(useSelectionStore.getState(), 'selectItems') const togglePlayPause = vi.spyOn(usePlaybackStore.getState(), 'togglePlayPause') const captureListener = vi.fn() @@ -190,7 +190,11 @@ describe('TimelineItem keyboard accessibility', () => { expect(event.defaultPrevented).toBe(true) expect(selectItems).toHaveBeenCalledTimes(1) expect(selectItems).toHaveBeenLastCalledWith([ITEM.id]) - expect(togglePlayPause).not.toHaveBeenCalled() + if (key === ' ') { + expect(togglePlayPause).toHaveBeenCalledTimes(1) + } else { + expect(togglePlayPause).not.toHaveBeenCalled() + } }) it('keeps native clip controls outside button semantics and lets them own keyboard events', () => { @@ -210,7 +214,7 @@ describe('TimelineItem keyboard accessibility', () => { expect(selectItems).not.toHaveBeenCalled() }) - it('semantically declines J, K, and L transport while the clip root is focused', () => { + it('routes J, K, and L transport while the clip root is focused', () => { const { container } = render( { dispatchKey(clip, 'l', 'KeyL') expect(usePlaybackStore.getState()).toMatchObject({ - isPlaying: false, + isPlaying: true, playbackRate: 1, - transportMode: 'normal', + transportMode: 'shuttle', }) }) diff --git a/src/features/timeline/deps/editor-contract.ts b/src/features/timeline/deps/editor-contract.ts index 6cbd446ad..a67e6c7a6 100644 --- a/src/features/timeline/deps/editor-contract.ts +++ b/src/features/timeline/deps/editor-contract.ts @@ -2,3 +2,4 @@ export { useEditorCapability, useEditorHostContext } from '@/features/editor/host/context' export { EditorHostProvider } from '@/features/editor/host/context-provider' +export type { EditorHost, EditorHistoryPort } from '@/features/editor/host/contract' diff --git a/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.test.tsx b/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.test.tsx index eb74b4ec1..f9a4454ae 100644 --- a/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.test.tsx +++ b/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.test.tsx @@ -46,6 +46,16 @@ const ITEM: VideoItem = { src: 'clip.mp4', } +const SECOND_ITEM: VideoItem = { + id: 'clip-2', + type: 'video', + trackId: TRACK.id, + from: 40, + durationInFrames: 40, + label: 'Clip 2', + src: 'clip-2.mp4', +} + function ShortcutHarness() { useToolShortcuts({}) return null @@ -61,6 +71,7 @@ describe('hover split and Razor shortcut ownership', () => { beforeEach(() => { registrations.calls = [] clearTimelineHover() + useSelectionStore.getState().clearSelection() useSelectionStore.setState({ activeTool: 'select' }) useMicRecordingStore.setState({ status: 'idle' }) useTimelineCommandStore.getState().clearHistory() @@ -111,6 +122,100 @@ describe('hover split and Razor shortcut ownership', () => { expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) }) + it('prefers the synchronously hovered clip over the selected clip and current frame', () => { + useTimelineStore.setState({ tracks: [TRACK], items: [ITEM, SECOND_ITEM], transitions: [] }) + useSelectionStore.getState().selectItems([SECOND_ITEM.id]) + usePlaybackStore.setState({ currentFrame: 55, previewFrame: 12, previewItemId: SECOND_ITEM.id }) + setTimelineHover(ITEM.id, 25) + render() + + act(() => { + getRegistration('SPLIT_AT_PLAYHEAD').callback({ preventDefault: vi.fn() }) + }) + + const items = useTimelineStore + .getState() + .items.toSorted((left, right) => left.from - right.from) + expect(items.map((item) => [item.from, item.durationInFrames])).toEqual([ + [0, 25], + [25, 15], + [40, 40], + ]) + }) + + it('falls back to the one selected clip at currentFrame when the pointer is away', () => { + useTimelineStore.setState({ tracks: [TRACK], items: [ITEM, SECOND_ITEM], transitions: [] }) + useSelectionStore.getState().selectItems([SECOND_ITEM.id]) + usePlaybackStore.setState({ currentFrame: 55, previewFrame: 12, previewItemId: ITEM.id }) + render() + + act(() => { + getRegistration('SPLIT_AT_PLAYHEAD').callback({ preventDefault: vi.fn() }) + }) + + const items = useTimelineStore + .getState() + .items.toSorted((left, right) => left.from - right.from) + expect(items.map((item) => [item.from, item.durationInFrames])).toEqual([ + [0, 40], + [40, 15], + [55, 25], + ]) + }) + + it.each([SECOND_ITEM.from, SECOND_ITEM.from + SECOND_ITEM.durationInFrames])( + 'rejects a selected fallback at clip endpoint %s', + (currentFrame) => { + useTimelineStore.setState({ tracks: [TRACK], items: [ITEM, SECOND_ITEM], transitions: [] }) + useSelectionStore.getState().selectItems([SECOND_ITEM.id]) + usePlaybackStore.setState({ currentFrame, previewFrame: 12, previewItemId: ITEM.id }) + render() + + act(() => { + getRegistration('SPLIT_AT_PLAYHEAD').callback({ preventDefault: vi.fn() }) + }) + + expect(useTimelineStore.getState().items).toEqual([ITEM, SECOND_ITEM]) + }, + ) + + it('rejects the fallback when there is no selected clip', () => { + useTimelineStore.setState({ tracks: [TRACK], items: [ITEM, SECOND_ITEM], transitions: [] }) + usePlaybackStore.setState({ currentFrame: 55, previewFrame: 12, previewItemId: SECOND_ITEM.id }) + render() + + act(() => { + getRegistration('SPLIT_AT_PLAYHEAD').callback({ preventDefault: vi.fn() }) + }) + + expect(useTimelineStore.getState().items).toEqual([ITEM, SECOND_ITEM]) + }) + + it('rejects the fallback when multiple clips are selected', () => { + useTimelineStore.setState({ tracks: [TRACK], items: [ITEM, SECOND_ITEM], transitions: [] }) + useSelectionStore.getState().selectItems([ITEM.id, SECOND_ITEM.id]) + usePlaybackStore.setState({ currentFrame: 25, previewFrame: 12, previewItemId: ITEM.id }) + render() + + act(() => { + getRegistration('SPLIT_AT_PLAYHEAD').callback({ preventDefault: vi.fn() }) + }) + + expect(useTimelineStore.getState().items).toEqual([ITEM, SECOND_ITEM]) + }) + + it('rejects a selected fallback whose item is missing', () => { + useSelectionStore.getState().selectItems(['missing-item']) + usePlaybackStore.setState({ currentFrame: 20, previewFrame: 12, previewItemId: ITEM.id }) + render() + + act(() => { + getRegistration('SPLIT_AT_PLAYHEAD').callback({ preventDefault: vi.fn() }) + }) + + expect(useTimelineStore.getState().items).toEqual([ITEM]) + }) + it('keeps Shift+C owned by the persistent Razor tool', () => { render() diff --git a/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.ts index f80e78cf9..ce56dff0f 100644 --- a/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.ts +++ b/src/features/timeline/hooks/shortcuts/use-tool-shortcuts.ts @@ -3,6 +3,7 @@ */ import { useCommandHotkey } from '@/hooks/use-hotkey-registration' +import { usePlaybackStore } from '@/shared/state/playback' import { useTimelineStore } from '../../stores/timeline-store' import { useTimelineCommandStore } from '../../stores/timeline-command-store' import { useSelectionStore } from '@/shared/state/selection' @@ -19,7 +20,18 @@ function splitHoveredTimelineItemAtPointer(): boolean { return false } - const { itemId, frame } = getTimelineHover() + let { itemId, frame } = getTimelineHover() + if (!itemId || frame === null) { + const { selectedItemIds } = useSelectionStore.getState() + if (selectedItemIds.length !== 1) { + notifySplitRejection('no-hover') + return false + } + + itemId = selectedItemIds[0] ?? null + frame = usePlaybackStore.getState().currentFrame + } + const { items } = useTimelineStore.getState() if (!itemId || frame === null) { @@ -84,9 +96,9 @@ export function useToolShortcuts(callbacks: TimelineShortcutCallbacks) { [activeTool, setActiveTool], ) - // Editing: C - Split the clip currently under the pointer at its exact hover frame. - // This intentionally does not fall back to currentFrame or the throttled - // playback preview: a stale preview must never become an edit location. + // Editing: C - Split the clip under the pointer at its exact hover frame, + // or the sole selected clip at the authoritative playhead when the pointer + // is away. The throttled playback preview is never an edit location. useCommandHotkey( 'SPLIT_AT_PLAYHEAD', (event) => { diff --git a/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts index 1bd43ab02..f7b71ffef 100644 --- a/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts +++ b/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts @@ -9,11 +9,33 @@ import { usePlaybackStore } from '@/shared/state/playback' import { HOTKEY_OPTIONS } from '@/config/hotkeys' import type { TimelineShortcutCallbacks } from '../use-timeline-shortcuts' import { useSettingsStore } from '@/features/timeline/deps/settings' +import { useEditorHostContext } from '../../deps/editor' + +function invokeHostHistoryAction( + action: () => Promise | void, + notify: ((notice: { kind: 'error'; message: string }) => void) | undefined, + label: 'undo' | 'redo', +): void { + const reportFailure = () => { + try { + notify?.({ kind: 'error', message: `Host ${label} failed` }) + } catch { + // A notification failure must not turn a rejected host action into an + // unhandled rejection. + } + } + + try { + void Promise.resolve(action()).catch(reportFailure) + } catch { + reportFailure() + } +} export interface UIShortcutOptions { /** - * Undo/redo mutate the timeline temporal store directly without emitting - * host commands, so host-embedded surfaces must mount with this disabled. + * Enables local temporal-store undo/redo. Host mode uses its optional + * history port instead, and disables these shortcuts when that port is absent. */ enableHistory?: boolean } @@ -23,6 +45,9 @@ export function useUIShortcuts( options: UIShortcutOptions = {}, ) { const { enableHistory = true } = options + const { mode: editorMode, host } = useEditorHostContext() + const hostHistory = editorMode === 'host' ? host?.history : undefined + const historyEnabled = editorMode === 'host' ? hostHistory !== undefined : enableHistory const toggleSnap = useTimelineStore((s) => s.toggleSnap) const zoomIn = useZoomStore((s) => s.zoomIn) const zoomOut = useZoomStore((s) => s.zoomOut) @@ -32,7 +57,15 @@ export function useUIShortcuts( 'UNDO', (event) => { event.preventDefault() - useTimelineStore.temporal.getState().undo() + if (hostHistory) { + invokeHostHistoryAction( + () => hostHistory.undo(), + (notice) => host?.notify?.(notice), + 'undo', + ) + } else if (historyEnabled) { + useTimelineStore.temporal.getState().undo() + } if (callbacks.onUndo) { callbacks.onUndo() } @@ -40,9 +73,9 @@ export function useUIShortcuts( { ...HOTKEY_OPTIONS, enableOnFormTags: true, - enabled: enableHistory, + enabled: historyEnabled, }, - [callbacks, enableHistory], + [callbacks, historyEnabled, host, hostHistory], ) // History: Cmd/Ctrl+Shift+Z - Redo @@ -50,7 +83,15 @@ export function useUIShortcuts( 'REDO', (event) => { event.preventDefault() - useTimelineStore.temporal.getState().redo() + if (hostHistory) { + invokeHostHistoryAction( + () => hostHistory.redo(), + (notice) => host?.notify?.(notice), + 'redo', + ) + } else if (historyEnabled) { + useTimelineStore.temporal.getState().redo() + } if (callbacks.onRedo) { callbacks.onRedo() } @@ -58,9 +99,9 @@ export function useUIShortcuts( { ...HOTKEY_OPTIONS, enableOnFormTags: true, - enabled: enableHistory, + enabled: historyEnabled, }, - [callbacks, enableHistory], + [callbacks, historyEnabled, host, hostHistory], ) // UI: S - Toggle Snap diff --git a/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx b/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx index 8a8107e91..caa15ec1d 100644 --- a/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx +++ b/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render } from '@testing-library/react' -import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' import { useEditorStore } from '@/shared/state/editor' import { usePlaybackStore } from '@/shared/state/playback' import { useSelectionStore } from '@/shared/state/selection' @@ -9,7 +9,7 @@ import { useKeyframeSelectionStore } from '../stores/keyframe-selection-store' import { clearTimelineHover, setTimelineHover } from '../utils/timeline-hover-state' import { useHostTimelineShortcuts, useTimelineShortcuts } from './use-timeline-shortcuts' import type { TimelineTrack, VideoItem } from '@/types/timeline' -import { EditorHostProvider } from '../deps/editor' +import { EditorHostProvider, type EditorHost } from '../deps/editor' // Some machines run jsdom with an opaque origin, leaving localStorage // undefined; the zustand persist middleware captures it at store creation @@ -38,16 +38,30 @@ function HostShortcutBindings() { function HostShortcutHarness({ onRippleDelete, + history, + host, }: { onRippleDelete?: (itemIds: readonly string[]) => void | Promise + history?: { undo: () => Promise | void; redo: () => Promise | void } + host?: EditorHost }) { - if (!onRippleDelete) return + if (!onRippleDelete && !history && !host) return + const hostValue = + host ?? + ({ + capabilities: {}, + load: vi.fn(), + resolveMedia: vi.fn(), + submitEdit: vi.fn(), + history, + } as unknown as EditorHost) return ( @@ -119,6 +133,20 @@ describe('useHostTimelineShortcuts', () => { }) }) + afterEach(() => { + document.body.replaceChildren() + }) + + function focusedClipTarget(): HTMLDivElement { + const target = document.createElement('div') + target.dataset.timelineItem = '' + target.dataset.itemId = ITEM.id + target.setAttribute('role', 'button') + target.tabIndex = 0 + document.body.append(target) + return target + } + it('toggles playback on Space', () => { render() @@ -194,6 +222,128 @@ describe('useHostTimelineShortcuts', () => { expect(useTimelineStore.getState().items[0]).toMatchObject({ id: 'clip-1', from: 30 }) }) + it.each([ + ['Ctrl+Z', { ctrlKey: true }], + ['Meta+Z', { metaKey: true }], + ] as const)('routes %s from a focused clip through host history', async (_name, modifier) => { + const undo = vi.fn(async () => undefined) + const redo = vi.fn(async () => undefined) + const target = focusedClipTarget() + useTimelineStore.getState().moveItem('clip-1', 30) + render() + + fireEvent.keyDown(target, { key: 'z', code: 'KeyZ', ...modifier }) + await Promise.resolve() + + expect(undo).toHaveBeenCalledTimes(1) + expect(redo).not.toHaveBeenCalled() + expect(useTimelineStore.getState().items[0]).toMatchObject({ id: 'clip-1', from: 30 }) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it('routes Shift+Meta+Z from a focused clip through host redo', async () => { + const undo = vi.fn(async () => undefined) + const redo = vi.fn(async () => undefined) + const target = focusedClipTarget() + useTimelineStore.getState().moveItem('clip-1', 30) + render() + + fireEvent.keyDown(target, { key: 'z', code: 'KeyZ', metaKey: true, shiftKey: true }) + await Promise.resolve() + + expect(redo).toHaveBeenCalledTimes(1) + expect(undo).not.toHaveBeenCalled() + expect(useTimelineStore.getState().items[0]).toMatchObject({ id: 'clip-1', from: 30 }) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it('notifies when a host history action rejects without an unhandled rejection', async () => { + const notify = vi.fn() + const undo = vi.fn(async () => { + throw new Error('history unavailable') + }) + const redo = vi.fn(async () => undefined) + const target = focusedClipTarget() + const host = { + capabilities: {}, + load: vi.fn(), + resolveMedia: vi.fn(), + submitEdit: vi.fn(), + history: { undo, redo }, + notify, + } as unknown as EditorHost + render() + + fireEvent.keyDown(target, { key: 'z', code: 'KeyZ', metaKey: true }) + await Promise.resolve() + await Promise.resolve() + + expect(undo).toHaveBeenCalledTimes(1) + expect(notify).toHaveBeenCalledWith({ kind: 'error', message: 'Host undo failed' }) + }) + + it('does nothing in host mode when the history port is missing', () => { + const target = focusedClipTarget() + useTimelineStore.getState().moveItem('clip-1', 30) + const host = { + capabilities: {}, + load: vi.fn(), + resolveMedia: vi.fn(), + submitEdit: vi.fn(), + } as unknown as EditorHost + render() + + fireEvent.keyDown(target, { key: 'z', code: 'KeyZ', metaKey: true }) + + expect(useTimelineStore.getState().items[0]).toMatchObject({ id: 'clip-1', from: 30 }) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it.each([ + [ + 'editable clip', + () => { + const target = focusedClipTarget() + target.setAttribute('contenteditable', 'true') + return target + }, + ], + [ + 'nested control', + () => { + const target = focusedClipTarget() + const button = document.createElement('button') + button.textContent = 'Nested' + target.append(button) + return button + }, + ], + [ + 'dialog clip', + () => { + const dialog = document.createElement('div') + dialog.setAttribute('role', 'dialog') + document.body.append(dialog) + const target = document.createElement('div') + target.dataset.timelineItem = '' + target.dataset.itemId = ITEM.id + target.setAttribute('role', 'button') + dialog.append(target) + return target + }, + ], + ] as const)('protects %s from host history shortcuts', (_name, createTarget) => { + const undo = vi.fn(async () => undefined) + const redo = vi.fn(async () => undefined) + const target = createTarget() + render() + + fireEvent.keyDown(target, { key: 'z', code: 'KeyZ', metaKey: true }) + + expect(undo).not.toHaveBeenCalled() + expect(redo).not.toHaveBeenCalled() + }) + it('still undoes on Mod+Z with the full timeline shortcuts (control)', () => { useTimelineStore.getState().moveItem('clip-1', 30) expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) diff --git a/src/features/timeline/hooks/use-timeline-shortcuts.ts b/src/features/timeline/hooks/use-timeline-shortcuts.ts index 9d49e76f5..f996505de 100644 --- a/src/features/timeline/hooks/use-timeline-shortcuts.ts +++ b/src/features/timeline/hooks/use-timeline-shortcuts.ts @@ -55,9 +55,10 @@ export function useTimelineShortcuts(callbacks: TimelineShortcutCallbacks = {}) * - Delete/Backspace — produces one authoritative ripple_delete request. * - UI zoom/snap (S, Shift+S, Cmd/Ctrl+=/-, \, Shift+\) — local view state. * - * Deliberately excluded: undo/redo (mutate the temporal store without host - * commands), modifier ripple delete, clipboard, markers, in/out points, nudges, join, - * freeze frame, and clear-keyframes — all unsupported by the host slice. + * Undo/redo use the optional host history port when supplied and stay disabled + * when it is absent. Deliberately excluded otherwise: modifier ripple delete, + * clipboard, markers, in/out points, nudges, join, freeze frame, and + * clear-keyframes — all unsupported by the host slice. */ export function useHostTimelineShortcuts(callbacks: TimelineShortcutCallbacks = {}) { usePlaybackShortcuts(callbacks) diff --git a/tests/browser/host-delete-ripple.spec.ts b/tests/browser/host-delete-ripple.spec.ts index e2319966b..1c96d44d4 100644 --- a/tests/browser/host-delete-ripple.spec.ts +++ b/tests/browser/host-delete-ripple.spec.ts @@ -3,40 +3,106 @@ import { expect, test, type Page } from 'playwright/test' test.setTimeout(120_000) test.describe('host authoritative delete/ripple', () => { - test('Delete submits one ripple command and updates only after the receipt', async ({ - page, - }: { - page: Page - }) => { - await page.goto('/tests/browser/host-delete-ripple.html') - await page.waitForSelector('[data-freecut-editor-surface="host"]') - await page.waitForFunction(() => Boolean(window.__freecutDeleteRippleFixture)) + for (const key of ['Delete', 'Backspace']) { + test(`${key} from a focused clip submits one ripple command and waits for the receipt`, async ({ + page, + }: { + page: Page + }) => { + await page.setViewportSize({ width: 1600, height: 1000 }) + await page.goto('/tests/browser/host-delete-ripple.html') + await page.waitForSelector('[data-freecut-editor-surface="host"]') + await page.waitForFunction(() => Boolean(window.__freecutDeleteRippleFixture)) - await page.evaluate(() => window.__freecutDeleteRippleFixture.selectClip()) - await page.keyboard.press('Delete') + const clip = page.locator('[data-timeline-item][data-item-id="video-1"]') + await clip.click() + await expect(clip).toBeFocused() + await expect(clip).toHaveAttribute('aria-pressed', 'true') + await page.keyboard.press(key) - await page.waitForFunction( - () => window.__freecutDeleteRippleFixture.getLastBatch()?.commands.length === 1, - ) - const batch = await page.evaluate(() => window.__freecutDeleteRippleFixture.getLastBatch()) - expect(batch).toMatchObject({ - commands: [ - { - type: 'ripple_delete', - start_us: 0, - end_us: 1_000_000, - track_ids: null, - item_ids: ['video-1'], - intent: 'ripple', - }, - ], + await page.waitForFunction( + () => window.__freecutDeleteRippleFixture.getLastBatch()?.commands.length === 1, + ) + const batch = await page.evaluate(() => window.__freecutDeleteRippleFixture.getLastBatch()) + expect(batch).toMatchObject({ + commands: [ + { + type: 'ripple_delete', + start_us: 0, + end_us: 1_000_000, + track_ids: null, + item_ids: ['video-1', 'audio-1', 'caption-1'], + intent: 'ripple', + }, + ], + }) + await expect(page.locator('[data-timeline-item="true"][data-item-id="video-1"]')).toHaveCount( + 1, + ) + await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseReceipt()) + await expect(page.locator('[data-timeline-item="true"][data-item-id="video-1"]')).toHaveCount( + 0, + ) + await expect(page.locator('[data-timeline-item="true"][data-item-id="video-2"]')).toHaveCount( + 1, + ) + await page.screenshot({ + path: `artifacts/host-${key.toLowerCase()}-ripple-after.png`, + fullPage: true, + }) }) - await expect(page.locator('[data-timeline-item="true"][data-item-id="video-1"]')).toHaveCount(1) - await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseReceipt()) - await expect(page.locator('[data-timeline-item="true"][data-item-id="video-1"]')).toHaveCount(0) - await expect(page.locator('[data-timeline-item="true"][data-item-id="video-2"]')).toHaveCount(1) - await page.screenshot({ path: 'artifacts/host-delete-ripple-after.png', fullPage: true }) - }) + } + + for (const modifier of ['Meta', 'Control']) { + test(`${modifier}+Z and Shift+Z restore host history after a focused clip deletion`, async ({ + page, + }) => { + await page.setViewportSize({ width: 1600, height: 1000 }) + await page.goto('/tests/browser/host-delete-ripple.html') + const first = page.locator('[data-timeline-item][data-item-id="video-1"]') + const second = page.locator('[data-timeline-item][data-item-id="video-2"]') + await first.click() + await expect(first).toBeFocused() + await page.keyboard.press('Delete') + await page.waitForFunction(() => Boolean(window.__freecutDeleteRippleFixture.getLastBatch())) + await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseReceipt()) + await expect(first).toHaveCount(0) + await second.click() + await expect(second).toBeFocused() + + await page.keyboard.press(`${modifier}+z`) + await expect + .poll(() => page.evaluate(() => window.__freecutDeleteRippleFixture.getHistoryCalls())) + .toEqual(['undo']) + // No local temporal rollback: only the host's snapshot restores the clip. + await expect(first).toHaveCount(0) + await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseHistory()) + await expect(first).toHaveCount(1) + await expect(first).toHaveAttribute('data-timeline-start-frame', '0') + await expect(first).toHaveAttribute('data-timeline-duration-frames', '30') + await expect(second).toHaveAttribute('data-timeline-start-frame', '30') + await first.scrollIntoViewIfNeeded() + await page.screenshot({ + path: `artifacts/host-history-${modifier.toLowerCase()}-undone.png`, + fullPage: true, + }) + await first.click() + await expect(first).toBeFocused() + + await page.keyboard.press(`${modifier}+Shift+z`) + await expect + .poll(() => page.evaluate(() => window.__freecutDeleteRippleFixture.getHistoryCalls())) + .toEqual(['undo', 'redo']) + await expect(first).toHaveCount(1) + await page.evaluate(() => window.__freecutDeleteRippleFixture.releaseHistory()) + await expect(first).toHaveCount(0) + await expect(second).toHaveAttribute('data-timeline-start-frame', '0') + await page.screenshot({ + path: `artifacts/host-history-${modifier.toLowerCase()}-after.png`, + fullPage: true, + }) + }) + } test('right-click Delete uses the same authoritative ripple path', async ({ page, @@ -84,6 +150,11 @@ test.describe('host authoritative delete/ripple', () => { const rejectedBatch = await page.evaluate(() => window.__freecutDeleteRippleFixture.getLastBatch(), ) + // Reacquire selection/focus after the rejected authoritative receipt. + const retryClip = page.locator('[data-timeline-item][data-item-id="video-1"]') + await retryClip.click() + await expect(retryClip).toBeFocused() + await expect(retryClip).toHaveAttribute('aria-pressed', 'true') await page.keyboard.press('Delete') await page.waitForFunction( (previousId) => diff --git a/tests/browser/host-delete-ripple.tsx b/tests/browser/host-delete-ripple.tsx index 9f561ae70..12dace07a 100644 --- a/tests/browser/host-delete-ripple.tsx +++ b/tests/browser/host-delete-ripple.tsx @@ -13,6 +13,8 @@ declare global { rejectNextDelete(): void getLastBatch(): EditCommandBatch | null releaseReceipt(): void + getHistoryCalls(): string[] + releaseHistory(): void } } } @@ -23,7 +25,7 @@ const cohortItems = [ { type: 'caption_cue' as const, id: 'caption-1', trackId: 'captions' }, ] -function fixtureSnapshot(revision = 0): EmbeddedEditorSnapshot { +function fixtureSnapshot(revision = 0, deleted = revision !== 0): EmbeddedEditorSnapshot { // fallow-ignore-next-line complexity const makeItems = (trackId: string) => { const cohort = cohortItems.find((item) => item.trackId === trackId)! @@ -53,7 +55,7 @@ function fixtureSnapshot(revision = 0): EmbeddedEditorSnapshot { ...(downstream.type !== 'caption_cue' ? { sourceStart: 30, sourceEnd: 60 } : {}), }, ] - return revision === 0 ? items : [{ ...items[1]!, from: 0 }] + return !deleted ? items : [{ ...items[1]!, from: 0 }] } return { @@ -68,7 +70,7 @@ function fixtureSnapshot(revision = 0): EmbeddedEditorSnapshot { timelineId: 'delete-ripple-timeline', revision, fps: 30, - durationInFrames: revision === 0 ? 90 : 60, + durationInFrames: !deleted ? 90 : 60, media: [ { media_id: 'media-1', @@ -131,11 +133,36 @@ let currentSnapshot = fixtureSnapshot() let lastBatch: EditCommandBatch | null = null let releaseReceipt: (() => void) | null = null let rejectNextDelete = false +const snapshotListeners = new Set<(snapshot: EmbeddedEditorSnapshot) => void>() +const historyCalls: string[] = [] +let releaseHistory: (() => void) | null = null + +function performHistory(action: 'undo' | 'redo'): Promise { + historyCalls.push(action) + return new Promise((resolve) => { + releaseHistory = () => { + releaseHistory = null + currentSnapshot = fixtureSnapshot(currentSnapshot.timeline.revision + 1, action === 'redo') + for (const listener of snapshotListeners) listener(currentSnapshot) + resolve() + } + }) +} const host: EditorHost = { capabilities: { 'timeline.remove': true, 'media.resolve': false }, load: () => currentSnapshot, resolveMedia: () => null, + subscribe: (listener) => { + snapshotListeners.add(listener) + return () => { + snapshotListeners.delete(listener) + } + }, + history: { + undo: () => performHistory('undo'), + redo: () => performHistory('redo'), + }, submitEdit: (batch) => { lastBatch = batch const command = batch.commands[0] @@ -179,6 +206,8 @@ window.__freecutDeleteRippleFixture = { }, getLastBatch: () => lastBatch, releaseReceipt: () => releaseReceipt?.(), + getHistoryCalls: () => [...historyCalls], + releaseHistory: () => releaseHistory?.(), } const hostBox = document.querySelector('#host-box')