feat: add blur selection (rectangle, oval)#394
Open
LorenzoLancia wants to merge 6 commits intosiddharthvaddem:mainfrom
Open
feat: add blur selection (rectangle, oval)#394LorenzoLancia wants to merge 6 commits intosiddharthvaddem:mainfrom
LorenzoLancia wants to merge 6 commits intosiddharthvaddem:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded first-class "blur" annotations: types and defaults, timeline/timeline UI, playback/overlay rendering (including freehand editing), settings panel, persistence normalization, export renderer, keyboard shortcut, and i18n entries; introduced blur-specific selection, creation, editing callbacks, and panel flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4f1c6a2ee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/i18n/locales/tr/settings.json (1)
101-121:⚠️ Potential issue | 🟡 MinorTurkish diacritics missing in blur translations.
A few spots need the proper Turkish characters:
- Line 101: "Bulanik" → "Bulanık"
- Line 118: "Bulanik Sekli" → "Bulanık Şekli"
- Line 119: "Dikdortgen" → "Dikdörtgen"
The rest of the file uses correct diacritics, so these look like they slipped through.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locales/tr/settings.json` around lines 101 - 121, Update the Turkish translations for the blur-related keys to use proper diacritics: change the value for "typeBlur" from "Bulanik" to "Bulanık", for "blurShape" from "Bulanik Sekli" to "Bulanık Şekli", and for "blurShapeRectangle" from "Dikdortgen" to "Dikdörtgen" in src/i18n/locales/tr/settings.json so the keys typeBlur, blurShape, and blurShapeRectangle all use correct Turkish characters.
🧹 Nitpick comments (2)
src/lib/exporter/annotationRenderer.ts (1)
168-173: nit: reuse a scratch canvas here for export perf.
Creating a fresh canvas at Line 168 for every blur annotation/frame is kinda expensive; reusing one scratch canvas/context would be cleaner and reduce GC pressure during long renders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/exporter/annotationRenderer.ts` around lines 168 - 173, The code creates a new canvas and context (tempCanvas/tempCtx) for every blur export which is expensive; instead add and reuse a single scratch canvas/context (e.g., a module- or class-scoped scratchCanvas and scratchCtx) and replace the per-call creation with resizing the existing scratchCanvas to sw/sh, clearing it, then using scratchCtx.drawImage(canvas, sx, sy, sw, sh, 0, 0, sw, sh). Ensure you handle the case where getContext returns null, and avoid retaining stale dimensions by updating width/height before drawing.src/components/video-editor/AnnotationOverlay.tsx (1)
94-99: nit: freehand point threshold might be too aggressive.The
0.03threshold (in 0-100% space) means points are added when the cursor moves just 0.03% of the container dimension. For a 1000px container, that's every ~0.3px of movement.This could generate a lot of points for a single stroke. Might want to bump this to something like
0.5or1.0to keep the point array manageable without noticeably affecting smoothness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/AnnotationOverlay.tsx` around lines 94 - 99, The freehand sampling threshold (currently hardcoded as 0.03) is too small and will produce excessive points; update the logic around the point-adding check (the variables points, point, lastPoint and the conditional Math.hypot(dx, dy) >= 0.03) to use a higher threshold (e.g., 0.5 or 1.0) or introduce a named constant like FREEHAND_POINT_THRESHOLD so it’s easy to tune; adjust the comment accordingly to explain the unit (percent of container) and ensure the new threshold reduces point density while preserving stroke smoothness.
🤖 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/AnnotationOverlay.tsx`:
- Around line 235-246: The shape detection in AnnotationOverlay.tsx currently
collapses all non-"oval" values to "rectangle" (const shape =
annotation.blurData?.shape === "oval" ? "oval" : "rectangle"), which prevents
"freehand" from ever being recognized and breaks the freehand flow; fix it by
preserving the actual annotation.blurData?.shape (falling back to a sensible
default like "rectangle" only when undefined), then leave the subsequent
activeFreehandPoints logic (which checks shape === "freehand") intact so
isFreehandDrawing, draftFreehandPoints, annotation.blurData?.freehandPoints, and
DEFAULT_BLUR_INTENSITY work as intended.
- Line 56: The freehand blur is broken because isSelectedFreehandBlur is
hardcoded and shape detection defaults incorrectly; in AnnotationOverlay.tsx
replace the hardcoded const isSelectedFreehandBlur with a boolean derived from
the actual selection state (use isFreehandSelected) and change the shape
detection to read the real value from annotation.blurData (e.g., const shape =
annotation.blurData?.shape || "rectangle") so the code that sets
isFreehandSelected and the interaction handlers render/disable correctly.
In `@src/components/video-editor/BlurSettingsPanel.tsx`:
- Line 96: The span in BlurSettingsPanel.tsx currently hardcodes "Blur
intensity"; add a new locale key annotation.blurIntensity (e.g.,
"blurIntensity": "Blur Intensity" under the annotation namespace in your locale
JSONs) and update the component to use the app's translation helper (e.g.,
replace the literal with the translation call used elsewhere in this panel such
as t('annotation.blurIntensity') or i18n.t('annotation.blurIntensity') so the
label is localized).
In `@src/components/video-editor/projectPersistence.ts`:
- Around line 319-325: In the normalization merge where DEFAULT_BLUR_DATA is
spread into region.blurData, validate region.blurData.shape against the allowed
blur shapes and only accept it if it matches one of the supported values;
otherwise fall back to DEFAULT_BLUR_DATA.shape. Concretely, in the block that
constructs the blur object (the code that spreads DEFAULT_BLUR_DATA and
region.blurData and computes intensity/freehandPoints), add a small validation
step for region.blurData.shape (e.g., check typeof === 'string' and that it
exists in the supported-shapes set or enum) and set shape =
supportedShapes.has(region.blurData.shape) ? region.blurData.shape :
DEFAULT_BLUR_DATA.shape so invalid strings from the project JSON are clamped to
a safe default.
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Line 728: The blur row is wired into the UI (Row with id BLUR_ROW_ID and hint
pressBlur) but the keyboard path is missing; update the keydown handler used for
add-item shortcuts (the same handler that currently invokes add actions for
other rows) to call handleAddBlur when the blur shortcut is pressed, and ensure
handleAddBlur remains in the effect dependency array; locate the Row usage with
id BLUR_ROW_ID and the surrounding keyboard/event effect where other add
handlers are invoked and add the branch that triggers handleAddBlur on the same
shortcut key check so blur works via keyboard like the others.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 667-674: handleSelectBlur fails to clear the selected speed id,
causing simultaneous selections; update the handleSelectBlur callback to also
call setSelectedSpeedId(null) when selecting a blur (i.e., inside the if (id)
branch where you already clear zoom/trim/annotation). Apply the same change to
the other handleSelectBlur occurrence referenced in the diff so selection
exclusivity is symmetric with handleSelectSpeed (which already clears blur).
In `@src/components/video-editor/VideoPlayback.tsx`:
- Around line 1398-1425: The click handler for blur overlays currently just
re-selects the clickedId (handleBlurClick) which prevents cycling through
overlapping blur regions; change handleBlurClick to mirror the annotation
click-through behavior: when a click occurs, if clickedId !== selectedBlurId
call onSelectBlur(clickedId), but if clickedId === selectedBlurId compute the
index of clickedId in the current sorted array (the same sorted variable used
for rendering) and call onSelectBlur with the next overlay's id (wrap to first
if at end) so repeated clicks cycle through overlapping AnnotationOverlay
instances; keep using the existing onSelectBlur callback and sorted/filtered
lists already in scope.
In `@src/i18n/locales/tr/timeline.json`:
- Line 8: The Turkish translation strings contain incorrect diacritics: replace
occurrences of "Bulanik" with the correct "Bulanık" (dotted ı) — e.g., update
the value for the "addBlur" key and any other entries that read "Aractan
bulanik" and standalone "Bulanik" to use "Bulanık" so all Turkish strings use
proper diacritics.
In `@src/i18n/locales/zh-CN/timeline.json`:
- Line 8: Replace the placeholder values for the i18n keys (including "addBlur"
and the two other entries currently set to "????"/"??????????") with proper
Simplified Chinese translations matching the English originals; locate the keys
in the zh-CN timeline JSON, replace the placeholder strings with accurate
translations (or copy the corresponding strings from the English timeline
locale), ensure the JSON remains valid (proper escaping/quoting) and run the
i18n/lint tests to confirm no missing keys or formatting errors.
---
Outside diff comments:
In `@src/i18n/locales/tr/settings.json`:
- Around line 101-121: Update the Turkish translations for the blur-related keys
to use proper diacritics: change the value for "typeBlur" from "Bulanik" to
"Bulanık", for "blurShape" from "Bulanik Sekli" to "Bulanık Şekli", and for
"blurShapeRectangle" from "Dikdortgen" to "Dikdörtgen" in
src/i18n/locales/tr/settings.json so the keys typeBlur, blurShape, and
blurShapeRectangle all use correct Turkish characters.
---
Nitpick comments:
In `@src/components/video-editor/AnnotationOverlay.tsx`:
- Around line 94-99: The freehand sampling threshold (currently hardcoded as
0.03) is too small and will produce excessive points; update the logic around
the point-adding check (the variables points, point, lastPoint and the
conditional Math.hypot(dx, dy) >= 0.03) to use a higher threshold (e.g., 0.5 or
1.0) or introduce a named constant like FREEHAND_POINT_THRESHOLD so it’s easy to
tune; adjust the comment accordingly to explain the unit (percent of container)
and ensure the new threshold reduces point density while preserving stroke
smoothness.
In `@src/lib/exporter/annotationRenderer.ts`:
- Around line 168-173: The code creates a new canvas and context
(tempCanvas/tempCtx) for every blur export which is expensive; instead add and
reuse a single scratch canvas/context (e.g., a module- or class-scoped
scratchCanvas and scratchCtx) and replace the per-call creation with resizing
the existing scratchCanvas to sw/sh, clearing it, then using
scratchCtx.drawImage(canvas, sx, sy, sw, sh, 0, 0, sw, sh). Ensure you handle
the case where getContext returns null, and avoid retaining stale dimensions by
updating width/height before drawing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3addd398-85d8-4bd1-981b-61a2b4241fe0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
src/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/BlurSettingsPanel.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/en/timeline.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/es/timeline.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/fr/timeline.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/tr/timeline.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-CN/timeline.jsonsrc/lib/exporter/annotationRenderer.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/video-editor/AnnotationOverlay.tsx (1)
86-99: distance threshold might be too small.
Math.hypot(dx, dy) >= 0.03in 0-100 normalized space means ~0.3px at 1000px container width. this will capture almost every pixel of movement, potentially creating very large point arrays during drawing.consider bumping to something like
1or2(1-2% of container) to reduce point density while maintaining smooth curves. alternatively, you could downsample points on commit if you want high-fidelity during drawing but compact storage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/AnnotationOverlay.tsx` around lines 86 - 99, The distance threshold in appendFreehandPoint is too small (Math.hypot(dx, dy) >= 0.03) which will record almost every pixel movement and bloat freehandPointsRef.current; increase the threshold to a more reasonable value (e.g., 1 or 2) to reduce point density, or implement a simple downsampling step when committing the stroke (e.g., filter points by minimum distance or run-length decimation) so appendFreehandPoint keeps strokes responsive while storage remains compact; update the comparison inside appendFreehandPoint accordingly and ensure downstream consumers of freehandPointsRef still work with the coarser point set.
🤖 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/timeline/TimelineEditor.tsx`:
- Around line 728-742: The Item in the BLUR_ROW_ID block is hardcoded with
variant="annotation" which overrides the intended variant
(TimelineRenderItem.variant / timelineItems variant: "blur"); update the Item
props in the Row mapping (the blurItems.map(...) block inside
TimelineEditor.tsx) to use the actual item's variant (e.g.,
variant={item.variant}) or, if annotation styling is intentional for blur, add
an inline comment explaining that choice; ensure you change only the Item's
variant prop in that map to preserve onSelectBlur, isSelected (selectedBlurId),
id and other existing props.
---
Nitpick comments:
In `@src/components/video-editor/AnnotationOverlay.tsx`:
- Around line 86-99: The distance threshold in appendFreehandPoint is too small
(Math.hypot(dx, dy) >= 0.03) which will record almost every pixel movement and
bloat freehandPointsRef.current; increase the threshold to a more reasonable
value (e.g., 1 or 2) to reduce point density, or implement a simple downsampling
step when committing the stroke (e.g., filter points by minimum distance or
run-length decimation) so appendFreehandPoint keeps strokes responsive while
storage remains compact; update the comparison inside appendFreehandPoint
accordingly and ensure downstream consumers of freehandPointsRef still work with
the coarser point set.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24e5e53a-e3c4-4aff-bcfc-249d6f47d9c1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
src/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/BlurSettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/timeline/TimelineEditor.tsxsrc/i18n/config.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/en/shortcuts.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/es/shortcuts.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/fr/shortcuts.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/tr/shortcuts.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-CN/shortcuts.jsonsrc/i18n/locales/zh-CN/timeline.jsonsrc/lib/shortcuts.ts
✅ Files skipped from review due to trivial changes (10)
- src/i18n/locales/es/shortcuts.json
- src/i18n/locales/en/shortcuts.json
- src/i18n/locales/tr/shortcuts.json
- src/i18n/locales/zh-CN/shortcuts.json
- src/i18n/locales/fr/shortcuts.json
- src/i18n/locales/fr/settings.json
- src/i18n/locales/tr/settings.json
- src/i18n/locales/en/settings.json
- src/i18n/locales/zh-CN/timeline.json
- src/i18n/locales/zh-CN/settings.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/i18n/locales/es/settings.json
- src/components/video-editor/projectPersistence.ts
- src/components/video-editor/BlurSettingsPanel.tsx
- src/components/video-editor/VideoEditor.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/exporter/annotationRenderer.ts (1)
191-193: nit: multiple clamp definitions could be consolidatedkinda cursed to have this function redefined 6+ times across the codebase. there's already a
mathUtils.tswith shared helpers—might be worth extractingclampthere or tosrc/lib/utils.tsto avoid accidental divergence down the road.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/exporter/annotationRenderer.ts` around lines 191 - 193, The clamp function is duplicated in multiple places (including this file's clamp function) — move the shared clamp implementation into the central utility module (e.g., mathUtils.ts or src/lib/utils.ts) and replace local definitions with imports; specifically, remove the local clamp in annotationRenderer.ts and import clamp from the chosen shared module, update any call sites in functions within annotationRenderer (and other files) to use the single exported clamp to avoid duplication and divergence.src/components/video-editor/timeline/TimelineEditor.tsx (1)
1316-1316: nit:blurRegionsin deps but unused in the effect body.unlike
annotationRegions(which is used for Tab cycling logic at line 1252),blurRegionsisn't actually referenced inside thehandleKeyDowncallback. it's harmless but kinda unnecessary - will cause extra effect re-runs when blur regions change.🧹 Proposed fix
], [ addKeyframe, handleAddZoom, handleAddTrim, handleAddAnnotation, handleAddBlur, handleAddSpeed, deleteSelectedKeyframe, deleteSelectedZoom, deleteSelectedTrim, deleteSelectedAnnotation, deleteSelectedBlur, deleteSelectedSpeed, selectedKeyframeId, selectedZoomId, selectedTrimId, selectedAnnotationId, selectedBlurId, selectedSpeedId, annotationRegions, - blurRegions, currentTime, onSelectAnnotation, keyShortcuts, isMac, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/timeline/TimelineEditor.tsx` at line 1316, The effect that defines handleKeyDown currently lists blurRegions in its dependency array but never references it in the effect body (unlike annotationRegions which is used for Tab cycling), causing unnecessary re-runs; remove blurRegions from the dependency list of the effect that declares handleKeyDown (leave annotationRegions and other actual dependencies intact) so the effect only re-runs when truly needed, or if blurRegions must be observed for some future logic, explicitly reference it or memoize it before including it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Line 1316: The effect that defines handleKeyDown currently lists blurRegions
in its dependency array but never references it in the effect body (unlike
annotationRegions which is used for Tab cycling), causing unnecessary re-runs;
remove blurRegions from the dependency list of the effect that declares
handleKeyDown (leave annotationRegions and other actual dependencies intact) so
the effect only re-runs when truly needed, or if blurRegions must be observed
for some future logic, explicitly reference it or memoize it before including
it.
In `@src/lib/exporter/annotationRenderer.ts`:
- Around line 191-193: The clamp function is duplicated in multiple places
(including this file's clamp function) — move the shared clamp implementation
into the central utility module (e.g., mathUtils.ts or src/lib/utils.ts) and
replace local definitions with imports; specifically, remove the local clamp in
annotationRenderer.ts and import clamp from the chosen shared module, update any
call sites in functions within annotationRenderer (and other files) to use the
single exported clamp to avoid duplication and divergence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96f5d938-9fcb-449d-a3c8-902d47f70f32
📒 Files selected for processing (5)
src/components/video-editor/AnnotationOverlay.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/tr/timeline.jsonsrc/lib/exporter/annotationRenderer.ts
✅ Files skipped from review due to trivial changes (2)
- src/i18n/locales/tr/settings.json
- src/i18n/locales/tr/timeline.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/video-editor/AnnotationOverlay.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
Author
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/video-editor/timeline/Item.tsx (1)
17-17: Makevariantrequired to avoid silent zoom fallbacksLine 17 now supports more modes, but keeping
variantoptional (plusvariant = "zoom"default on Line 48) is lowkey risky: a missing prop silently renders as zoom instead of failing fast.TimelineEditor.tsxcurrently passesvariantfor blur, so behavior is okay today, but this can hide future regressions.nit: cleaner fail-fast typing
interface ItemProps { id: string; span: Span; rowId: string; children: React.ReactNode; isSelected?: boolean; onSelect?: () => void; zoomDepth?: number; speedValue?: number; - variant?: "zoom" | "trim" | "annotation" | "speed" | "blur"; + variant: "zoom" | "trim" | "annotation" | "speed" | "blur"; } @@ export default function Item({ @@ - variant = "zoom", + variant, children, }: ItemProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/timeline/Item.tsx` at line 17, The prop type for the Item component should be made required to fail fast: remove the optional marker from the variant prop in the Item props/interface (change variant?: ... to variant: ...) and remove the inline default assignment (remove 'variant = "zoom"' from the Item function signature). Then update all call sites (e.g., TimelineEditor.tsx and any other places rendering <Item ... />) to explicitly pass a valid variant value ("zoom" | "trim" | "annotation" | "speed" | "blur") so the component no longer silently falls back to zoom.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/video-editor/timeline/Item.tsx`:
- Line 17: The prop type for the Item component should be made required to fail
fast: remove the optional marker from the variant prop in the Item
props/interface (change variant?: ... to variant: ...) and remove the inline
default assignment (remove 'variant = "zoom"' from the Item function signature).
Then update all call sites (e.g., TimelineEditor.tsx and any other places
rendering <Item ... />) to explicitly pass a valid variant value ("zoom" |
"trim" | "annotation" | "speed" | "blur") so the component no longer silently
falls back to zoom.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67f8852b-5f50-43c3-ae2e-aaa569be17eb
📒 Files selected for processing (2)
src/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/Item.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/video-editor/VideoEditor.tsx
LorenzoLancia
changed the title
Contributor
|
as said on discord can you add the keyboard shortcut like the rest? |
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
2 participants
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.
Description
This PR introduces the Blur Selection feature (#303), allowing users to select specific areas within the editor to be obscured (using rectangular or oval shapes). It is also possible to adjust the intensity of the blur applied.
Compared to the original feature request in #303, only these two selection types have been implemented so far.
In the future, the blur functionality could be further refined and expanded with additional features, as outlined in the original request.
Motivation
During screen recordings, it often happens that sensitive information—such as credentials or portions of code—may appear and should not be shown for privacy or security reasons. Being able to hide this content during the editing phase would make the workflow much simpler and safer.
Type of Change
Related Issue(s)
#303
Screenshots / Video
Video (if applicable):
feature_blur.2.1.mp4
Testing
I manually tested the correct functionality of the blur selection, including processing and layering across multiple levels.
Checklist
Thank you for contributing!
Summary by CodeRabbit
New Features
Bug Fixes