Multiple editor ux bits (text dragging, caption model downloading, etc) #1458
Conversation
|
Warning Rate limit exceeded@richiemcilroy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (9)
WalkthroughReplaces boolean caption bold with numeric font_weight, adds fadeDuration to text/mask segments, refactors TextOverlay/MaskOverlay to render multiple time-visible segments, moves Whisper model download progress emission from Window to AppHandle, and updates rendering to honor per-segment fade and opacity. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 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-tauri/src/captions.rs (1)
2086-2090: Log emit failures instead of silently discarding them with.ok()The
emit()method returnsResult<>(), which must be handled per the coding guidelines. Currently at line 2157, the error is discarded via.ok(), preventing visibility into repeated emit failures.Replace:
.emit(&app).ok();With proper error logging:
if let Err(error) = DownloadProgress { progress, message: format!("Downloading model: {progress:.1}%"), } .emit(&app) { tracing::warn!("Failed to emit DownloadProgress: {error}"); }This surfaces emit issues without breaking the download.
🧹 Nitpick comments (2)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
2635-2698: KSelect “Style” control should use an option from the options array as its valueThe Style select declares options for
{label, value}with weights 400/500/700, but passes a literal{ label: "Custom", value: props.segment.fontWeight }asvalue. Depending on KSelect’s API, this can meanstate.selectedOption()isundefinedeven when the font weight matches one of the predefined options, which may affect focus/selection semantics and accessibility.Since you already compute the label fallback based on
props.segment.fontWeight, a safer pattern is to derivevaluefrom the options list:const FONT_WEIGHT_OPTIONS = [ { label: "Normal", value: 400 }, { label: "Medium", value: 500 }, { label: "Bold", value: 700 }, ] as const; <KSelect options={FONT_WEIGHT_OPTIONS} optionValue="value" optionTextValue="label" value={ FONT_WEIGHT_OPTIONS.find(o => o.value === props.segment.fontWeight) ?? FONT_WEIGHT_OPTIONS[0] } // ... />This keeps KSelect’s notion of the selected option aligned with the actual weight while still letting you treat out‑of‑range weights as “Custom” in the label if desired.
2723-2736: Fade duration sliders for text and mask segments are wired consistentlyBoth the TextSegment and MaskSegment “Fade Duration” sliders bind to
segment.fadeDurationin the [0,1] range with a default of 0.15, matching the new Rust defaults and caption settings. The additionalclampNumberon the text slider helps sanitize any NaNs or out‑of‑range values; you might optionally apply the same clamp to the mask slider for symmetry, but it isn’t strictly necessary given the Slider’s own bounds.Also applies to: 2895-2907
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/desktop/src-tauri/src/captions.rs(5 hunks)apps/desktop/src/routes/editor/CaptionsTab.tsx(4 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(6 hunks)apps/desktop/src/routes/editor/MaskOverlay.tsx(5 hunks)apps/desktop/src/routes/editor/TextOverlay.tsx(3 hunks)apps/desktop/src/routes/editor/Timeline/TextTrack.tsx(2 hunks)apps/desktop/src/routes/editor/Timeline/Track.tsx(1 hunks)apps/desktop/src/routes/editor/masks.ts(2 hunks)apps/desktop/src/routes/editor/text.ts(2 hunks)apps/desktop/src/routes/screenshot-editor/AnnotationLayer.tsx(1 hunks)apps/desktop/src/store/captions.ts(2 hunks)apps/desktop/src/utils/tauri.ts(3 hunks)crates/project/src/configuration.rs(7 hunks)crates/rendering/src/layers/captions.rs(1 hunks)crates/rendering/src/layers/text.rs(1 hunks)crates/rendering/src/mask.rs(1 hunks)crates/rendering/src/text.rs(3 hunks)crates/video-decode/src/avassetreader.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations
Files:
apps/desktop/src/routes/editor/Timeline/TextTrack.tsxapps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/Timeline/Track.tsxapps/desktop/src/routes/editor/CaptionsTab.tsxapps/desktop/src/routes/screenshot-editor/AnnotationLayer.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/TextOverlay.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets
**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome usingpnpm format
Use Biome for code formatting and linting; runpnpm formatregularly
Use kebab-case for file names (e.g.,user-menu.tsx); use PascalCase for components
Files:
apps/desktop/src/routes/editor/Timeline/TextTrack.tsxapps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/text.tsapps/desktop/src/store/captions.tsapps/desktop/src/routes/editor/Timeline/Track.tsxapps/desktop/src/routes/editor/masks.tsapps/desktop/src/routes/editor/CaptionsTab.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/screenshot-editor/AnnotationLayer.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/TextOverlay.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention
Files:
apps/desktop/src/routes/editor/Timeline/TextTrack.tsxapps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/text.tsapps/desktop/src/store/captions.tsapps/desktop/src/routes/editor/Timeline/Track.tsxapps/desktop/src/routes/editor/masks.tsapps/desktop/src/routes/editor/CaptionsTab.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/screenshot-editor/AnnotationLayer.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/TextOverlay.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
Never add comments to code (
//,/* */,///,//!,#, etc.); code must be self-explanatory through naming, types, and structure
Files:
apps/desktop/src/routes/editor/Timeline/TextTrack.tsxcrates/rendering/src/layers/text.rsapps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/text.tscrates/rendering/src/mask.rsapps/desktop/src/store/captions.tsapps/desktop/src/routes/editor/Timeline/Track.tsxcrates/rendering/src/text.rsapps/desktop/src/routes/editor/masks.tsapps/desktop/src/routes/editor/CaptionsTab.tsxcrates/rendering/src/layers/captions.rsapps/desktop/src/utils/tauri.tscrates/project/src/configuration.rscrates/video-decode/src/avassetreader.rsapps/desktop/src/routes/screenshot-editor/AnnotationLayer.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/TextOverlay.tsxapps/desktop/src-tauri/src/captions.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never use dbg!() macro; use proper logging (tracing::debug!, etc.) instead
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them
Use duration.saturating_sub(other) instead of duration - other to avoid panics on underflow
Merge nested if statements: write 'if a && b { }' instead of 'if a { if b { } }'
Don't call .clone() on Copy types (integers, bools, etc.); copy them directly
Use function references directly: iter.map(foo) instead of iter.map(|x| foo(x))
Accept &[T] or &str instead of &Vec or &String in function parameters for flexibility
Use .is_empty() instead of .len() == 0 or .len() > 0 / .len() != 0
Don't assign () to a variable: write foo(); instead of let _ = foo(); or let x = foo(); when return is unit
Use .unwrap_or(val) instead of .unwrap_or_else(|| val) when the default is a simple/cheap value
Use 'for item in &collection' or 'for (i, item) in collection.iter().enumerate()' instead of 'for i in 0..collection.len()'
Use value.clamp(min, max) instead of manual if chains or .min(max).max(min) patterns
Always handle Result/Option or types marked #[must_use]; never ignore them
**/*.rs: Userustfmtand workspace clippy lints for Rust code formatting and linting
Use snake_case for Rust module names and kebab-case for crate names
Never usedbg!()macro in Rust code; use proper logging instead (Clippy:dbg_macro= deny)
Always handleResult/Optionor types marked#[must_use]; never ignore them (Rust compiler lint:unused_must_use= deny)
Never writelet _ = async_fn()which silently drops futures; await or explicitly handle them (Clippy:let_underscore_future= deny)
Usesaturating_subinstead of-forDurationto avoid panics (Clippy:unchecked_duration_subtraction= deny)
Merge nestedifstatements: useif a && b { }instead ofif a { if b { } }(Clippy:collapsible_if= deny)
Don't call.clone()onCopytypes; just copy them directly (Clippy:clone_on_copy= deny)
U...
Files:
crates/rendering/src/layers/text.rscrates/rendering/src/mask.rscrates/rendering/src/text.rscrates/rendering/src/layers/captions.rscrates/project/src/configuration.rscrates/video-decode/src/avassetreader.rsapps/desktop/src-tauri/src/captions.rs
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TanStack Query v5 for all client-side server state and data fetching in TypeScript files
Files:
apps/desktop/src/routes/editor/text.tsapps/desktop/src/store/captions.tsapps/desktop/src/routes/editor/masks.tsapps/desktop/src/utils/tauri.ts
apps/desktop/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.ts: Use @tanstack/solid-query for server state management in SolidJS components
Use generated commands and events from tauri_specta for IPC; never manually construct IPC calls
Listen directly to generated events from tauri_specta and use typed event interfaces
Files:
apps/desktop/src/routes/editor/text.tsapps/desktop/src/store/captions.tsapps/desktop/src/routes/editor/masks.tsapps/desktop/src/utils/tauri.ts
🧠 Learnings (6)
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Applied to files:
apps/desktop/src/routes/editor/Timeline/TextTrack.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Minimize useEffect usage in React components; compute during render, handle logic in event handlers, and ensure cleanups for subscriptions/timers
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:40.743Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T14:29:40.743Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Use `useEffectQuery`/`useEffectMutation` from `@/lib/EffectRuntime` in client components; do not call `EffectRuntime.run*` directly
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Listen directly to generated events from tauri_specta and use typed event interfaces
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Use tanstack/solid-query for server state management in SolidJS components
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-11-19T18:25:41.457Z
Learnt from: ItsEeleeya
Repo: CapSoftware/Cap PR: 1396
File: apps/desktop/src/routes/(window-chrome)/settings/general.tsx:183-192
Timestamp: 2025-11-19T18:25:41.457Z
Learning: In SolidJS, when using the Show component with the `keyed` attribute and a dynamic reactive value (like a memo result), the component will recreate its children when the condition value changes by reference. This pattern is useful for triggering CSS animations (like fade-in) when the rendered content needs to update. For example, in apps/desktop/src/routes/(window-chrome)/settings/general.tsx, `<Show when={previews()[theme.id]} keyed>` ensures the image re-animates when switching between different preview sources.
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
🧬 Code graph analysis (4)
apps/desktop/src/routes/editor/MaskOverlay.tsx (2)
apps/desktop/src/routes/editor/masks.ts (2)
MaskSegment(22-35)evaluateMask(61-76)apps/desktop/src/store/captions.ts (1)
updateSegment(66-75)
apps/desktop/src/utils/tauri.ts (2)
apps/desktop/src/routes/editor/masks.ts (3)
MaskSegment(22-35)MaskKind(3-3)MaskKeyframes(16-20)apps/desktop/src/routes/editor/text.ts (1)
TextSegment(3-16)
apps/desktop/src/routes/editor/TextOverlay.tsx (2)
apps/desktop/src/routes/editor/text.ts (1)
TextSegment(3-16)apps/desktop/src/utils/tauri.ts (1)
TextSegment(498-498)
apps/desktop/src-tauri/src/captions.rs (1)
apps/desktop/src-tauri/src/upload.rs (5)
app(712-713)app(794-795)stream(93-93)progress(404-414)progress(855-955)
⏰ 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 (aarch64-apple-darwin, macos-latest)
- GitHub Check: Clippy (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (32)
crates/video-decode/src/avassetreader.rs (1)
174-174: LGTM - Good formatting improvement.The change from positional to named parameter formatting (
{other:?}instead of{:?}, other) improves readability and is consistent with modern Rust formatting practices. The output remains identical, and this makes the format string more self-documenting.apps/desktop/src/routes/editor/text.ts (2)
15-15: LGTM: fadeDuration field addition is consistent with the broader PR changes.The new
fadeDurationproperty with a default of0.15aligns with the same pattern applied toMaskSegmentand caption settings across the codebase.Also applies to: 33-33
27-27: Verify the intent of the very small default size.The default size changed to
{ x: 0.01, y: 0.01 }(1% of viewport), which is significantly smaller than typical defaults. This may be intentional for auto-sizing behavior where actual dimensions are computed dynamically, but could result in nearly invisible text if used directly without adjustment.apps/desktop/src/utils/tauri.ts (1)
381-381: Generated file - types align with Rust backend changes.This file is auto-generated by tauri-specta. The new optional fields (
fontWeightinCaptionSettings,fadeDurationinMaskSegmentandTextSegment) correctly reflect the corresponding Rust type definitions.Also applies to: 439-439, 498-498
apps/desktop/src/routes/screenshot-editor/AnnotationLayer.tsx (2)
570-573: LGTM: Real-time text updates improve UX.The
onInputhandler correctly updates the annotation text in the store as the user types, using the proper SolidJS store update pattern.
582-589: LGTM: History snapshot logic is now more precise.The updated logic correctly avoids pushing duplicate history entries by comparing against the original text from
textSnapshot. This prevents unnecessary undo stack entries when the user opens and closes the text editor without making changes.crates/rendering/src/layers/captions.rs (1)
545-551: LGTM: Font weight logic follows CSS conventions.The numeric font weight thresholds (≥700 → BOLD, ≥500 → MEDIUM, else NORMAL) correctly align with standard CSS font-weight semantics and the UI options (400/500/700) provided in the editor.
apps/desktop/src/routes/editor/masks.ts (1)
33-33: LGTM: fadeDuration addition is consistent with TextSegment.The new
fadeDurationfield with a default of0.15matches the pattern established inTextSegmentand provides consistent fade behavior across mask and text overlays.Also applies to: 57-57
crates/rendering/src/text.rs (4)
4-4: LGTM! Font size cap is reasonable.The MAX_FONT_SIZE_PX constant provides a sensible upper bound to prevent excessively large font sizes that could cause rendering issues.
15-15: LGTM! Opacity field supports fade animation.The new opacity field correctly extends PreparedText to support per-segment fade animations.
75-86: LGTM! Fade logic is correctly implemented.The fade duration calculation properly handles both fade-in and fade-out transitions:
- Clamps fade_duration to non-negative values
- Computes time-based fade coefficients correctly
- Multiplies fade-in and fade-out for smooth transitions
- Defaults to full opacity when fade_duration is zero
93-94: LGTM! Font size calculation includes proper bounds.The font size calculation correctly applies scaling factors and clamps the result to a safe range [1.0, MAX_FONT_SIZE_PX].
crates/rendering/src/layers/text.rs (1)
56-61: LGTM! Alpha composition correctly combines color and opacity.The alpha calculation properly multiplies the text color's alpha channel with the fade opacity, ensuring both factors contribute to the final transparency. Both values are clamped to [0, 1] before multiplication, which is the correct approach.
crates/rendering/src/mask.rs (1)
86-95: LGTM! Mask fade logic is consistent with text rendering.The fade duration handling for masks follows the same pattern as the text fade implementation, ensuring consistent behavior across mask and text overlays. The per-segment fade_duration replaces the previous hardcoded value, providing greater flexibility.
apps/desktop/src/routes/editor/Timeline/Track.tsx (3)
116-116: LGTM! Lower threshold improves usability for smaller segments.Reducing the hidden threshold from 80 to 40 pixels allows resize handles to appear on smaller segments, improving the editing experience for shorter timeline items.
122-126: LGTM! Handle styling improvements enhance visual consistency.The updated styling with fixed width (w-4 min-w-4), z-index positioning, and centered alignment with margin adjustments provides better visual consistency and layering for segment handles.
129-129: LGTM! Data attribute aids testing and debugging.The data-hidden attribute provides a useful hook for automated testing and debugging without affecting the component's functionality.
apps/desktop/src/routes/editor/Timeline/TextTrack.tsx (1)
291-291: LGTM! Layout improvements ensure proper text truncation.The additions of overflow-hidden, min-w-0, and w-full follow Tailwind best practices for text truncation within flex containers. The use of min-w-0 is particularly important as it allows flex children to shrink below their content size, enabling proper truncation. The change from max-w-[10rem] to max-w-full provides more flexible text display.
Also applies to: 328-331
apps/desktop/src/store/captions.ts (2)
25-26: LGTM! Default settings align with styling refactor.The addition of fontWeight: 700 (bold) as a default and the change of outline to false are consistent with the broader UI styling improvements in this PR.
142-142: LGTM! Font weight properly serialized.Including fontWeight in the saved caption settings ensures the new styling option is correctly persisted alongside other caption preferences.
apps/desktop/src/routes/editor/CaptionsTab.tsx (2)
278-297: LGTM! Deferred effects prevent unnecessary initial writes.Using
defer: truein the on() options correctly delays effect execution until dependencies actually change, avoiding unnecessary localStorage writes during initial component mount. This is a good practice for side-effect management.
792-857: Font Weight control implemented consistently.The Font Weight control follows the established KSelect pattern used elsewhere in this file with proper option handling, disabled state, and dropdown animations.
One minor observation: Line 802 sets the value to
{ label: "Custom", value: getSetting("fontWeight") }, but the display logic in lines 829-838 correctly maps the weight value back to the appropriate label (Normal/Medium/Bold). The "Custom" label is effectively a placeholder that gets replaced during rendering, which works but could be slightly clearer.apps/desktop/src/routes/editor/MaskOverlay.tsx (3)
17-26: LGTM! Multi-segment rendering enables better mask editing workflow.The refactoring from single-mask to multi-segment rendering provides a more flexible editing experience:
- Time-based filtering shows only currently visible masks
- For loop correctly iterates visible segments
- Show component appropriately renders selected vs unselected states
- Fallback placeholder with hover styling improves discoverability
The implementation follows SolidJS best practices with proper reactivity.
Also applies to: 92-123
28-40: LGTM! Selection handlers properly manage mask interaction.The selection logic correctly:
- Derives selected mask from timeline selection state
- Handles segment selection with proper event handling (preventDefault, stopPropagation)
- Clears selection when clicking the background
- Computes mask rectangles from evaluated state
The handlers follow established patterns and maintain proper event flow control.
Also applies to: 58-84
103-103: LGTM! Gray color scheme provides more neutral mask visualization.The styling updates from blue to gray tones create a more subtle visual treatment for mask overlays and handles, aligning with the broader design system improvements in this PR.
Also applies to: 235-235, 281-281
crates/project/src/configuration.rs (2)
589-611: Mask/Text segment fade duration defaults look consistent and backward‑compatibleAdding
fade_duration: f64toMaskSegmentandTextSegmentwith serde defaults of0.15matches the new UI sliders (0–1s, default 0.15) and preserves backwards compatibility for older projects that lack this field. No functional issues spotted here.Also applies to: 627-690
778-866: CaptionSettings.font_weight and updated defaults are coherent with parsing/UIThe new
font_weight: u32field withdefault_font_weight() -> 700plus the updatedDefaultimpl (settingfont_weightand makingoutlinefalse) lines up withparse_captions_jsonand the captions UI, which both assume a 700 default and outline disabled by default. The serde alias and rename_all keep JSON shape stable.apps/desktop/src/routes/editor/TextOverlay.tsx (3)
21-63: Visible-segment selection, normalization, and update plumbing look solidThe refactor to derive
visibleTextSegmentsfrom current time, trackselectedTextIndex, and update segments viaupdateSegmentByIndexwithproduceis clean and defensive (null‑checking the segment). TheSegmentWithDefaults/normalizeSegmentlayer provides consistent defaults from the Tauri shape without leaking optionality into the overlay, which should make the drag/resize logic more robust.Also applies to: 145-173
175-233: TextSegmentOverlay measurement and drag/resize behavior is correct and well-boundedThe hidden measurement element plus
measureAndUpdateSize(with guards for zero dimensions and container size) gives a good auto‑sizing path for segments without reliable size, and the effects watchingmounted, container size, and text/font properties are careful to avoid measuring while resizing.Drag (
onMove) and resize (createResizeHandler) handlers correctly normalize deltas into [0,1] space, enforce minimum size and padding, clamp centers within bounds, and scale bothfontSizeandsizeon corner drags in a numerically stable way (using the prior font size as the baseline). I don’t see correctness issues here.Also applies to: 285-430, 432-513
102-141: The pointer-events pattern is correctly implementedThe TextSegmentOverlay component applies
pointer-events-auto(line 458), which explicitly overrides the parent'spointer-events-none. This is a valid CSS pattern: child elements with pointer-events explicitly set can be targets of pointer events even when the parent has pointer-events: none. The overlay will receive mouse events and fire onSelect correctly regardless of whether text is selected.Likely an incorrect or invalid review comment.
apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
2517-2538: HexColorInput swatch + color input integration looks correctThe new swatch/button with an overlaid
<input type="color">keeps the internaltextstate and the visual color in sync, and callsonChangeonly with valid hex values coming from the color input or normalized text on blur. No issues from a behavior standpoint.
2613-2627: Font size slider scaling segment.size is numerically soundUsing
newFontSize / (segment.fontSize || 48)as the scale factor and then multiplyingsegment.sizeby that factor (capped at 0.95) gives predictable proportional resizing as users drag the slider. Guarding onsegment.size.x/y > 0.025avoids blowing up default/placeholder sizes. This looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/captions.rs (1)
2092-2162: Streaming Whisper model download via AppHandle looks good; consider handling emit failuresThe refactored
download_whisper_model:
- Uses
AppHandle+http_client::HttpClientto stream the response body into atokio::fs::File.- Tracks
downloadedbytes to compute percentage whencontent_lengthis available.- Emits typed
DownloadProgressevents derived viatauri_specta::Event, which is a nice improvement over the old stringly-typed window event.Two minor suggestions:
.emit(&app).ok();silently discards any emission error. It might be worth at least logging failures (at debug/info) so they’re visible during debugging, instead of fully ignoring the Result.- Optionally, you could emit a final
DownloadProgresswithprogress: 100.0after a successful flush to guarantee the frontend always sees a completion state, even ifcontent_lengthwas missing or slightly off.apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
2635-2683: KSelect “Style” control should use an option from the list forvalueto keep selection highlighting in syncRight now
KSelectis always givenvalue={{ label: "Custom", value: props.segment.fontWeight }}, which isn’t one of theoptions. That meansstate.selectedOption()will never match an entry, so the menu won’t highlight/check the currently active style even whenfontWeightis exactly 400/500/700.You already compute the matching option in the
Valuerender; you can reuse that forvalueso the control behaves like the other KSelects in this file:- <KSelect - options={[ - { label: "Normal", value: 400 }, - { label: "Medium", value: 500 }, - { label: "Bold", value: 700 }, - ]} + const styleOptions = [ + { label: "Normal", value: 400 }, + { label: "Medium", value: 500 }, + { label: "Bold", value: 700 }, + ] as const + + <KSelect + options={styleOptions} optionValue="value" optionTextValue="label" - value={{ - label: "Custom", - value: props.segment.fontWeight, - }} + value={ + styleOptions.find( + (option) => option.value === props.segment.fontWeight, + ) ?? null + }and inside the
Valuerender you can then rely onselectedOption()for the standard cases and only fall back toCustom (...)when no matching option is found.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src-tauri/src/captions.rs(6 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(6 hunks)apps/desktop/src/routes/editor/MaskOverlay.tsx(6 hunks)apps/desktop/src/routes/editor/Player.tsx(0 hunks)apps/desktop/src/routes/editor/TextOverlay.tsx(3 hunks)apps/desktop/src/routes/editor/text.ts(2 hunks)apps/desktop/src/store/captions.ts(2 hunks)crates/project/src/configuration.rs(7 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/routes/editor/Player.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/store/captions.ts
- apps/desktop/src/routes/editor/text.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never use dbg!() macro; use proper logging (tracing::debug!, etc.) instead
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them
Use duration.saturating_sub(other) instead of duration - other to avoid panics on underflow
Merge nested if statements: write 'if a && b { }' instead of 'if a { if b { } }'
Don't call .clone() on Copy types (integers, bools, etc.); copy them directly
Use function references directly: iter.map(foo) instead of iter.map(|x| foo(x))
Accept &[T] or &str instead of &Vec or &String in function parameters for flexibility
Use .is_empty() instead of .len() == 0 or .len() > 0 / .len() != 0
Don't assign () to a variable: write foo(); instead of let _ = foo(); or let x = foo(); when return is unit
Use .unwrap_or(val) instead of .unwrap_or_else(|| val) when the default is a simple/cheap value
Use 'for item in &collection' or 'for (i, item) in collection.iter().enumerate()' instead of 'for i in 0..collection.len()'
Use value.clamp(min, max) instead of manual if chains or .min(max).max(min) patterns
Always handle Result/Option or types marked #[must_use]; never ignore them
**/*.rs: Userustfmtand workspace clippy lints for Rust code formatting and linting
Use snake_case for Rust module names and kebab-case for crate names
Never usedbg!()macro in Rust code; use proper logging instead (Clippy:dbg_macro= deny)
Always handleResult/Optionor types marked#[must_use]; never ignore them (Rust compiler lint:unused_must_use= deny)
Never writelet _ = async_fn()which silently drops futures; await or explicitly handle them (Clippy:let_underscore_future= deny)
Usesaturating_subinstead of-forDurationto avoid panics (Clippy:unchecked_duration_subtraction= deny)
Merge nestedifstatements: useif a && b { }instead ofif a { if b { } }(Clippy:collapsible_if= deny)
Don't call.clone()onCopytypes; just copy them directly (Clippy:clone_on_copy= deny)
U...
Files:
crates/project/src/configuration.rsapps/desktop/src-tauri/src/captions.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
Never add comments to code (
//,/* */,///,//!,#, etc.); code must be self-explanatory through naming, types, and structure
Files:
crates/project/src/configuration.rsapps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/TextOverlay.tsxapps/desktop/src-tauri/src/captions.rsapps/desktop/src/routes/editor/ConfigSidebar.tsx
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations
Files:
apps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/TextOverlay.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets
**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome usingpnpm format
Use Biome for code formatting and linting; runpnpm formatregularly
Use kebab-case for file names (e.g.,user-menu.tsx); use PascalCase for components
Files:
apps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/TextOverlay.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention
Files:
apps/desktop/src/routes/editor/MaskOverlay.tsxapps/desktop/src/routes/editor/TextOverlay.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Minimize useEffect usage in React components; compute during render, handle logic in event handlers, and ensure cleanups for subscriptions/timers
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:40.743Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T14:29:40.743Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Use `useEffectQuery`/`useEffectMutation` from `@/lib/EffectRuntime` in client components; do not call `EffectRuntime.run*` directly
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Listen directly to generated events from tauri_specta and use typed event interfaces
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Use tanstack/solid-query for server state management in SolidJS components
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-11-19T18:25:41.457Z
Learnt from: ItsEeleeya
Repo: CapSoftware/Cap PR: 1396
File: apps/desktop/src/routes/(window-chrome)/settings/general.tsx:183-192
Timestamp: 2025-11-19T18:25:41.457Z
Learning: In SolidJS, when using the Show component with the `keyed` attribute and a dynamic reactive value (like a memo result), the component will recreate its children when the condition value changes by reference. This pattern is useful for triggering CSS animations (like fade-in) when the rendered content needs to update. For example, in apps/desktop/src/routes/(window-chrome)/settings/general.tsx, `<Show when={previews()[theme.id]} keyed>` ensures the image re-animates when switching between different preview sources.
Applied to files:
apps/desktop/src/routes/editor/TextOverlay.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Applied to files:
apps/desktop/src/routes/editor/ConfigSidebar.tsx
🧬 Code graph analysis (3)
apps/desktop/src/routes/editor/MaskOverlay.tsx (3)
apps/desktop/src/routes/editor/masks.ts (2)
MaskSegment(22-35)evaluateMask(61-76)apps/desktop/src/utils/tauri.ts (1)
MaskSegment(439-439)apps/desktop/src/store/captions.ts (1)
updateSegment(65-74)
apps/desktop/src-tauri/src/captions.rs (2)
apps/desktop/src-tauri/src/upload.rs (4)
app(712-713)app(794-795)progress(404-414)progress(855-955)apps/desktop/src/utils/tauri.ts (1)
DownloadProgress(404-404)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
apps/desktop/src/routes/editor/ui.tsx (3)
Field(25-47)MenuItem(266-281)Slider(65-149)apps/desktop/src/store/captions.ts (1)
updateSegment(65-74)
⏰ 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). (4)
- GitHub Check: Clippy (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
crates/project/src/configuration.rs (2)
591-610: Mask/Text segment fade_duration defaults look consistent and safeAdding
fade_duration: f64with serde defaults of0.15for bothMaskSegmentandTextSegmentmatches the frontend sliders (0–1s) and preserves backward compatibility for existing projects via#[serde(default = "...::default_fade_duration")]. No issues from a type/serialization standpoint.Also applies to: 613-625, 629-652, 654-690
780-809: CaptionSettings font_weight/fadeDuration wiring is coherent with the rest of the modelIntroducing
font_weight: u32with alias"fontWeight"anddefault_font_weight() -> 700, plus the newdefault_fade_duration() -> 0.15and Default impl updates, all align with the editor UI and TypeScript types. The aliasing/backwards-compatibility story looks correct, and changingoutlineto defaultfalseis a clear, explicit choice.Also applies to: 821-833, 843-864
apps/desktop/src-tauri/src/captions.rs (2)
1784-1821: Caption settings now persist fontWeight and timing fields correctlyIncluding
"fontWeight","fadeDuration","lingerDuration", and"wordTransitionDuration"insettings_objkeeps the saved JSON in sync withCaptionSettingsand the editor UI. The numeric conversions are appropriate for the underlyingu32/f32fields.
1924-1990: parse_captions_json correctly hydrates new font_weight and duration fieldsThe loader now:
- Reads
fontWeightorfont_weightwith default700and passes it intoCaptionSettings.font_weight.- Keeps
outlinedefaulting tofalse, matching the Rust Default.- Hydrates fade/linger/word transition durations from either camelCase or snake_case keys with sensible defaults.
This keeps legacy caption files compatible while honoring the new styling options.
apps/desktop/src/routes/editor/ConfigSidebar.tsx (3)
2517-2538: HexColorInput swatch + native color picker integration looks solidThe added swatch button and overlaid
<input type="color">are wired cleanly to the existingtext/prevColorstate andonChangecallback. The hex text and picker stay in sync without extra side effects.
2613-2627: Font size scaling logic for text segments is well‑guardedScaling
segment.sizeproportionally whenfontSizechanges (with sensible min font size and amaxSizecap) should give intuitive resizing while avoiding zero/huge values. The checks onsegment.size.x/y > 0.025prevent exploding tiny segments.
2725-2737: Fade Duration sliders for text and mask segments align with the new data modelBoth text and mask Fade Duration controls are constrained to
[0, 1]seconds and default to0.15, which matches the Rust defaults. UsingclampNumberon the text side is a nice defensive guard; the mask slider relying on the slider’s range is acceptable here.Also applies to: 2897-2910
apps/desktop/src/routes/editor/MaskOverlay.tsx (2)
14-40: Multi‑segment, time‑aware mask overlay and selection flow look consistentThe new
currentAbsoluteTime+visibleMaskSegmentsmemo,selectedMaskIndexderivation, andhandleSelectSegment/handleBackgroundClicklogic together give a clear model:
- Only masks active at the current time are rendered.
- Each visible mask gets a hitbox; clicking sets the mask selection in editor state.
- When a mask is selected and visible,
MaskOverlayContenttakes over for drag/resize; otherwise a lightweight outline is shown.This mirrors the TextOverlay behavior and keeps interactions scoped to the currently selected mask.
Also applies to: 58-85, 87-125
128-132: MaskOverlayContent refactor cleanly separates state and visualsPassing a
maskStateaccessor intoMaskOverlayContentand derivingrectfrom it simplifies the component and keeps geometry in one place. The updated styling (gray border/handles) and reuse of the existing drag/resize math look correct, with no new correctness issues introduced.Also applies to: 173-181, 225-270, 274-282
apps/desktop/src/routes/editor/TextOverlay.tsx (2)
23-41: Time‑aware multi‑segment text overlay and selection wiring are solidThe new
visibleTextSegmentsmemo,selectedTextIndex, sharedcreateMouseDownDrag, and background click handler together give a clean interaction model: only active segments are rendered; you can select one from the timeline/overlay and then drag it around; pointer events are disabled when nothing is selected. This matches MaskOverlay and keeps the editor state the single source of truth.Also applies to: 50-101, 103-141
146-174: TextSegmentOverlay normalization, auto‑sizing, and drag/resize behavior are implemented carefully
normalizeSegmentprovides robust defaults for partially‑populated segments coming from Tauri.- The hidden measurement node plus the two
createEffectblocks give reliable auto‑sizing on mount and when content/typography change, whileisResizingavoids fighting user‑driven corner resizes.- Drag and resize handlers clamp positions and sizes to sensible ranges and maintain padding from edges; corner resizing scales both
fontSizeandsizecoherently.Overall this refactor significantly improves text overlay behavior without introducing new correctness issues.
Also applies to: 176-293, 295-445, 447-528
Improves the caption and mask editing experience in the desktop app, with a particular emphasis on simplifying font and style controls, enhancing color input usability, and adding new options for fade duration. Also improves the download progress reporting for Whisper models.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.