feat(#93): add clip right-click context menu#108
Conversation
appergb
left a comment
There was a problem hiding this comment.
@cuic19053-hue 自动审核结论:请修改(REQUEST_CHANGES)。右键菜单方向正确(onContextMenu→hitTestClip→setMenu 接线真实有效、命中后先选中再弹菜单与上游一致),但有两个阻塞 + 两个 React 正确性问题:
阻塞项:
- 冲突,需 rebase:diff 基于旧版 TimelineContainer.tsx(
const [, forceTick]),main 现已是const [dragTick, forceTick]并新增 waveformsRef/waveformVersion/useT/mediaItems 等。请 rebase 到最新 main 解决 hunk 冲突。 - 菜单定位 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
left a comment
There was a problem hiding this comment.
@cuic19053-hue 复审:rebase 冲突已解决、重复 handler 已清,CI 双绿 👍。但上轮两个阻塞项仍未修,合不了:
-
菜单定位 bug 仍在(功能不可用)。
TimelineContainer里const [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;并做视口边界翻转(贴右/底边时向左上展开),对齐上游右键菜单跟随光标的行为。 -
渲染期调用
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 可后续补。
1d94634 to
fdf8e78
Compare
|
@appergb 请求重新审查。此前反馈已全部修复,CI 双绿(Rust ✅ Web ✅,commit \�1e673e\):
请 re-review,谢谢! |
* 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.
a1e673e to
f9de899
Compare
|
@appergb 请求重新审查。R2 反馈已逐条核对落实,CI 双绿(Rust ✅ Web ✅,commit
请 re-review,谢谢! |
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'slinkGroupIdto 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: addedonContextMenuto 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 inApp.tsxstill hides the native menu).dict.ts: addscontextMenu.split/delete/link/unlink(zh-CN + en).Notes
Testing
pnpm run build(tsc -b + vite build) passes