Skip to content

feat: add draggable crop region and aspect ratio select options#732

Open
sandeepannandi wants to merge 5 commits into
webadderallorg:mainfrom
sandeepannandi:fix/crop-drag-aspect-ratios
Open

feat: add draggable crop region and aspect ratio select options#732
sandeepannandi wants to merge 5 commits into
webadderallorg:mainfrom
sandeepannandi:fix/crop-drag-aspect-ratios

Conversation

@sandeepannandi

@sandeepannandi sandeepannandi commented Jul 8, 2026

Copy link
Copy Markdown

Changes

  • Added Draggable Crop Area: Implemented a draggable center region on the crop box so users can now move the selected crop area around instead of only resizing it.
  • Added Aspect Ratio Options: Added preset buttons to easily lock the crop box into standard target ratios.
  • Hidden Dialog Scrollbars (UI): Visually hid the scrollbar track on the modal container while maintaining full overlay scroll functionality.

Summary by CodeRabbit

  • New Features

    • Added aspect-ratio crop presets in the video editor, with automatic resizing and re-centering for selected ratios.
    • Improved HUD drag behavior during recording for a more consistent on-screen experience.
  • Bug Fixes

    • Fixed HUD window taskbar visibility so it stays out of the taskbar while recording and returns afterward.
    • Kept the HUD aligned properly when recording starts, reducing unexpected position shifts.
    • Hid crop editor scrollbars while preserving scrolling.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This 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.

Changes

HUD Overlay Recording Behavior

Layer / File(s) Summary
HUD passthrough and taskbar state
electron/windows.ts
Recording branch of setHudOverlayMousePassthrough simplified to just re-enable mouse events; setHudOverlayRecordingActive now toggles setSkipTaskbar based on recording state.
HUD bar drag lock during recording
src/components/launch/hooks/useHudBarDrag.ts, src/components/launch/LaunchWindow.tsx
useHudBarDrag accepts a recording flag, resets and pins the HUD offset transform while recording; LaunchWindow passes recording into the hook and forces native drag mode during recording.

Crop Control Ratio Presets and Modal Styling

Layer / File(s) Summary
Ref-based drag and aspect-ratio constraints
src/components/video-editor/CropControl.tsx
Drag state moved from React state to refs; added CropRatioPreset types, constrainToRatio logic, and reworked pointer-driven move/resize handle overlays.
Crop modal scrollbar styling
src/components/video-editor/VideoEditor.tsx
Crop editor modal container hides scrollbars across browsers while preserving scroll functionality.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It only lists changes and omits required sections like Description, Motivation, Type of Change, Testing Guide, screenshots, and checklist. Add the template sections with purpose, motivation, change type, related issues, screenshots/video, testing steps, and checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main crop UI changes and is concise enough for history scanning.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/components/video-editor/CropControl.tsx (2)

149-162: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Capture the pointer during drag so it isn't dropped when the cursor leaves the container.

The drag handlers live on the container and onPointerLeave is wired to handlePointerUp (Line 246). During a fast move/resize the pointer can exit the container bounds, which fires pointerleave and ends the drag prematurely; pointer motion outside the element is also not tracked. Capturing the pointer on the active handle keeps the interaction stable until pointerup.

♻️ 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 win

Resize handles are only 3px wide, making edge grabbing hard (and inaccessible).

The four resize strips use a fixed 3px thickness 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

📥 Commits

Reviewing files that changed from the base of the PR and between 641d223 and d10eab6.

📒 Files selected for processing (5)
  • electron/windows.ts
  • src/components/launch/LaunchWindow.tsx
  • src/components/launch/hooks/useHudBarDrag.ts
  • src/components/video-editor/CropControl.tsx
  • src/components/video-editor/VideoEditor.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant