Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions src/components/video-editor/AnnotationSettingsPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Block from "@uiw/react-color-block";
import {
AlignCenter,
Copy,
AlignLeft,
AlignRight,
Bold,
Expand Down Expand Up @@ -40,6 +41,7 @@ interface AnnotationSettingsPanelProps {
onTypeChange: (type: AnnotationType) => void;
onStyleChange: (style: Partial<AnnotationRegion["style"]>) => void;
onFigureDataChange?: (figureData: FigureData) => void;
onDuplicate?: () => void;
onDelete: () => void;
}

Expand All @@ -62,6 +64,7 @@ export function AnnotationSettingsPanel({
onTypeChange,
onStyleChange,
onFigureDataChange,
onDuplicate,
onDelete,
}: AnnotationSettingsPanelProps) {
const t = useScopedT("settings");
Expand Down Expand Up @@ -597,15 +600,28 @@ export function AnnotationSettingsPanel({
</TabsContent>
</Tabs>

<Button
onClick={onDelete}
variant="destructive"
size="sm"
className="w-full gap-2 bg-red-500/10 text-red-400 border border-red-500/20 hover:bg-red-500/20 hover:border-red-500/30 transition-all mt-4"
>
<Trash2 className="w-4 h-4" />
{t("annotation.deleteAnnotation")}
</Button>
<div className="mt-4 grid grid-cols-2 gap-2">
<Button
onClick={() => onDuplicate?.()}
variant="outline"
size="sm"
disabled={!onDuplicate}
className="w-full gap-2 bg-white/5 text-slate-200 border border-white/10 hover:bg-white/10 hover:border-white/20 transition-all"
>
<Copy className="w-4 h-4" />
Duplicate
</Button>
Comment on lines +603 to +613
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize the new Duplicate button label.

Line 612 hardcodes "Duplicate" while the panel otherwise uses translation keys. This introduces a localization gap in non-English locales.

Suggested fix
 					<Button
 						onClick={() => onDuplicate?.()}
 						variant="outline"
 						size="sm"
 						disabled={!onDuplicate}
 						className="w-full gap-2 bg-white/5 text-slate-200 border border-white/10 hover:bg-white/10 hover:border-white/20 transition-all"
 					>
 						<Copy className="w-4 h-4" />
-						Duplicate
+						{t("annotation.duplicateAnnotation")}
 					</Button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div className="mt-4 grid grid-cols-2 gap-2">
<Button
onClick={() => onDuplicate?.()}
variant="outline"
size="sm"
disabled={!onDuplicate}
className="w-full gap-2 bg-white/5 text-slate-200 border border-white/10 hover:bg-white/10 hover:border-white/20 transition-all"
>
<Copy className="w-4 h-4" />
Duplicate
</Button>
<div className="mt-4 grid grid-cols-2 gap-2">
<Button
onClick={() => onDuplicate?.()}
variant="outline"
size="sm"
disabled={!onDuplicate}
className="w-full gap-2 bg-white/5 text-slate-200 border border-white/10 hover:bg-white/10 hover:border-white/20 transition-all"
>
<Copy className="w-4 h-4" />
{t("annotation.duplicateAnnotation")}
</Button>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/video-editor/AnnotationSettingsPanel.tsx` around lines 603 -
613, The "Duplicate" button in AnnotationSettingsPanel is hardcoded and must be
localized; replace the literal "Duplicate" string inside the Button (the element
with onClick={() => onDuplicate?.()} and className...) with the component's i18n
call (the same translation helper used elsewhere in this file, e.g., t('...') or
useTranslations('...')) using the appropriate translation key (e.g., "duplicate"
or "annotation.duplicate"), and add that key to the project's locale files so
the label is translated in non-English locales.


<Button
onClick={onDelete}
variant="destructive"
size="sm"
className="w-full gap-2 bg-red-500/10 text-red-400 border border-red-500/20 hover:bg-red-500/20 hover:border-red-500/30 transition-all"
>
<Trash2 className="w-4 h-4" />
{t("annotation.deleteAnnotation")}
</Button>
</div>

<div className="mt-6 p-3 bg-white/5 rounded-lg border border-white/5">
<div className="flex items-center gap-2 mb-2 text-slate-300">
Expand Down
3 changes: 3 additions & 0 deletions src/components/video-editor/SettingsPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ interface SettingsPanelProps {
onAnnotationTypeChange?: (id: string, type: AnnotationType) => void;
onAnnotationStyleChange?: (id: string, style: Partial<AnnotationRegion["style"]>) => void;
onAnnotationFigureDataChange?: (id: string, figureData: FigureData) => void;
onAnnotationDuplicate?: (id: string) => void;
onAnnotationDelete?: (id: string) => void;
selectedSpeedId?: string | null;
selectedSpeedValue?: PlaybackSpeed | null;
Expand Down Expand Up @@ -213,6 +214,7 @@ export function SettingsPanel({
onAnnotationTypeChange,
onAnnotationStyleChange,
onAnnotationFigureDataChange,
onAnnotationDuplicate,
onAnnotationDelete,
selectedSpeedId,
selectedSpeedValue,
Expand Down Expand Up @@ -466,6 +468,7 @@ export function SettingsPanel({
? (figureData) => onAnnotationFigureDataChange(selectedAnnotation.id, figureData)
: undefined
}
onDuplicate={onAnnotationDuplicate ? () => onAnnotationDuplicate(selectedAnnotation.id) : undefined}
onDelete={() => onAnnotationDelete(selectedAnnotation.id)}
/>
);
Expand Down
28 changes: 28 additions & 0 deletions src/components/video-editor/VideoEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,33 @@ export default function VideoEditor() {
[pushState],
);

const handleAnnotationDuplicate = useCallback(
(id: string) => {
const duplicateId = `annotation-${nextAnnotationIdRef.current++}`;
const duplicateZIndex = nextAnnotationZIndexRef.current++;
pushState((prev) => {
const source = prev.annotationRegions.find((region) => region.id === id);
if (!source) return {};

const duplicate: AnnotationRegion = {
...source,
id: duplicateId,
zIndex: duplicateZIndex,
position: { x: source.position.x + 4, y: source.position.y + 4 },
size: { ...source.size },
style: { ...source.style },
figureData: source.figureData ? { ...source.figureData } : undefined,
};

return { annotationRegions: [...prev.annotationRegions, duplicate] };
});
setSelectedAnnotationId(duplicateId);
setSelectedZoomId(null);
setSelectedTrimId(null);
},
[pushState],
);
Comment on lines +834 to +859
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard duplicate flow when the source annotation is missing.

At Line 840, returning {} means no annotation is duplicated, but Line 854 still selects duplicateId, and Line 836-837 already consumed ID/z-index values. This can create a no-op history checkpoint and transient invalid selection.

Suggested fix
 const handleAnnotationDuplicate = useCallback(
 		(id: string) => {
+			const source = annotationRegions.find((region) => region.id === id);
+			if (!source) return;
+
 			const duplicateId = `annotation-${nextAnnotationIdRef.current++}`;
 			const duplicateZIndex = nextAnnotationZIndexRef.current++;
 			pushState((prev) => {
-				const source = prev.annotationRegions.find((region) => region.id === id);
-				if (!source) return {};
-
 				const duplicate: AnnotationRegion = {
 					...source,
 					id: duplicateId,
 					zIndex: duplicateZIndex,
 					position: { x: source.position.x + 4, y: source.position.y + 4 },
 					size: { ...source.size },
 					style: { ...source.style },
 					figureData: source.figureData ? { ...source.figureData } : undefined,
 				};

 				return { annotationRegions: [...prev.annotationRegions, duplicate] };
 			});
 			setSelectedAnnotationId(duplicateId);
 			setSelectedZoomId(null);
 			setSelectedTrimId(null);
 		},
-		[pushState],
+		[annotationRegions, pushState],
 	);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/video-editor/VideoEditor.tsx` around lines 834 - 859,
handleAnnotationDuplicate currently increments nextAnnotationIdRef and
nextAnnotationZIndexRef and sets selection even when the source region isn't
found, causing wasted IDs and invalid selection; modify the flow so you first
use pushState to find the source region and only generate/consume the new
duplicateId and duplicateZIndex after confirming the source exists (or abort
without mutating refs), returning the previous state unchanged if source is
missing, and only call setSelectedAnnotationId(duplicateId) /
setSelectedZoomId(null) / setSelectedTrimId(null) when a duplicate was actually
created; you can either move the id/zIndex increments inside the pushState
callback after the source lookup or detect source absence and skip both state
change and selection updates to avoid no-op history entries and invalid
selection.


const handleAnnotationDelete = useCallback(
(id: string) => {
pushState((prev) => ({
Expand Down Expand Up @@ -1680,6 +1707,7 @@ export default function VideoEditor() {
onAnnotationTypeChange={handleAnnotationTypeChange}
onAnnotationStyleChange={handleAnnotationStyleChange}
onAnnotationFigureDataChange={handleAnnotationFigureDataChange}
onAnnotationDuplicate={handleAnnotationDuplicate}
onAnnotationDelete={handleAnnotationDelete}
selectedSpeedId={selectedSpeedId}
selectedSpeedValue={
Expand Down
Loading