feat(editor): duplicate annotations#342
Open
kuishou68 wants to merge 1 commit intosiddharthvaddem:mainfrom
Open
feat(editor): duplicate annotations#342kuishou68 wants to merge 1 commit intosiddharthvaddem:mainfrom
kuishou68 wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request adds annotation duplication functionality to the video editor. Users can now duplicate annotations via a new button in the annotation settings panel, preserving styling and position while offsetting the duplicate by a small amount. Changes
Sequence DiagramsequenceDiagram
actor User
participant ASP as AnnotationSettingsPanel
participant SP as SettingsPanel
participant VE as VideoEditor
participant State as annotationRegions
User->>ASP: Click Duplicate Button
ASP->>SP: onDuplicate()
SP->>VE: onAnnotationDuplicate(annotationId)
VE->>VE: Clone annotation<br/>Generate new ID<br/>Offset position (+4, +4)<br/>Increment z-index<br/>Deep copy properties
VE->>State: pushState(new annotation)
VE->>VE: Update selection<br/>to new annotation ID
VE-->>ASP: UI updates
ASP-->>User: Duplicate annotation displayed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/AnnotationSettingsPanel.tsx`:
- Around line 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.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f645b4f1-d924-463b-9d88-8ba84ead134b
📒 Files selected for processing (3)
src/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsx
Comment on lines
+603
to
+613
| <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> |
There was a problem hiding this comment.
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.
Comment on lines
+834
to
+859
| 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], | ||
| ); |
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- add a duplicate action for selected annotations in the settings panel\n- preserve annotation content, style, timing, and type when duplicating\n- assign a fresh annotation id/z-index and slightly offset the duplicate so it is easy to distinguish\n\nCloses #167
Summary by CodeRabbit