feat: add draggable crop region and aspect ratio select options#732
feat: add draggable crop region and aspect ratio select options#732sandeepannandi wants to merge 5 commits into
Conversation
… Properly, and Disappears During Recording
📝 WalkthroughWalkthroughThis PR adjusts HUD overlay recording behavior in Electron (mouse passthrough simplification, taskbar skip toggling) and locks HUD bar dragging via native mode during recording. Separately, CropControl is reworked to use ref-based drag state with aspect-ratio preset constraints and new pointer handles, plus a scrollbar-hiding style tweak in VideoEditor. ChangesHUD Overlay Recording Behavior
Crop Control Ratio Presets and Modal Styling
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/video-editor/CropControl.tsx (2)
149-162: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCapture the pointer during drag so it isn't dropped when the cursor leaves the container.
The drag handlers live on the container and
onPointerLeaveis wired tohandlePointerUp(Line 246). During a fast move/resize the pointer can exit the container bounds, which firespointerleaveand ends the drag prematurely; pointer motion outside the element is also not tracked. Capturing the pointer on the active handle keeps the interaction stable untilpointerup.♻️ Suggested change
const handlePointerDown = (e: React.PointerEvent, handle: DragHandle) => { e.stopPropagation(); e.preventDefault(); const rect = containerRef.current?.getBoundingClientRect(); if (!rect || rect.width <= 0 || rect.height <= 0) return; + e.currentTarget.setPointerCapture(e.pointerId); isDraggingRef.current = handle;Then drop
onPointerLeave={handlePointerUp}(or keep it only as a safety net), since capture makes it unnecessary and it can otherwise cut drags short.Also applies to: 244-246
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/CropControl.tsx` around lines 149 - 162, The drag in CropControl is ending too early when the pointer leaves the container because the active pointer is not being captured. Update handlePointerDown to capture the pointer on the pressed handle/target so move and up events continue outside the container, and release that capture in the corresponding pointer-up path in CropControl. Also remove or demote the onPointerLeave={handlePointerUp} wiring so it does not terminate an in-progress drag prematurely.
295-341: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResize handles are only 3px wide, making edge grabbing hard (and inaccessible).
The four resize strips use a fixed
3pxthickness with no larger invisible hit area, so precise edge grabbing is difficult with a mouse and effectively impossible on touch. Consider a wider transparent hit region (e.g. an inner visible 3px bar inside a ~12px pointer target).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/CropControl.tsx` around lines 295 - 341, The resize handles in CropControl are too thin because the top/bottom/left/right strips use only a 3px visible target, making pointer interaction hard. Update the handle rendering around the existing handle divs and handlePointerDown logic to add a larger transparent hit area (for example a wider absolute wrapper with the visible 3px bar centered inside it) so mouse and touch users can grab edges more easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/video-editor/CropControl.tsx`:
- Around line 149-162: The drag in CropControl is ending too early when the
pointer leaves the container because the active pointer is not being captured.
Update handlePointerDown to capture the pointer on the pressed handle/target so
move and up events continue outside the container, and release that capture in
the corresponding pointer-up path in CropControl. Also remove or demote the
onPointerLeave={handlePointerUp} wiring so it does not terminate an in-progress
drag prematurely.
- Around line 295-341: The resize handles in CropControl are too thin because
the top/bottom/left/right strips use only a 3px visible target, making pointer
interaction hard. Update the handle rendering around the existing handle divs
and handlePointerDown logic to add a larger transparent hit area (for example a
wider absolute wrapper with the visible 3px bar centered inside it) so mouse and
touch users can grab edges more easily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d9adcf0-9e5c-436f-ae45-9d36d7909279
📒 Files selected for processing (5)
electron/windows.tssrc/components/launch/LaunchWindow.tsxsrc/components/launch/hooks/useHudBarDrag.tssrc/components/video-editor/CropControl.tsxsrc/components/video-editor/VideoEditor.tsx
Changes
Summary by CodeRabbit
New Features
Bug Fixes