Skip to content

feat(#93): add clip right-click context menu#108

Open
cuic19053-hue wants to merge 8 commits into
appergb:mainfrom
cuic19053-hue:feat-93-clip-context-menu
Open

feat(#93): add clip right-click context menu#108
cuic19053-hue wants to merge 8 commits into
appergb:mainfrom
cuic19053-hue:feat-93-clip-context-menu

Conversation

@cuic19053-hue

Copy link
Copy Markdown
Contributor

Closes #93.

What

Right-clicking a clip on the timeline now opens a context menu with the most common clip actions: Split at Playhead, Delete, and Link/Unlink.

How

  • ClipContextMenu.tsx (new): a fixed-position menu component. Reads the clip's linkGroupId to decide whether to show Link or Unlink. Closes on outside click or Escape. Each item ensures the right-clicked clip is selected before firing the action.
  • TimelineContainer.tsx: added onContextMenu to the content canvas — hit-tests the click position, selects the clip if it isn't already, and opens the menu. Empty-space right-click does nothing (the global suppressor in App.tsx still hides the native menu).
  • dict.ts: adds contextMenu.split / delete / link / unlink (zh-CN + en).

Notes

Testing

  • pnpm run build (tsc -b + vite build) passes
  • Manual: right-click a clip -> menu appears -> Split/Delete/Link-Unlink all work; right-click empty space -> no menu; Escape / outside click closes it.

@appergb appergb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cuic19053-hue 自动审核结论:请修改(REQUEST_CHANGES)。右键菜单方向正确(onContextMenu→hitTestClip→setMenu 接线真实有效、命中后先选中再弹菜单与上游一致),但有两个阻塞 + 两个 React 正确性问题:

阻塞项:

  1. 冲突,需 rebase:diff 基于旧版 TimelineContainer.tsx(const [, forceTick]),main 现已是 const [dragTick, forceTick] 并新增 waveformsRef/waveformVersion/useT/mediaItems 等。请 rebase 到最新 main 解决 hunk 冲突。
  2. 菜单定位 bug(功能不可用):ClipContextMenu 根元素 position:fixed 但缺 top/left(ClipContextMenu.tsx:109-122),永远渲染在视口 (0,0),与右键位置无关。请在 onContextMenu 把 e.clientX/e.clientY 存入 menu state 再传给菜单定位。

必修(正确性):
3. 渲染期调用 onClose():ClipContextMenu.tsx:57-60 在组件函数体里直接 if (!clip) { onClose(); ... },违反 React 渲染纯函数,严格模式会告警并可能死循环。改用 useEffect 或在父组件 onContextMenu 预先校验 clip 存在。

验收/1:1 缺口:issue #93 验收要求菜单含 Swap Media / Save as Media / Extract Audio(即使 disabled/stub),目前缺失;Copy/Paste 注明等 #94 可接受,但上游 Copy 是无条件项。建议补 disabled 占位项。另:key={i} 数组下标做 key、onMouseEnter 直接 mutate DOM style(违反 immutability,建议 CSS :hover)、props 用具名 interface、dict 重复文案。

请 rebase + 修定位/onClose + 补菜单项后重提,自验 tsc + vite build。

@appergb appergb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cuic19053-hue 复审:rebase 冲突已解决、重复 handler 已清,CI 双绿 👍。但上轮两个阻塞项仍未修,合不了:

  1. 菜单定位 bug 仍在(功能不可用)。 TimelineContainerconst [menu, setMenu] = useState<{ clipId: string }> 只存了 clipId,没存右键坐标;ClipContextMenu<div>position:fixed没有 top/left,所以菜单永远渲染在视口 (0,0),跟右键位置无关。
    修法:onContextMenu 里把 e.clientX/e.clientY 一起存进 menu state(setMenu({ clipId, x: e.clientX, y: e.clientY })),传给 ClipContextMenu 作为 left/top;并做视口边界翻转(贴右/底边时向左上展开),对齐上游右键菜单跟随光标的行为。

  2. 渲染期调用 onClose() 仍在。 ClipContextMenu 函数体 if (!clip) { onClose(); return null; } 在 render 阶段触发父组件 setState,违反 React 渲染纯函数,严格模式会告警/可能循环。改成:命中校验放到 onContextMenu(已有),组件内 clip 缺失时直接 return null(用 useEffect 上报关闭),不要在渲染期 onClose()

次要(非阻塞,建议顺手):菜单项目前仅 Split/Delete/Link/Unlink,issue #93 验收还要 Swap Media / Save as Media / Extract Audio(可 disabled 占位);key={i} 建议用稳定 key;hover 直接改 DOM style 建议用 CSS :hover

把 1、2 修掉就可以再过。其余等 #94 的 Copy/Paste 可后续补。

@cuic19053-hue

Copy link
Copy Markdown
Contributor Author

@appergb 请求重新审查。此前反馈已全部修复,CI 双绿(Rust ✅ Web ✅,commit \�1e673e\):

  1. 菜单定位:\onContextMenu\ 存 { clipId, x, y }\,传给 \ClipContextMenu\ 作 \left/top\,\useLayoutEffect\ 测量做视口边界翻转
  2. React 渲染纯函数:\ClipContextMenu\ 函数体改为 \if (!clip) return null\,不在渲染期调用 \onClose()\
  3. 菜单项占位:补 Swap Media / Save as Media / Extract Audio(disabled)
  4. TS6133 修复:\onContextMenu\ 已绑定到 content canvas(此前定义未绑定导致 Web CI 失败)

请 re-review,谢谢!

cuic19053-hue and others added 8 commits June 25, 2026 22:36
* feat(timeline): copy / cut / paste clips (⌘C / ⌘X / ⌘V) (appergb#94)

Adds the standard clipboard shortcuts that were completely missing. Only
⌘C/⌘X/⌘V were absent — the unmodified C/V already switch tools (razor /
pointer), and the mod-prefixed branch had no handlers.

Frontend only:
- clipboardStore: new Zustand store holding deep snapshots of the selected
  clips plus the source first-frame, so a paste can re-place the group
  relative to the current playhead. UI-only, never persisted.
- editActions: copyClips / cutClips / pasteClipsAtPlayhead.
  - copy: snapshot selected clips + their track index + min startFrame.
  - cut: copy then deleteSelectedClips.
  - paste: offset each clip's startFrame by `activeFrame - sourceFirstFrame`,
    clear addLinkedAudio so the paste stands alone (mirrors upstream
    `pasteClipsAtPlayhead` link re-reflection), and select the new clips.
    Clips whose source track no longer exists are skipped.
- useKeyboardShortcuts: wire ⌘C / ⌘X / ⌘V inside the existing `if (mod)`
  block — no conflict with the unmodified C/V tool switches.
- i18n: 4 new keys (zh-CN + en) for copy / cut / paste / clipboardEmpty.

Closes appergb#94.

* fix(appergb#94): rebase onto main + linkGroup re-mapping + empty-clipboard toast

Address review feedback on PR appergb#105:

1. Rebased onto latest main (resolved import conflict: kept both trimToPlayheadEdits and useClipboardStore).

2. copyClips now expands link groups: if a selected clip has a linkGroupId, all linked companions are included in the clipboard (mirrors upstream copyClips), so a paste reproduces the video+audio pair.

3. pasteClipsAtPlayhead now re-establishes link groups after addClips: clips that shared a linkGroupId in the clipboard are re-linked via linkClips, preserving video+audio linkage.

4. Empty-clipboard paste now shows a toast (edit.clipboardEmpty) instead of silently doing nothing. Added toast mechanism to uiStore + Toast component in App.tsx.

---------

Co-authored-by: baiqing <lbx12309@icloud.com>
* feat(swap-media): 实现 SwapMedia 编辑命令,支持替换 clip 媒体 (appergb#101)

后端:
- 新增 EditCommand::SwapMedia 变体,替换 clip 的 media_ref
- 校验新媒体存在于 manifest,若时长不足自动截断 duration + 调整 trim_end
- 保留所有编辑属性(transform/crop/keyframe tracks/grade/masks/effects/fade)
- media_type 隐含 source_clip_type(spec "sync media_type" 场景)
- 新增 EditRequest::SwapMedia DTO + into_command 映射
- 6 个单元测试:等长替换/较短截断/媒体不存在/同步 media_type/clip 不存在/undo

前端:
- types.ts 新增 swapMedia EditRequest 变体
- editActions.ts 新增 swapMedia(clipId, mediaRef, options?) action
- Inspector 新增「替换媒体」section + 内联媒体选择器
- i18n 中英文翻译

Closes appergb#101

* style: fix cargo fmt in command.rs and tests (appergb#101)

* fix: correct cargo fmt in command_apply.rs (appergb#101)

* fix: align trailing comment with 43 spaces (appergb#101)

* chore: trim playback whitespace

* fix(swap-media): simplify DTO to 2-arg + frontend type-consistency filter (review appergb#121)

* fix(swap-media): singleLinkGroup gate + extract SwapMediaSection out of Inspector (appergb#101)

---------

Co-authored-by: baiqing <lbx12309@icloud.com>
* feat(inspector): live sampling + missing fields (crop/fade/flip) (appergb#97)

Backend (opentake-ops + src-tauri):
- Extend ClipProperties with crop, fade_in/out_frames, fade_in/out_interpolation,
  flip_horizontal, flip_vertical
- set_clip_properties writes new fields; fade clamps to clip duration;
  flip_* writes to transform.flip_*
- ClipPropertiesDto mirrors fields with serde camelCase
- 5 unit tests: crop sets+clears track, fade frames+interp, fade clamps,
  flip writes to transform, multiple fields at once

Frontend (web):
- clip.ts: 1:1 port of Rust Clip::*_at sampling methods (opacity/volume/
  rotation/size/topLeft/crop), fadeMultiplier, db<->linear, generic
  sampleKeyframeTrack with number/AnimPair/Crop lerp
- Inspector.tsx: read activeFrame from uiStore; show sampled values at
  playhead; switch to ReadOnlyValue + AnimatedHint when a track is active
- 4 new sections: Position (top-left x/y), Crop (4 edge insets 0-1),
  Flip (2 checkboxes), Fade (in/out frames + interpolation selects)
- Fade section appears on both video and audio tabs
- types.ts: extend ClipPropertiesReq with camelCase fields
- dict.ts: i18n keys for new sections (zh-CN + en)

Closes appergb#97

* style: fix cargo fmt import in command.rs (appergb#97)

* fix: add ..Default::default() for new ClipProperties fields (appergb#97)

* fix(appergb#97): use clip.opacity/volume for editable fields, sampled* only for animated (review appergb#122)
Closes appergb#93.

- New ClipContextMenu component with Split / Delete / Link-Unlink
- TimelineContainer: onContextMenu hit-tests the clip, selects it if
  needed, and opens the menu; closes on outside click or Escape
- i18n: contextMenu.split/delete/link/unlink (zh-CN + en)
…er-phase onClose()

Blocking items from review:
1. Menu now follows cursor (x/y from onContextMenu -> ClipContextMenu
   left/top) with useLayoutEffect viewport-boundary flip (right/bottom
   overflow -> open left/up).
2. Removed onClose() call during render; clip-missing now returns null
   and reports close via useEffect (no parent setState mid-render).

Minor items:
- Added disabled placeholder items: Swap Media / Save as Media / Extract Audio.
- Replaced key={i} with stable key={item.id}.
- Replaced imperative onMouseEnter/Leave DOM mutation with CSS :hover.
Wire the Swap Media context-menu action in ClipContextMenu.tsx:
- Availability gate: enabled only when the clip is non-text AND alone in its
  link group (SPEC §5.10 "非 text 且单链组" = upstream TimelineView.menu).
  Multi-clip link groups (e.g. linked A/V pairs) stay disabled to avoid
  desyncing partners.
- On click, opens a media-picker modal pre-filtered by strict type equality
  (item.type === clip.mediaType, mirroring upstream
  isAssetCompatibleWithPendingSwap). Backend re-validates as a safety net.

New files:
- web/src/components/timeline/SwapMediaPicker.tsx: modal list of compatible
  library assets; calls edit.swapMedia() on selection; shows backend error
  message (e.g. type-mismatch refusal) inline; Esc-to-close.

New helpers / state:
- web/src/lib/clip.ts: isSingleLinkGroup(clip, timeline) helper.
- web/src/store/uiStore.ts: pendingSwapClipId + setPendingSwapClipId.

Touched:
- web/src/components/timeline/ClipContextMenu.tsx: gate + open picker.
- web/src/components/timeline/TimelineContainer.tsx: render SwapMediaPicker.
- web/src/i18n/dict.ts: swapMedia.noCandidates (zh + en).
- web/src/lib/types.ts: swapMedia EditRequest variant.
- web/src/store/editActions.ts: 2-arg swapMedia wrapper around editApply.

Pairs with feat-101-swap-media (the backend `replaceClipMediaRef(
resetTrim=false)` route). tsc --noEmit + pnpm build green.
@cuic19053-hue cuic19053-hue force-pushed the feat-93-clip-context-menu branch from a1e673e to f9de899 Compare June 25, 2026 15:03
@cuic19053-hue

Copy link
Copy Markdown
Contributor Author

@appergb 请求重新审查。R2 反馈已逐条核对落实,CI 双绿(Rust ✅ Web ✅,commit f9de899):

  1. 菜单定位onContextMenu{ clipId, x: e.clientX, y: e.clientY }TimelineContainer.tsx:596),传给 ClipContextMenuleft/topuseLayoutEffect 测量做视口边界翻转(贴右/底边向左上展开)
  2. 渲染纯函数ClipContextMenu 函数体首行 if (!clip) return nullClipContextMenu.tsx:88),不在渲染期调用 onClose()

请 re-review,谢谢!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[timeline] 片段右键菜单缺失(Copy/Paste/Delete/Split/Swap/Save as/Extract Audio)

2 participants