New Cropper across the app #1079
Conversation
…move haptics toggle
WalkthroughAdds a full-featured Cropper and integrates it across capture, editor, and overlay flows; removes legacy canvas/box utilities and CropAreaRenderer; removes the haptics setting and normalizes haptic enum casing; registers a SetCaptureAreaPending event/command and adds a typed webview emit shim; marks some windows non-resizable and adds a Close capability/button. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as CaptureArea/Editor/Overlay
participant Cropper
participant Persist as Persistence
participant Webview as MainWindow(WebView)
Note right of UI `#f9f9f9`: Cropper initialized with ref & initialCrop
User->>UI: open capture/editor UI
UI->>Cropper: init(ref, initialCrop, aspectRatio, snapToRatio)
User->>Cropper: pointer drag / resize / keyboard
Cropper-->>UI: onCropChange(bounds)
User->>UI: confirm
UI->>Persist: persist lastSelectedBounds[screenId]
UI->>Webview: emit setCaptureAreaPending(false)
Note over Cropper,UI: external control via ref (fill/reset/animate)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
apps/desktop/src/routes/capture-area.tsx (1)
97-98: Drop stray debug logThe
console.log("Hei")slipped in from debugging. Please remove it before shipping.apps/desktop/src/routes/editor/Editor.tsx (1)
322-343: Consider extracting menu positioning logic.The menu positioning logic in
showCropOptionsMenucould be reused across the app for consistent menu placement.Consider extracting this into a utility function:
// In a utils file: export async function showMenuBelowElement(menu: Menu, element: HTMLElement, yOffset = 40) { const rect = element.getBoundingClientRect(); const pos = new LogicalPosition(rect.x, rect.y + yOffset); await menu.popup(pos); await menu.close(); }apps/desktop/src/components/Cropper.tsx (1)
298-307: Add more descriptive error messages.The error messages could be more helpful by including the actual constraint values.
Enhance the error messages:
-throw new Error( - `Cropper error: minSize.x (${min.x}) cannot be greater than maxSize.x (${max.x}).`, -); +throw new Error( + `Cropper constraint error: minSize.x (${min.x}px) exceeds maxSize.x (${max.x}px). Please adjust the size constraints.`, +);apps/desktop/src/routes/target-select-overlay.tsx (1)
439-443: Consider extracting the menu positioning logic.This menu positioning pattern is repeated. Consider creating a shared utility.
Extract to a shared utility as suggested in the Editor.tsx review to maintain consistency across the application.
apps/desktop/src-tauri/src/windows.rs (1)
571-575: macOS: Verify NSPopUpMenuWindowLevel is correct for CaptureArea stackingSwitching to NSPopUpMenuWindowLevel should place the capture area above normal windows but below critical system UI. Please verify stacking with camera, overlay, and menus to ensure no regressions across Spaces/mission control.
If crate::platform::set_window_level expects a numeric CGWindowLevel, confirm that passing objc2_app_kit::NSPopUpMenuWindowLevel matches the expected type on your target toolchain. If needed, cast explicitly.
apps/desktop/src/routes/window-capture-occluder.tsx (1)
8-13: Deduplicate window focusing logicgetAllWindows().then(...setFocus()) is called twice. Extract into a small helper or run once (e.g., onMount) to avoid repeated invokes on reactive updates.
Also applies to: 29-35
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src-tauri/src/general_settings.rs(0 hunks)apps/desktop/src-tauri/src/platform/mod.rs(2 hunks)apps/desktop/src-tauri/src/windows.rs(2 hunks)apps/desktop/src/components/CapErrorBoundary.tsx(2 hunks)apps/desktop/src/components/CropAreaRenderer.tsx(0 hunks)apps/desktop/src/components/Cropper.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(0 hunks)apps/desktop/src/routes/capture-area.tsx(3 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(2 hunks)apps/desktop/src/routes/editor/Editor.tsx(6 hunks)apps/desktop/src/routes/target-select-overlay.tsx(7 hunks)apps/desktop/src/routes/window-capture-occluder.tsx(1 hunks)apps/desktop/src/utils/box.ts(0 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)packages/ui-solid/src/auto-imports.d.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/desktop/src/utils/box.ts
- apps/desktop/src/components/CropAreaRenderer.tsx
- apps/desktop/src/routes/(window-chrome)/settings/general.tsx
- apps/desktop/src-tauri/src/general_settings.rs
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/capture-area.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src-tauri/src/windows.rsapps/desktop/src/routes/(window-chrome)/(main).tsxpackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src-tauri/src/platform/mod.rsapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/capture-area.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/(main).tsxpackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/capture-area.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/(main).tsxpackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/capture-area.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/Editor.tsx
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file tauri.ts
Do not edit auto-generated files named
tauri.ts.
Files:
apps/desktop/src/utils/tauri.ts
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/windows.rsapps/desktop/src-tauri/src/platform/mod.rs
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:17:47.407Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.407Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Do not manually import icons in the desktop app; rely on auto-imported icons
Applied to files:
packages/ui-solid/src/auto-imports.d.ts
🧬 Code graph analysis (5)
apps/desktop/src/routes/target-select-overlay.tsx (3)
apps/desktop/src/components/Cropper.tsx (6)
CropBounds(21-26)CROP_ZERO(27-27)CropperRef(219-229)Ratio(74-74)createCropOptionsMenuItems(1476-1503)Cropper(231-1253)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(398-398)
apps/desktop/src/components/Cropper.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
commands(7-269)
apps/desktop/src/routes/capture-area.tsx (3)
apps/desktop/src/utils/tauri.ts (2)
DisplayId(368-368)LogicalPosition(398-398)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(21-26)CropperRef(219-229)CROP_ZERO(27-27)Ratio(74-74)createCropOptionsMenuItems(1476-1503)Cropper(231-1253)apps/desktop/src/utils/queries.ts (1)
createOptionsQuery(91-128)
apps/desktop/src/components/CapErrorBoundary.tsx (1)
packages/ui-solid/src/Button.tsx (1)
Button(40-50)
apps/desktop/src/routes/editor/Editor.tsx (4)
apps/desktop/src/components/Cropper.tsx (6)
CropperRef(219-229)CROP_ZERO(27-27)Ratio(74-74)createCropOptionsMenuItems(1476-1503)CropBounds(21-26)Cropper(231-1253)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(398-398)apps/desktop/src/utils/composeEventHandlers.ts (1)
composeEventHandlers(20-28)apps/desktop/src/routes/editor/ui.tsx (1)
EditorButton(353-418)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (23)
packages/ui-solid/src/auto-imports.d.ts (1)
71-92: Icon typings update looks consistentThe new Lucide icon globals line up alphabetically with the existing block and follow the established import typing convention. LGTM.
apps/desktop/src/routes/(window-chrome)/(main).tsx (1)
662-666: Verified matching emittersetCaptureAreaPending. Listener and emitter use the same channel; no action required.apps/desktop/src/routes/editor/Editor.tsx (3)
345-369: Good use of composition for NumberField inputs!The BoundInput component elegantly encapsulates the NumberField configuration and event handling, reducing duplication across the four input fields.
484-494: Verify crop bounds conversion consistency. Ensure bounds.x/y map correctly to position.x/y and bounds.width/height to size.x/y, matching the editor’s expected coordinate space.
2-2: Confirm NumberField import path The NumberField component exists in @kobalte/core v0.13 and the import from"@kobalte/core/number-field"is correct.apps/desktop/src/components/Cropper.tsx (8)
93-95: Platform-specific haptic feedback implementation looks good.The haptic feedback is correctly limited to macOS and uses the appropriate command interface.
175-187: Excellent handling of aspect ratio constraints!The code correctly calculates effective min/max sizes when an aspect ratio is enforced, preventing invalid size combinations.
267-273: Good animation frame management.The stopAnimation function properly cleans up animation frames and synchronizes display bounds.
595-626: Well-structured CropperRef implementation.The ref implementation provides a clean API surface with appropriate methods for external control. The conditional handling of function vs object refs is correct.
873-932: Comprehensive keyboard control implementation!The keyboard action loop correctly handles arrow key navigation with appropriate modifiers (Shift for larger steps, Meta/Ctrl for resizing, Alt for center-origin resize).
984-1011: Smart use of requestAnimationFrame for DOM updates.Using RAF for updating element styles prevents layout thrashing and ensures smooth visual updates. The cleanup of previous frame IDs prevents memory leaks.
1265-1338: Robust aspect ratio resize logic.The
computeAspectRatioResizefunction correctly handles aspect-constrained resizing with proper boundary checking and ratio maintenance.
1476-1503: Well-designed menu item creation utility.The
createCropOptionsMenuItemsfunction provides a clean interface for generating aspect ratio menu items with proper type safety.apps/desktop/src/routes/target-select-overlay.tsx (4)
292-292: Good platform-aware safe margin for macOS notch.The TOP_SAFE_MARGIN correctly accounts for the macOS notch with 40px margin while using 10px for other platforms.
294-331: Excellent controls positioning logic!The dynamic positioning algorithm intelligently places controls based on available space with clear priority: below > above > inside. The boundary checking prevents UI elements from being cut off.
248-271: Clean integration of crop options menu.The showCropOptionsMenu function properly integrates with the Cropper's menu system and handles the reset action appropriately.
355-364: Proper Cropper integration with all necessary callbacks.The Cropper is correctly configured with all required props and callbacks for the area selection mode.
apps/desktop/src-tauri/src/windows.rs (1)
543-543: Making CaptureArea non-resizable aligns with UX goalsSetting resizable(false) for the capture-area window matches the unified cropper behavior and prevents accidental window resizes. LGTM.
apps/desktop/src-tauri/src/platform/mod.rs (1)
13-14: Enum casing normalization via serde is correctAdding #[serde(rename_all = "camelCase")] aligns Rust enum variants with the updated TS string literals. This preserves TAURI invoke compatibility while keeping NS APIs fed with repr casts.
Confirm tauri-specta regenerated bindings (see tauri.ts) and there are no downstream string-case mismatches.
Also applies to: 23-24
apps/desktop/src/utils/tauri.ts (2)
378-378: GeneralSettingsStore type update acknowledgedhapticsEnabled removal is reflected here. Ensure any UI checks (like the Editor sidebar) no longer rely on it.
389-390: Haptic enums now lowercase — matches Rust serde camelCaseTypes switched to "alignment" | "levelChange" | "generic" and "default" | "now" | "drawCompleted". Ensure all call sites use lowercase (e.g., commands.performHapticFeedback("alignment","now")).
Also, please confirm this file is generated by tauri-specta and not manually edited, per repo policy. As per coding guidelines.
apps/desktop/src-tauri/capabilities/default.json (1)
27-27: Allowing resource close looks goodAdding core:resources:allow-close should unblock programmatic close flows (e.g., context menus). Validate on all windows where menus/resources are created to ensure no permission denials in logs.
apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
898-899: Ignore async fix:ostype()is synchronous in plugin-os v2
type()returns anOsTypestring (e.g."macos") synchronously, sohapticsEnabledis correctly computed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/routes/capture-area.tsx (1)
98-107: Use the validated bounds for saving and optionsYou validate
currentBoundsbut then read fromcrop()which can drift. Use the same snapshot for consistency.Apply this diff:
- const b = crop(); + const b = currentBounds; @@ - position: { x: b.x, y: b.y }, - size: { width: b.width, height: b.height }, + position: { x: b.x, y: b.y }, + size: { width: b.width, height: b.height },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/routes/capture-area.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/capture-area.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/capture-area.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/capture-area.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/capture-area.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/capture-area.tsx (3)
apps/desktop/src/utils/tauri.ts (2)
DisplayId(368-368)LogicalPosition(398-398)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(21-26)CropperRef(219-229)CROP_ZERO(27-27)Ratio(74-74)createCropOptionsMenuItems(1476-1503)Cropper(231-1253)apps/desktop/src/utils/queries.ts (1)
createOptionsQuery(91-128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (1)
apps/desktop/src/routes/capture-area.tsx (1)
59-66: Fix debounce misuse:createScheduledis invoked without a callback (Confirm can stay disabled)You’re calling
scheduled()with no callback and using its return as a boolean. That enqueues a noop and never re-computes validity. Port the debounce to wrap the validity computation and store it in a signal.Apply this diff:
-const scheduled = createScheduled((fn) => debounce(fn, 50)); - -const isValid = createMemo((p: boolean = true) => { - const b = crop(); - return scheduled() - ? b.width >= MIN_SIZE.width && b.height >= MIN_SIZE.height - : p; -}); +const scheduled = createScheduled((fn) => debounce(fn, 50)); +const [isValid, setIsValid] = createSignal(true); +createEffect(() => { + const b = crop(); + scheduled(() => { + setIsValid(b.width >= MIN_SIZE.width && b.height >= MIN_SIZE.height); + }); +});Also add the missing import:
-import { createMemo, createSignal, onCleanup, onMount, Show } from "solid-js"; +import { createEffect, createMemo, createSignal, onCleanup, onMount, Show } from "solid-js";
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/routes/window-capture-occluder.tsx (1)
8-13: Move focus side-effects out of render and dedupe callsCalling getAllWindows().setFocus() during render (twice) can cause flicker and unnecessary work. Run it once in onMount and remove the inner call.
-import { Show, Suspense } from "solid-js"; +import { Show, Suspense, onMount } from "solid-js"; @@ - getAllWindows().then((w) => - w.forEach((w) => { - if (w.label === "camera" || w.label === "in-progress-recording") - w.setFocus(); - }), - ); + onMount(() => { + getAllWindows().then((ws) => { + for (const w of ws) { + if (w.label === "camera" || w.label === "in-progress-recording") { + w.setFocus(); + } + } + }); + }); @@ - getAllWindows().then((w) => - w.forEach((w) => { - if (w.label === "camera" || w.label === "in-progress-recording") - w.setFocus(); - }), - ); + // focus handled in onMountAlso applies to: 29-34
🧹 Nitpick comments (7)
apps/desktop/src/routes/window-capture-occluder.tsx (1)
42-46: Remove inline comments per coding guidelinesInline JSX comments were added; the repository guidelines prohibit inline/block comments in TS/TSX/JS/JSX.
As per coding guidelines
- {/* Top blind */} @@ - {/* Bottom blind */} @@ - {/* Left blind */} @@ - {/* Right blind */}Also applies to: 47-51, 52-60, 61-69
apps/desktop/src/routes/editor/Editor.tsx (1)
354-361: Guard NumberField raw valueonRawValueChange can emit null/NaN during typing; setCropProperty expects a number. Guard to avoid propagating invalid values.
- onRawValueChange={(v) => { - cropperRef?.setCropProperty(props.field, v); - }} + onRawValueChange={(v) => { + if (typeof v === "number" && Number.isFinite(v)) { + cropperRef?.setCropProperty(props.field, v); + } + }}apps/desktop/src/components/Cropper.tsx (1)
112-116: Remove inline comments to conform with repo rulesNew inline comments were added; repo policy disallows inline/block/doc comments in TS/TSX/JS/JSX.
As per coding guidelines
Also applies to: 783-785, 1052-1056, 1063-1066, 1079-1087
apps/desktop/src/routes/target-select-overlay.tsx (4)
441-445: Use currentTarget and a generic Element cast for menu positioninge.target may be a child (e.g., SVG). Use currentTarget to position reliably.
- function showMenu(menu: Promise<Menu>, e: UIEvent) { + function showMenu(menu: Promise<Menu>, e: UIEvent) { e.stopPropagation(); - const rect = (e.target as HTMLDivElement).getBoundingClientRect(); + const rect = (e.currentTarget as Element).getBoundingClientRect(); menu.then((menu) => menu.popup(new LogicalPosition(rect.x, rect.y + 40))); }
247-249: Hide controls while interacting (state unused)shouldHideControls is set via onInteraction but not used. Apply it to opacity to avoid hover conflicts during drag/resize.
- class="fixed z-50 transition-opacity" + class="fixed z-50 transition-opacity" + classList={{ "opacity-0 pointer-events-none": shouldHideControls() }}Also applies to: 339-355
146-151: Remove inline comments per coding rulesInline comments were added; repo policy disallows them in TS/TSX/JS/JSX.
As per coding guidelines
Also applies to: 286-295
115-117: Keyboard default prevention is globalPreventing all keydown defaults on document can affect assistive tech and OS shortcuts. Consider scoping to relevant keys or element.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/components/Cropper.tsx(1 hunks)apps/desktop/src/routes/editor/Editor.tsx(6 hunks)apps/desktop/src/routes/target-select-overlay.tsx(6 hunks)apps/desktop/src/routes/window-capture-occluder.tsx(1 hunks)apps/desktop/src/utils/canvas.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/utils/canvas.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/window-capture-occluder.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/target-select-overlay.tsx
🧬 Code graph analysis (4)
apps/desktop/src/routes/window-capture-occluder.tsx (1)
crates/scap-targets/src/bounds.rs (4)
position(15-17)position(42-44)size(19-21)size(46-48)
apps/desktop/src/components/Cropper.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
commands(7-269)
apps/desktop/src/routes/editor/Editor.tsx (4)
apps/desktop/src/components/Cropper.tsx (6)
CropperRef(219-229)CROP_ZERO(27-27)Ratio(74-74)createCropOptionsMenuItems(1476-1503)CropBounds(21-26)Cropper(231-1253)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(398-398)apps/desktop/src/utils/composeEventHandlers.ts (1)
composeEventHandlers(20-28)apps/desktop/src/routes/editor/ui.tsx (1)
EditorButton(353-418)
apps/desktop/src/routes/target-select-overlay.tsx (3)
apps/desktop/src/components/Cropper.tsx (6)
CropBounds(21-26)CROP_ZERO(27-27)CropperRef(219-229)Ratio(74-74)createCropOptionsMenuItems(1476-1503)Cropper(231-1253)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(398-398)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (1)
apps/desktop/src/routes/editor/Editor.tsx (1)
304-308: Confirm display presence in first segmentAccess to firstSegment.display assumes it exists. If there are recordings without display, this throws.
Would you like a defensive guard with early return in the dialog branch?
| if (props.ref) { | ||
| const cropperRef: CropperRef = { | ||
| reset: () => { | ||
| const bounds = computeInitialBounds(); | ||
| setRawBoundsAndAnimate(bounds); | ||
| setAspectState("snapped", null); | ||
| }, | ||
| fill, | ||
| setCropProperty: (field, value) => { | ||
| setAspectState("snapped", null); | ||
| setRawBoundsConstraining( | ||
| boundsToRaw({ ...realBounds(), [field]: value }), | ||
| { x: 0, y: 0 }, | ||
| ); | ||
| }, | ||
| setCrop: (value, origin) => | ||
| setRawBoundsConstraining( | ||
| boundsToRaw( | ||
| typeof value === "function" ? value(rawBounds()) : value, | ||
| ), | ||
| origin, | ||
| ), | ||
| get bounds() { | ||
| return realBounds; | ||
| }, | ||
| animateTo: (real, durationMs) => | ||
| setRawBoundsAndAnimate(boundsToRaw(real), undefined, durationMs), | ||
| }; | ||
|
|
||
| if (typeof props.ref === "function") props.ref(cropperRef); | ||
| else props.ref = cropperRef; | ||
| } |
There was a problem hiding this comment.
Ref pattern is unsafe; avoid assigning to props
Assigning to props.ref won’t set the parent variable. Accept only a callback ref and invoke it.
- ref?: CropperRef | ((ref: CropperRef) => void);
+ ref?: (ref: CropperRef) => void;
@@
- if (props.ref) {
- const cropperRef: CropperRef = {
+ if (props.ref) {
+ const cropperRef: CropperRef = {
/* ... */
- };
-
- if (typeof props.ref === "function") props.ref(cropperRef);
- else props.ref = cropperRef;
+ };
+ props.ref(cropperRef);
}Follow up: update all callsites to pass a callback ref (see Editor and target-select-overlay). As per coding guidelines
Also applies to: 231-247
| let cropperRef: CropperRef | undefined; | ||
|
|
There was a problem hiding this comment.
Fix ref wiring for CropperRef (parent variable not assigned)
Same pattern as Editor: ref={cropperRef} won’t set the variable. Use a callback ref.
- let cropperRef: CropperRef | undefined;
+ let cropperRef: CropperRef | undefined;
@@
- <Cropper
- ref={cropperRef}
+ <Cropper
+ ref={(r) => (cropperRef = r)}Also applies to: 357-366
🤖 Prompt for AI Agents
In apps/desktop/src/routes/target-select-overlay.tsx around lines 242-243 (and
similarly at lines 357-366), the local variable declaration "let cropperRef:
CropperRef | undefined;" is never assigned because using JSX prop
ref={cropperRef} does not wire the ref; replace the current ref usage with a
callback ref that assigns the element to the parent variable (e.g., ref={el =>
(cropperRef = el)}), update both occurrences, and ensure type safety by
narrowing or asserting the assigned value to CropperRef so the parent variable
is populated for later use.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/components/Cropper.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/components/Cropper.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (1)
apps/desktop/src/components/Cropper.tsx (1)
233-234: Use callback refs instead of mutatingprops.refSolid props are immutable; reassigning
props.refis ignored, so consumers never receive the instance when they pass an object ref. Restrict the prop to a callback and invoke it directly to align with the framework expectations. As per coding guidelines.- ref?: CropperRef | ((ref: CropperRef) => void); + ref?: (ref: CropperRef) => void; @@ - if (props.ref) { - const cropperRef: CropperRef = { + if (props.ref) { + const cropperRef: CropperRef = { @@ - if (typeof props.ref === "function") props.ref(cropperRef); - else props.ref = cropperRef; + props.ref(cropperRef); }Also applies to: 592-623
| const shouldTriggerHaptic = ostype() === "macos"; | ||
| function triggerHaptic() { | ||
| if (shouldTriggerHaptic) commands.performHapticFeedback("alignment", null); | ||
| } |
There was a problem hiding this comment.
Fix async OS detection before gating haptics
ostype() from @tauri-apps/plugin-os returns a promise, so this equality check always yields false and haptics never fire on macOS. Resolve the promise once and gate the feedback on the resolved value.
-const shouldTriggerHaptic = ostype() === "macos";
-function triggerHaptic() {
- if (shouldTriggerHaptic) commands.performHapticFeedback("alignment", null);
-}
+let shouldTriggerHaptic = false;
+
+ostype()
+ .then((platform) => {
+ shouldTriggerHaptic = platform === "macos";
+ })
+ .catch(() => {
+ shouldTriggerHaptic = false;
+ });
+
+function triggerHaptic() {
+ if (!shouldTriggerHaptic) return;
+ void commands.performHapticFeedback("alignment", null);
+}📝 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.
| const shouldTriggerHaptic = ostype() === "macos"; | |
| function triggerHaptic() { | |
| if (shouldTriggerHaptic) commands.performHapticFeedback("alignment", null); | |
| } | |
| let shouldTriggerHaptic = false; | |
| ostype() | |
| .then((platform) => { | |
| shouldTriggerHaptic = platform === "macos"; | |
| }) | |
| .catch(() => { | |
| shouldTriggerHaptic = false; | |
| }); | |
| function triggerHaptic() { | |
| if (!shouldTriggerHaptic) return; | |
| void commands.performHapticFeedback("alignment", null); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/components/Cropper.tsx around lines 92-95, ostype() is
asynchronous so the current equality check always fails and haptics never fire;
fix by resolving the promise once and gating haptics on the resolved value
(e.g., on mount call ostype().then(value => set a local state/ref like isMac =
value === "macos" or store in a module-scoped variable, handle errors), then
change triggerHaptic to use that cached boolean (or make triggerHaptic async and
await ostype() if you prefer), ensuring you don't call ostype() repeatedly and
you handle promise rejection.
…ng after size constraint checks
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/Cropper.tsx(1 hunks)apps/desktop/src/routes/target-select-overlay.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/Cropper.tsx
🧬 Code graph analysis (2)
apps/desktop/src/routes/target-select-overlay.tsx (3)
apps/desktop/src/components/Cropper.tsx (6)
CropBounds(21-26)CROP_ZERO(27-27)CropperRef(216-226)Ratio(74-74)createCropOptionsMenuItems(1473-1500)Cropper(228-1250)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(398-398)
apps/desktop/src/components/Cropper.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
commands(7-269)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (5)
apps/desktop/src/components/Cropper.tsx (3)
92-95: Fix async OS detection before gating hapticsThe
ostype()from@tauri-apps/plugin-osreturns a promise but is being compared synchronously, so this equality check always yieldsfalseand haptics never fire on macOS. Resolve the promise once and gate the feedback on the resolved value.
201-214: Bug: width/height clamps are discardedThe
slideBoundsIntoContainerfunction clamps width and height into local variables but returns the original bounds object, losing those clamps.Apply this diff to fix the issue:
- return { ...bounds, x, y }; + return { ...bounds, x, y, width, height };
592-623: Ref pattern is unsafe; avoid assigning to propsAssigning to
props.refwon't set the parent variable. Accept only a callback ref and invoke it.Apply this diff:
- ref?: CropperRef | ((ref: CropperRef) => void); + ref?: (ref: CropperRef) => void; @@ - if (props.ref) { - const cropperRef: CropperRef = { + if (props.ref) { + const cropperRef: CropperRef = { /* ... */ - }; - - if (typeof props.ref === "function") props.ref(cropperRef); - else props.ref = cropperRef; + }; + props.ref(cropperRef); }apps/desktop/src/routes/target-select-overlay.tsx (2)
242-243: Fix ref wiring for CropperRef (parent variable not assigned)The ref assignment
ref={cropperRef}won't set the variable. Use a callback ref.Apply this diff:
- let cropperRef: CropperRef | undefined; + let cropperRef: CropperRef | undefined; @@ - <Cropper - ref={cropperRef} + <Cropper + ref={(r) => (cropperRef = r)}
489-490: Prevent duplicate menu popupsBoth onMouseDown and onClick trigger showMenu causing duplicate menu popups.
Apply this diff:
- onMouseDown={(e) => showMenu(menuModes(), e)} - onClick={(e) => showMenu(menuModes(), e)} + onClick={(e) => showMenu(menuModes(), e)} @@ - onMouseDown={(e) => showMenu(preRecordingMenu(), e)} - onClick={(e) => showMenu(preRecordingMenu(), e)} + onClick={(e) => showMenu(preRecordingMenu(), e)}Also applies to: 497-498
| checked: !options.aspect, | ||
| action: () => options.onAspectSet(null), | ||
| } satisfies CheckMenuItemOptions, | ||
| ...COMMON_RATIOS.map( | ||
| (ratio) => | ||
| ({ | ||
| text: `${ratio[0]}:${ratio[1]}`, | ||
| checked: options.aspect === ratio, |
There was a problem hiding this comment.
🧩 Analysis chain
Correct satisfies usage with proper TypeScript version
The satisfies operator was introduced in TypeScript v4.9. The current usage ratio === ratio in Line 1489 appears incorrect. It should compare the option's aspect ratio against the current ratio for the checked state.
Apply this diff to fix the comparison:
- checked: options.aspect === ratio,
+ checked: options.aspect?.[0] === ratio[0] && options.aspect?.[1] === ratio[1],🌐 Web query:
What is the latest stable version of TypeScript and does it support the satisfies keyword in version 4.9+?
💡 Result:
Latest stable TypeScript: v5.9.2 (released July 31, 2025). [1]
Yes — the satisfies operator was introduced in TypeScript 4.9 and is available in 4.9 and later. [2]
Sources:
- TypeScript GitHub releases (v5.9.2). [1]
- TypeScript 4.9 release notes — "The
satisfiesOperator". [2]
🏁 Script executed:
#!/bin/bash
# Locate TypeScript version in package.json
rg -n '"typescript"\s*:' -g 'package.json'Length of output: 484
🏁 Script executed:
#!/bin/bash
# Find definition of COMMON_RATIOS
rg -n "COMMON_RATIOS" -C3 --glob '*.ts*'Length of output: 1531
🏁 Script executed:
#!/bin/bash
# Find onAspectSet usage and definition context
rg -n "onAspectSet" -C5 --glob 'apps/desktop/src/components/Cropper.tsx'Length of output: 1807
Compare tuple values for checked state
options.aspect === ratio tests reference identity and won’t match tuples returned by findClosestRatio. Change to a value comparison:
- checked: options.aspect === ratio,
+ checked: options.aspect?.[0] === ratio[0] && options.aspect?.[1] === ratio[1],apps/desktop/src/components/Cropper.tsx:1489
📝 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.
| checked: !options.aspect, | |
| action: () => options.onAspectSet(null), | |
| } satisfies CheckMenuItemOptions, | |
| ...COMMON_RATIOS.map( | |
| (ratio) => | |
| ({ | |
| text: `${ratio[0]}:${ratio[1]}`, | |
| checked: options.aspect === ratio, | |
| ...COMMON_RATIOS.map( | |
| (ratio) => | |
| ({ | |
| text: `${ratio[0]}:${ratio[1]}`, | |
| checked: | |
| options.aspect?.[0] === ratio[0] && | |
| options.aspect?.[1] === ratio[1], | |
| action: () => options.onAspectSet(ratio), | |
| } satisfies CheckMenuItemOptions) | |
| ) |
🤖 Prompt for AI Agents
apps/desktop/src/components/Cropper.tsx lines ~1482-1489: the checked expression
uses reference equality (options.aspect === ratio) which fails for separately
created tuples; replace it with a value comparison that handles null/undefined,
e.g. check that Array.isArray(options.aspect) and compare elements
(options.aspect[0] === ratio[0] && options.aspect[1] === ratio[1]) or use a
small equality helper (or JSON.stringify) to compare the two tuples by value so
the correct menu item is marked checked.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
apps/desktop/src/components/Cropper.tsx (3)
92-95: Async ostype() comparison always fails—haptics never fire on macOS.
ostype()returnsPromise<string>, soostype() === "macos"always evaluates tofalseand haptic feedback never triggers. This issue was previously flagged but remains unresolved.Apply this diff to fix:
-const shouldTriggerHaptic = ostype() === "macos"; -function triggerHaptic() { - if (shouldTriggerHaptic) commands.performHapticFeedback("alignment", null); -} +let shouldTriggerHaptic = false; + +ostype() + .then((platform) => { + shouldTriggerHaptic = platform === "macos"; + }) + .catch(() => { + shouldTriggerHaptic = false; + }); + +function triggerHaptic() { + if (!shouldTriggerHaptic) return; + void commands.performHapticFeedback("alignment", null); +}
592-623: Ref pattern unsafe: assignment toprops.refwon't set parent variable.Line 622 assigns
props.ref = cropperRef, which mutates the prop but doesn't populate the caller's variable. Accept only a callback ref:- ref?: CropperRef | ((ref: CropperRef) => void); + ref?: (ref: CropperRef) => void; @@ if (props.ref) { const cropperRef: CropperRef = { /* ... */ }; - - if (typeof props.ref === "function") props.ref(cropperRef); - else props.ref = cropperRef; + props.ref(cropperRef); }Then update call sites (Editor, target-select-overlay) to pass callback refs like
ref={(r) => (cropperRef = r)}.
1495-1502: Tuple reference equality fails—checked state won't work.Line 1499 uses
options.aspect === ratio, which compares tuple references, not values. Separately created[1, 1]tuples never match.Apply this diff:
(ratio) => ({ text: `${ratio[0]}:${ratio[1]}`, - checked: options.aspect === ratio, + checked: options.aspect?.[0] === ratio[0] && options.aspect?.[1] === ratio[1], action: () => options.onAspectSet(ratio), }) satisfies CheckMenuItemOptions,apps/desktop/src/routes/target-select-overlay.tsx (3)
269-269: Ref wiring still unresolved despite past "addressed" status.Lines 269 and 384 use
ref={cropperRef}wherecropperRefisundefined, so the ref never populates. Past comments marked this addressed in commits d23ce3b–1e83773, but the current code still has the issue. Either the fix was incomplete or reverted.Apply this diff:
let cropperRef: CropperRef | undefined; @@ <Cropper - ref={cropperRef} + ref={(r) => (cropperRef = r)}This issue also affects the Cropper component itself (see my earlier comment on Cropper.tsx lines 592–623).
Also applies to: 384-384
315-320: Async ostype() still broken despite past "addressed" status.Line 315 synchronously compares
ostype() === "macos", which always returnsfalsebecauseostype()is a Promise. This breaks macOS-specific margins (lines 319–320) meant to avoid the notch. Past comments marked this addressed, but the current code still has the bug.Apply this diff:
+ let macos = false; + ostype() + .then((os) => { + macos = os === "macos"; + }) + .catch(() => { + macos = false; + }); + - const macos = ostype() === "macos"; const SIDE_MARGIN = 16; const MARGIN_BELOW = 16; const MARGIN_TOP_OUTSIDE = 16; - const MARGIN_TOP_INSIDE = macos ? 40 : 28; - const TOP_SAFE_MARGIN = macos ? 40 : 10; + const MARGIN_TOP_INSIDE = () => macos ? 40 : 28; + const TOP_SAFE_MARGIN = () => macos ? 40 : 10;Then update usages in
controlsStyleat lines 340, 344 to call the functions.
517-518: Duplicate event handlers still present—menu opens twice per click.Lines 517–518 and 525–526 both have
onMouseDownandonClickcallingshowMenu, despite past comments marking this addressed. Remove the duplicate handlers:<div class="pl-2.5 group-hover:bg-blue-10 transition-colors pr-3 py-1.5 flex items-center" onMouseDown={(e) => showMenu(menuModes(), e)} - onClick={(e) => showMenu(menuModes(), e)} > @@ <div class="flex justify-center items-center rounded-full border transition-opacity bg-gray-6 text-gray-12 size-9 hover:opacity-80" onMouseDown={(e) => showMenu(preRecordingMenu(), e)} - onClick={(e) => showMenu(preRecordingMenu(), e)} >Also applies to: 525-526
🧹 Nitpick comments (1)
apps/desktop/src/components/Cropper.tsx (1)
1360-1401: Alt-mode centering may drift when min/max constraints apply near edges.In Alt mode (center-origin resize), lines 1369–1372 compute symmetric expansion distances (
expLeft,expRight,expTop,expBottom), but min/max constraints (lines 1377–1384) apply to the final width/height without re-centering. When the crop is near an edge, the constrained box may not remain centered at the original point.Consider whether to re-center after applying constraints, or document that Alt mode prioritizes min/max constraints over strict centering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/Cropper.tsx(1 hunks)apps/desktop/src/routes/target-select-overlay.tsx(10 hunks)packages/ui-solid/src/auto-imports.d.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/components/Cropper.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use PascalCase for React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/target-select-overlay.tsx (4)
apps/desktop/src/utils/tauri.ts (3)
displayInformation(275-277)LogicalPosition(421-421)commands(7-287)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(21-26)CROP_ZERO(27-27)CropperRef(216-226)Ratio(74-74)createCropOptionsMenuItems(1483-1510)Cropper(228-1251)apps/desktop/src/routes/(window-chrome)/OptionsContext.tsx (1)
useRecordingOptions(9-16)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (10)
packages/ui-solid/src/auto-imports.d.ts (1)
73-73: LGTM! Auto-generated icon declarations are correctly formatted and actively used.The two new Lucide icon declarations (
IconLucideExpandandIconLucideX) are properly formatted, alphabetically sorted, and verified to be used in the codebase:
IconLucideExpandis used in the cropper's fill button (apps/desktop/src/routes/capture-area.tsx:199)IconLucideXis used for close/remove buttons in both the cropper (apps/desktop/src/routes/capture-area.tsx:180) and settings (apps/desktop/src/routes/(window-chrome)/settings/general.tsx:723)apps/desktop/src/components/Cropper.tsx (6)
201-214: Verify dimensions don't exceed container bounds before calling.If
width > containerWidthorheight > containerHeight, lines 210–211 can produce negativexory. Ensure all call sites constrain dimensions before sliding, or add clamping here:function slideBoundsIntoContainer( bounds: CropBounds, containerWidth: number, containerHeight: number, ): CropBounds { let { x, y, width, height } = bounds; + + width = Math.min(width, containerWidth); + height = Math.min(height, containerHeight); if (x < 0) x = 0; if (y < 0) y = 0; if (x + width > containerWidth) x = containerWidth - width; if (y + height > containerHeight) y = containerHeight - height; - return { ...bounds, x, y }; + return { ...bounds, x, y, width, height }; }
980-1009: Well-optimized DOM update batching.The
requestAnimationFramepattern with cleanup viaprevFrameIdis an excellent optimization that batches style updates and prevents memory leaks during rapid resize/drag operations. The comment clearly explains the rationale.
626-659: Drag logic is sound and properly cleaned up.The pointer drag handler correctly:
- Stops any active animation before starting the drag
- Uses
createRoot(dispose)for proper event listener cleanup- Clamps movement to container boundaries
- Updates both raw and display bounds
790-813: Double-click expansion logic correctly handles all directions.Each handle direction properly expands the selection to the corresponding edge while maintaining the opposite anchor point via
handle.origin. Corner handles expand in both dimensions as expected.
1308-1315: Verify fixed-ratio out-of-bounds behavior matches expectations.Lines 1308–1315 return
nullwhen bounds exceed the container, falling back torawBounds()in the caller. This stops resizing when the cursor goes offscreen, addressing the reported issue. However, afterconstrainBoundsToSize(lines 1318–1324) and container scaling (lines 1326–1335), bounds might still drift out of the original container check, then get slid back (line 1337), potentially breaking the aspect ratio.Test the following scenario to ensure the ratio is preserved and resizing stops cleanly:
- Set a fixed aspect ratio (e.g., 16:9)
- Drag a corner handle rapidly off-screen in various directions
- Verify the box doesn't resize from the opposite side or break the ratio
Based on PR comments, the author noted this edge case is tricky and was investigating workarounds.
871-978: Keyboard navigation well-structured with proper cleanup.The RAF-based loop efficiently handles continuous key presses, and modifier combinations (Shift for speed, Cmd/Ctrl for resize vs. move, Alt for center-origin) provide intuitive control. Cleanup correctly stops the loop when all keys are released.
apps/desktop/src/routes/target-select-overlay.tsx (3)
467-471: Menu positioning helper is concise and correctly placed below the trigger.The
showMenuhelper properly awaits the menu promise and positions it 40px below the trigger element usingLogicalPosition. Error handling for menu creation is omitted but acceptable for this UI flow.
322-359: Controls positioning logic handles edge cases well.The three-tier fallback (below → above → inside) with platform-specific margins is well-designed. Horizontal clamping ensures controls stay on-screen. Minor note: the off-screen translation at line 327 might briefly flash; consider
opacity: 0for smoother initial render.
275-298: Context menu implementation correctly awaits user interaction.The async flow—
popup()waits for dismissal, thenclose()cleans up—is correct. Showing at cursor position (no explicit coordinates) is appropriate for a context menu. UsingcreateCropOptionsMenuItemsmaintains consistency with the Cropper's public API.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src/routes/capture-area.tsx (1)
264-278: Fix Cropper ref wiring to avoid runtime errors
ref={cropperRef}never assigns the instance to the parent variable, socropperRefstaysundefinedandhandleConfirmimmediately throws “Cropper not initialized.” Switch to a callback ref so the CropperRef is captured.- <Cropper - ref={cropperRef} + <Cropper + ref={(instance) => { + cropperRef = instance; + }}apps/desktop/src/routes/target-select-overlay.tsx (1)
384-392: Wire the Cropper ref with a callbackHere too the plain
ref={cropperRef}never captures the instance, so every call tocropperRefremains a no-op (reset/fill/aspect toggles silently fail). Use a callback ref so the instance is stored.- <Cropper - ref={cropperRef} + <Cropper + ref={(instance) => { + cropperRef = instance; + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/components/Cropper.tsx(1 hunks)apps/desktop/src/routes/capture-area.tsx(3 hunks)apps/desktop/src/routes/target-select-overlay.tsx(13 hunks)apps/desktop/src/utils/events.ts(2 hunks)packages/ui-solid/src/auto-imports.d.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui-solid/src/auto-imports.d.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/utils/events.tsapps/desktop/src/routes/capture-area.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/utils/events.tsapps/desktop/src/routes/capture-area.tsxapps/desktop/src/components/Cropper.tsxapps/desktop/src/routes/target-select-overlay.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use PascalCase for React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
🧬 Code graph analysis (3)
apps/desktop/src/routes/capture-area.tsx (4)
apps/desktop/src/utils/tauriSpectaHack.ts (1)
emitTo(19-24)apps/desktop/src/utils/tauri.ts (2)
DisplayId(391-391)LogicalPosition(421-421)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(26-31)CropperRef(221-231)CROP_ZERO(32-32)Ratio(79-79)createCropOptionsMenuItems(1494-1521)Cropper(233-1267)apps/desktop/src/utils/queries.ts (1)
createOptionsQuery(115-155)
apps/desktop/src/components/Cropper.tsx (1)
apps/desktop/src/utils/events.ts (1)
createKeyDownSignal(16-32)
apps/desktop/src/routes/target-select-overlay.tsx (2)
apps/desktop/src/utils/tauri.ts (3)
displayInformation(275-277)LogicalPosition(421-421)commands(7-287)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(26-31)CROP_ZERO(32-32)CropperRef(221-231)Ratio(79-79)createCropOptionsMenuItems(1494-1521)Cropper(233-1267)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (2)
apps/desktop/src/utils/events.ts (2)
1-3: LGTM: Import updates support the new functionality.The import changes appropriately support the new
createKeyDownSignalfunction and the type constraint update increateQueryInvalidate.
6-6: Type change verified as compatible with all usages.The generic constraint change from
CreateQueryResulttoUseQueryResultis correct.createQueryfrom@tanstack/solid-queryreturnsUseQueryResult, and the single call site atapps/desktop/src/utils/queries.ts:160passes a compatible value. The function correctly uses the.refetch()method available onUseQueryResult.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/desktop/src/routes/target-select-overlay.tsx (3)
419-419: Verify ref wiring for cropperRef.The ref is assigned with
ref={cropperRef}which may not populate the parent-scope variable for component refs in SolidJS (unlike DOM refs). Line 293 callscropperRef?.reset(), suggesting the ref must be populated.If the ref isn't being populated, use a callback ref:
- <Cropper - ref={cropperRef} + <Cropper + ref={(r) => (cropperRef = r)}To verify whether the ref is working, run this script to check if Cropper component properly handles ref forwarding:
#!/bin/bash # Check if Cropper component uses ref forwarding patterns ast-grep --pattern $'export $_ Cropper($$$) { $$$ ref $$$ }'
555-556: Remove duplicate event handlers causing double menu popups (still unresolved).Both
onMouseDownandonClickcallshowMenu, which opens the menu twice per interaction. Keep only one handler.Apply this diff:
<div - class="pl-2.5 group-hover:bg-blue-10 transition-colors pr-3 py-1.5 flex items-center" + class="pl-2.5 group-hover:bg-blue-10 transition-colors pr-3 py-1.5 flex items-center" - onMouseDown={(e) => showMenu(menuModes(), e)} onClick={(e) => showMenu(menuModes(), e)} > @@ <div class="flex justify-center items-center rounded-full border transition-opacity bg-gray-6 text-gray-12 size-9 hover:opacity-80" - onMouseDown={(e) => showMenu(preRecordingMenu(), e)} onClick={(e) => showMenu(preRecordingMenu(), e)} >Also applies to: 563-564
328-333: Fix async ostype() synchronous comparison (still unresolved).
ostype()from@tauri-apps/plugin-osreturnsPromise<string>, but line 328 compares it synchronously. This will always befalse, causing the macOS safe-margin logic to fail (lines 332-333 will use non-macOS values even on macOS).Apply this diff to properly handle the async OS detection:
+ let isMacOS = false; + ostype() + .then((os) => { + isMacOS = os === "macos"; + }) + .catch(() => { + isMacOS = false; + }); + - const macos = ostype() === "macos"; const SIDE_MARGIN = 16; const MARGIN_BELOW = 16; const MARGIN_TOP_OUTSIDE = 16; - const MARGIN_TOP_INSIDE = macos ? 40 : 28; - const TOP_SAFE_MARGIN = macos ? 40 : 10; + const MARGIN_TOP_INSIDE = () => isMacOS ? 40 : 28; + const TOP_SAFE_MARGIN = () => isMacOS ? 40 : 10;Then update usage at lines 355, 360:
- if (aboveY >= TOP_SAFE_MARGIN) { + if (aboveY >= TOP_SAFE_MARGIN()) { finalY = aboveY; setControllerInside(false); } else { // Default to inside - finalY = bounds.y + MARGIN_TOP_INSIDE; + finalY = bounds.y + MARGIN_TOP_INSIDE(); setControllerInside(true); }
🧹 Nitpick comments (2)
apps/desktop/src/routes/target-select-overlay.tsx (2)
502-506: Add error handling to showMenu function.The
showMenufunction doesn't handle promise rejection. If menu creation or popup fails, it will silently fail, leaving the user with no feedback.Apply this diff to add error handling:
function showMenu(menu: Promise<Menu>, e: UIEvent) { e.stopPropagation(); const rect = (e.target as HTMLDivElement).getBoundingClientRect(); - menu.then((menu) => menu.popup(new LogicalPosition(rect.x, rect.y + 40))); + menu + .then((menu) => menu.popup(new LogicalPosition(rect.x, rect.y + 40))) + .catch((err) => { + console.error("Failed to show menu:", err); + }); }
287-310: Add error handling to showCropOptionsMenu.The async function doesn't handle errors from menu operations. Menu creation, popup, or close operations could fail.
Apply this diff to add error handling:
async function showCropOptionsMenu(e: UIEvent) { e.preventDefault(); - const items = [ - { - text: "Reset selection", - action: () => { - cropperRef?.reset(); - setAspect(null); - }, - }, - await PredefinedMenuItem.new({ - item: "Separator", - }), - ...createCropOptionsMenuItems({ - aspect: aspect(), - snapToRatioEnabled: snapToRatioEnabled(), - onAspectSet: setAspect, - onSnapToRatioSet: setSnapToRatioEnabled, - }), - ]; - const menu = await Menu.new({ items }); - await menu.popup(); - await menu.close(); + try { + const items = [ + { + text: "Reset selection", + action: () => { + cropperRef?.reset(); + setAspect(null); + }, + }, + await PredefinedMenuItem.new({ + item: "Separator", + }), + ...createCropOptionsMenuItems({ + aspect: aspect(), + snapToRatioEnabled: snapToRatioEnabled(), + onAspectSet: setAspect, + onSnapToRatioSet: setSnapToRatioEnabled, + }), + ]; + const menu = await Menu.new({ items }); + await menu.popup(); + await menu.close(); + } catch (err) { + console.error("Failed to show crop options menu:", err); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/routes/target-select-overlay.tsx(13 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/target-select-overlay.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use PascalCase for React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/target-select-overlay.tsx (4)
apps/desktop/src/utils/tauri.ts (3)
displayInformation(275-277)LogicalPosition(421-421)commands(7-287)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(26-31)CROP_ZERO(32-32)CropperRef(221-231)Ratio(79-79)createCropOptionsMenuItems(1494-1521)Cropper(233-1267)apps/desktop/src/routes/(window-chrome)/OptionsContext.tsx (1)
useRecordingOptions(9-16)apps/desktop/src/store.ts (1)
generalSettingsStore(61-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (2)
apps/desktop/src/routes/target-select-overlay.tsx (2)
280-285: Debounced validation pattern looks good.The
isValidmemo usescreateScheduledwith debouncing to prevent excessive validation during rapid crop changes. This is a good performance optimization.
337-376: Positioning logic is well-designed.The responsive positioning for recording controls is thoughtful, with fallback strategies for different screen configurations. Once the async
ostype()issue at line 328 is fixed, the macOS-specific spacing will work correctly.
|
This should be ready now!
This is now fixed!
I still have no idea how it occurred and I couldn't reproduce it.
It's been added back but adjust the look and wording as you see fit.
I spent over a day trying to figure out why animations kept breaking for a new indicator I added, nothing helped until I randomly came across an evil I was also looking at #1282 Althouth @ameer2468 said to me that there isn't yet a plan to make a change like this. I was thinking there could just be a floating menu (similar to the one on capture-area) to select the target mode. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/utils/events.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/utils/events.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/utils/events.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (2)
apps/desktop/src/utils/events.ts (2)
1-3: LGTM!The import updates correctly support the new
createKeyDownSignalutility and the type parameter change increateQueryInvalidate.
6-14: Type update is appropriate.The change from
CreateQueryResulttoUseQueryResultaligns with the library API, and the function logic remains correct.
| export function createKeyDownSignal( | ||
| element: HTMLElement | Window, | ||
| key: string, | ||
| ) { | ||
| const [isDown, setDown] = createSignal(false); | ||
|
|
||
| createEventListenerMap(element, { | ||
| keydown: (e) => { | ||
| if (e.key === key) setDown(true); | ||
| }, | ||
| keyup: (e) => { | ||
| if (e.key === key) setDown(false); | ||
| }, | ||
| blur: () => { | ||
| setDown(false); | ||
| }, | ||
| }); | ||
|
|
||
| return isDown; | ||
| } |
There was a problem hiding this comment.
Blur handler doesn't fully address the previous review concern.
The blur handler was added (lines 29-31), but the previous review specifically requested conditional logic to use "blur" for Window and "focusout" for HTMLElement. The current implementation uses "blur" for both element types, which may not work reliably for HTMLElement containers:
bluronHTMLElementdoesn't bubble, so it only fires if that specific element loses focus- If the element is a container with focusable children,
blurwon't fire when those children lose focus - This can leave
isDownstuck attrue, which was the original issue the past review aimed to prevent
Apply this diff to implement the conditional logic as requested:
export function createKeyDownSignal(
element: HTMLElement | Window,
key: string,
) {
const [isDown, setDown] = createSignal(false);
createEventListenerMap(element, {
keydown: (e) => {
if (e.key === key) setDown(true);
},
keyup: (e) => {
if (e.key === key) setDown(false);
},
- blur: () => {
+ [element instanceof Window ? "blur" : "focusout"]: () => {
setDown(false);
},
});
return isDown;
}🤖 Prompt for AI Agents
In apps/desktop/src/utils/events.ts around lines 16 to 35, the blur handler
currently always registers the "blur" event which doesn't bubble for
HTMLElements and can leave isDown stuck true; change the event registration so
that if element is Window you listen for "blur", otherwise (HTMLElement) listen
for "focusout" and attach a handler that calls setDown(false). Update
createEventListenerMap call to use this conditional event name (and keep the
keydown/keyup handlers unchanged).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/routes/target-select-overlay.tsx (1)
515-518: Add aria-labels to icon-only buttons for accessibility.The close button (lines 515-518) and settings button (lines 560-566) display only icons without text labels or aria-labels. Screen reader users won't be able to identify these buttons' purposes.
Add accessible labels:
<div onClick={() => { setOptions("targetMode", null); commands.closeTargetSelectOverlays(); }} + aria-label="Close target selector" class="flex justify-center items-center rounded-full transition-opacity bg-gray-12 size-9 hover:opacity-80" ><div class="flex justify-center items-center rounded-full border transition-opacity bg-gray-6 text-gray-12 size-9 hover:opacity-80" onMouseDown={(e) => showMenu(preRecordingMenu(), e)} onClick={(e) => showMenu(preRecordingMenu(), e)} + aria-label="Recording settings" >Also applies to: 560-566
🧹 Nitpick comments (1)
apps/desktop/src/routes/target-select-overlay.tsx (1)
501-505: Verify event target type before casting.Line 503 casts
e.targettoHTMLDivElementwithout verifying it's the expected element. If the event bubbles from a child element (e.g., the IconCapCaretDown or text spans),e.targetcould be a different element type, potentially causinggetBoundingClientRect()to return incorrect bounds.Use
currentTargetinstead, which always refers to the element the handler is attached to:function showMenu(menu: Promise<Menu>, e: UIEvent) { e.stopPropagation(); - const rect = (e.target as HTMLDivElement).getBoundingClientRect(); + const rect = (e.currentTarget as HTMLDivElement).getBoundingClientRect(); menu.then((menu) => menu.popup(new LogicalPosition(rect.x, rect.y + 40))); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/routes/target-select-overlay.tsx(14 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/target-select-overlay.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use PascalCase for React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/target-select-overlay.tsx (1)
apps/desktop/src/components/Cropper.tsx (6)
CropBounds(26-31)CROP_ZERO(32-32)CropperRef(221-231)Ratio(79-79)createCropOptionsMenuItems(1494-1521)Cropper(233-1267)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/routes/editor/Editor.tsx (1)
404-409: Drop the extraonMouseDowntrigger.Both
onMouseDownandonClickcallshowCropOptionsMenu, so activating the button opens two menus. RemoveonMouseDownand keeponClickonly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/routes/editor/Editor.tsx(7 hunks)apps/desktop/src/routes/editor/Player.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/editor/Player.tsxapps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/Player.tsxapps/desktop/src/routes/editor/Editor.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.
Applied to files:
apps/desktop/src/routes/editor/Editor.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/Editor.tsx (3)
apps/desktop/src/components/Cropper.tsx (6)
CropperRef(221-231)CROP_ZERO(32-32)Ratio(79-79)createCropOptionsMenuItems(1494-1521)CropBounds(26-31)Cropper(233-1267)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(418-418)apps/desktop/src/utils/composeEventHandlers.ts (1)
composeEventHandlers(20-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src/routes/editor/Editor.tsx (2)
458-478: Hook the Cropper ref with a callback.
cropperRefstaysundefinedbecause Solid only assigns refs via callback. As written, every call tocropperRef?.setCropProperty,fill(), orreset()silently no-ops, so the numeric inputs and buttons fail. Swap to a callback ref so the instance is captured.- <Cropper - ref={cropperRef} + <Cropper + ref={(instance) => { + cropperRef = instance; + }}
399-405: Remove duplicate menu trigger handlers.Both
onMouseDownandonClickcallshowCropOptionsMenu, so the menu fires twice per activation. Drop theonMouseDownhandler and keeponClickfor accessibility.- <Button + <Button variant="white" size="xs" class="flex items-center justify-center text-center rounded-full h-[2rem] w-[2rem] border focus:border-blue-9" - onMouseDown={showCropOptionsMenu} onClick={showCropOptionsMenu}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src-tauri/src/platform/mod.rs(2 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(1 hunks)apps/desktop/src/routes/editor/Editor.tsx(7 hunks)apps/desktop/src/utils/tauri.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src-tauri/src/platform/mod.rs
- apps/desktop/src-tauri/src/lib.rs
- apps/desktop/src/utils/tauri.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/routes/editor/Editor.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.
Applied to files:
apps/desktop/src/routes/editor/Editor.tsx
🧬 Code graph analysis (2)
apps/desktop/src/routes/(window-chrome)/(main).tsx (1)
apps/desktop/src/utils/tauri.ts (1)
events(297-341)
apps/desktop/src/routes/editor/Editor.tsx (4)
apps/desktop/src/components/Cropper.tsx (6)
CropperRef(221-231)CROP_ZERO(32-32)Ratio(79-79)createCropOptionsMenuItems(1494-1521)CropBounds(26-31)Cropper(233-1267)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(424-424)apps/desktop/src/utils/composeEventHandlers.ts (1)
composeEventHandlers(20-28)apps/desktop/src/routes/editor/ui.tsx (1)
EditorButton(353-418)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/(window-chrome)/(main).tsx
[failure] 644-644:
Parameter 'event' implicitly has an 'any' type.
[failure] 642-642:
Cannot find name 'events'. Did you mean 'event'?
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Clippy (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/routes/editor/Editor.tsx (1)
305-305: Add safety check for display access.The code accesses
editorInstance.recordings.segments[0].displaywithout verifying that the segments array has elements. Ifsegmentsis empty or the display property is missing, this will throw a runtime error.Add a guard to handle missing data:
{(dialog) => { const { setProject: setState, editorInstance } = useEditorContext(); + const segment = editorInstance.recordings.segments[0]; + if (!segment?.display) { + console.error("No display found in first segment"); + return null; + } - const display = editorInstance.recordings.segments[0].display; + const display = segment.display;
🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/Editor.tsx (1)
364-366: Simplify single handler to avoid unnecessary composition.Using
composeEventHandlerswith a single handler adds unnecessary overhead. The utility is designed for combining multiple handlers.Apply this simplification:
- onKeyDown={composeEventHandlers<HTMLInputElement>([ - (e) => e.stopPropagation(), - ])} + onKeyDown={(e) => e.stopPropagation()}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/routes/capture-area.tsx(3 hunks)apps/desktop/src/routes/editor/Editor.tsx(7 hunks)apps/desktop/src/routes/target-select-overlay.tsx(14 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/capture-area.tsxapps/desktop/src/routes/target-select-overlay.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/capture-area.tsxapps/desktop/src/routes/target-select-overlay.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.
Applied to files:
apps/desktop/src/routes/editor/Editor.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
apps/desktop/src/routes/target-select-overlay.tsx
🧬 Code graph analysis (3)
apps/desktop/src/routes/editor/Editor.tsx (4)
apps/desktop/src/components/Cropper.tsx (6)
CropperRef(221-231)CROP_ZERO(32-32)Ratio(79-79)createCropOptionsMenuItems(1494-1521)CropBounds(26-31)Cropper(233-1267)apps/desktop/src/utils/tauri.ts (1)
LogicalPosition(424-424)apps/desktop/src/utils/composeEventHandlers.ts (1)
composeEventHandlers(20-28)apps/desktop/src/routes/editor/ui.tsx (1)
EditorButton(353-418)
apps/desktop/src/routes/capture-area.tsx (4)
apps/desktop/src/utils/tauriSpectaHack.ts (1)
emitTo(19-24)apps/desktop/src/utils/tauri.ts (2)
DisplayId(394-394)LogicalPosition(424-424)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(26-31)CropperRef(221-231)CROP_ZERO(32-32)Ratio(79-79)createCropOptionsMenuItems(1494-1521)Cropper(233-1267)apps/desktop/src/utils/queries.ts (1)
createOptionsQuery(121-161)
apps/desktop/src/routes/target-select-overlay.tsx (2)
apps/desktop/src/utils/tauri.ts (3)
displayInformation(280-282)LogicalPosition(424-424)commands(7-292)apps/desktop/src/components/Cropper.tsx (6)
CropBounds(26-31)CROP_ZERO(32-32)CropperRef(221-231)Ratio(79-79)createCropOptionsMenuItems(1494-1521)Cropper(233-1267)
🔇 Additional comments (1)
apps/desktop/src/routes/editor/Editor.tsx (1)
404-405: Verify intentional dual event handlers for menu.Both
onMouseDownandonClickcallshowCropOptionsMenu, which can trigger the menu twice on a mouse click. A previous review comment (commit 41edf40) indicated this was addressed, but both handlers remain in the current code.If both handlers are intentional (e.g.,
onMouseDownfor mouse,onClickfor keyboard/touch), consider adding event checks or documentation. Otherwise, remove the duplicate:<Button variant="white" size="xs" class="flex items-center justify-center text-center rounded-full h-[2rem] w-[2rem] border focus:border-blue-9" - onMouseDown={showCropOptionsMenu} onClick={showCropOptionsMenu} >
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/desktop/src/components/Cropper.tsx (3)
1509-1509: Critical: Tuple reference equality always fails
options.aspect === ratiotests reference identity and won't match tuples with equal values. The checked state will always befalseeven when the aspect matches.Apply this diff to use value comparison:
({ text: `${ratio[0]}:${ratio[1]}`, - checked: options.aspect === ratio, + checked: options.aspect?.[0] === ratio[0] && options.aspect?.[1] === ratio[1], action: () => options.onAspectSet(ratio), }) satisfies CheckMenuItemOptions,
97-100: Critical: Fix async OS detection for haptics
ostype()returns aPromise<string>, so this equality check always evaluates tofalseand haptics never fire on macOS.Apply this diff to resolve the promise correctly:
-const shouldTriggerHaptic = ostype() === "macos"; -function triggerHaptic() { - if (shouldTriggerHaptic) commands.performHapticFeedback("alignment", null); -} +let shouldTriggerHaptic = false; + +ostype() + .then((platform) => { + shouldTriggerHaptic = platform === "macos"; + }) + .catch(() => { + shouldTriggerHaptic = false; + }); + +function triggerHaptic() { + if (!shouldTriggerHaptic) return; + void commands.performHapticFeedback("alignment", null); +}
604-635: Major: Unsafe ref pattern – assignment to props doesn't propagateAssigning directly to
props.ref(line 634) won't set the parent's variable. Props are read-only. Accept only a callback ref.Apply this diff:
ref?: CropperRef | ((ref: CropperRef) => void);Change to:
- ref?: CropperRef | ((ref: CropperRef) => void); + ref?: (ref: CropperRef) => void;And update the implementation:
if (props.ref) { const cropperRef: CropperRef = { /* ... */ }; - - if (typeof props.ref === "function") props.ref(cropperRef); - else props.ref = cropperRef; + props.ref(cropperRef); }Then update all callsites (Editor, target-select-overlay, etc.) to pass a callback ref.
🧹 Nitpick comments (3)
apps/desktop/src/components/Cropper.tsx (3)
41-41: Minor: Export Vec2 type for public API consistency
Vec2is used in the publicCropperRefAPI (lines 227, 230) but isn't exported. Consumers need this type to callsetCropandanimateTowith custom origin values.Apply this diff:
-type Vec2 = { x: number; y: number }; +export type Vec2 = { x: number; y: number };
944-944: Optional: Prefer immutable Set updates for clarityMutating the Set in place (
.add(),.delete()) and returning it works with Solid's store proxy, but creating a new Set is more idiomatic and explicit about the update.Consider this pattern:
- setKeyboardState("pressedKeys", (p) => p.add(e.key)); + setKeyboardState("pressedKeys", (p) => new Set(p).add(e.key));And:
- setKeyboardState("pressedKeys", (p) => { - p.delete(e.key); - return p; - }); + setKeyboardState("pressedKeys", (p) => { + const next = new Set(p); + next.delete(e.key); + return next; + });Also applies to: 966-969
1016-1016: Optional: Consider extracting altDown signal creationThe
altDownsignal is created at the component's render level. Consider moving it closer to where keyboard state is managed (near line 884) for better locality of related logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/Cropper.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/(main).tsx(3 hunks)apps/desktop/src/utils/tauriSpectaHack.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/utils/tauriSpectaHack.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/components/Cropper.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/(window-chrome)/(main).tsxapps/desktop/src/components/Cropper.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.
Applied to files:
apps/desktop/src/routes/(window-chrome)/(main).tsx
🧬 Code graph analysis (2)
apps/desktop/src/routes/(window-chrome)/(main).tsx (2)
apps/desktop/src/utils/createEventListener.ts (1)
createTauriEventListener(30-44)apps/desktop/src/utils/tauri.ts (1)
events(297-341)
apps/desktop/src/components/Cropper.tsx (2)
apps/desktop/src/utils/tauri.ts (1)
commands(7-292)apps/desktop/src/utils/events.ts (1)
createKeyDownSignal(16-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
- GitHub Check: Clippy (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (4)
apps/desktop/src/routes/(window-chrome)/(main).tsx (2)
36-36: LGTM! Imports are correct.The addition of
eventsandcreateTauriEventListenerimports properly supports the new event listening pattern. This addresses the previous critical issue regarding the missingeventsimport.Also applies to: 569-569
643-646: LGTM! Event listener implementation is clean and correct.The new
createTauriEventListenerutility simplifies event handling by:
- Automatically managing the unlisten promise lifecycle
- Handling cleanup registration internally
- Extracting the payload for the callback
This is a cleaner pattern than the previous manual promise and cleanup management.
apps/desktop/src/components/Cropper.tsx (2)
1278-1491: LGTM!The resize computation functions correctly handle:
- Aspect ratio constraints with corner/edge handle differentiation
- Alt-mode center-origin resizing
- Boundary clamping and size constraints
- Snapping to common ratios with haptic feedback
- Null safety when bounds exceed container
The logic is clear and well-structured.
302-316: LGTM!Good defensive programming – validating that minSize doesn't exceed maxSize and providing clear error messages.
6 participants
This RP removes the two previous cropper implementation and replaces them with a new, feature rich, more efficient unified cropper component.
The new cropper no longer uses a canvas and consumes much less resources in comparison.
Changes:
core:resources:allow-closefor properly closing context menus.With this, I tried having it resemble most of the functionality that is available in the cropper that's part of macOS's native Screenshot app.
Summary by CodeRabbit
New Features
Bug Fixes
Chores