Customisable Delete Recording behaviour #865
Conversation
WalkthroughA new user-configurable setting for post-recording deletion behavior was introduced across backend and frontend components. The Rust backend added an enum and updated settings storage, while the frontend UI and types were extended to support and display the new option. Additional changes improved actor shutdown logic and removed unused icon declarations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (Settings)
participant Backend (Tauri)
participant Window Manager
User->>UI (Settings): Selects "After deleting recording" option
UI (Settings)->>Backend (Tauri): Saves postDeletionBehaviour setting
User->>Backend (Tauri): Deletes a recording
Backend (Tauri)->>Backend (Tauri): Cancels and deletes recording
Backend (Tauri)->>Backend (Tauri): Reads postDeletionBehaviour
alt postDeletionBehaviour == DoNothing
Backend (Tauri)->>Window Manager: Close main window
else postDeletionBehaviour == ReopenRecordingWindow
Backend (Tauri)->>Window Manager: Reopen main recording window
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
apps/desktop/src/utils/tauri.ts (1)
386-386: PostDeletionBehaviour union matches Rust variants.Literal values "exit" and "reopenRecordingWindow" are consistent with the Rust enum and rename_all = "camelCase". Consider adding brief docs on the Rust enum variants so tauri-specta emits helpful TS comments.
crates/recording/src/studio_recording.rs (1)
453-461: Cancel path now stops cursor actor before pipeline shutdown — confirm ordering or unify.You now:
- stop the cursor actor if present, then
- shutdown the pipeline.
In Pause you do the opposite (shutdown pipeline first, then stop cursor inside shutdown()). Both orders are arguably safe, but the discrepancy is subtle. Either:
- document why Cancel needs a different order, or
- unify the order across paths for predictability.
Also, Cancel intentionally avoids persisting partial cursor data (cursor.json). If that’s the desired behavior, great; otherwise consider reusing the shutdown helper with a flag.
Example (if you prefer pipeline-first for Cancel too):
- if let Some(cursor) = &mut pipeline.cursor - && let Some(actor) = cursor.actor.take() - { - actor.stop().await; - } - - pipeline.inner.shutdown().await + let res = pipeline.inner.shutdown().await; + if let Some(cursor) = &mut pipeline.cursor + && let Some(actor) = cursor.actor.take() + { + actor.stop().await; + } + resapps/desktop/src-tauri/src/recording.rs (1)
488-490: Nit: clarify comment or remove.“Actor hasn't errored, it's just finished” is fine but redundant with the surrounding match. Optional: drop or expand to say why nothing needs to be done on this branch.
apps/desktop/src-tauri/src/general_settings.rs (1)
24-31: New PostDeletionBehaviour enum: serde defaults and casing are correct.
- rename_all = "camelCase" will map to "exit" and "reopenRecordingWindow" in TS.
- Default variant Exit defined. Looks good.
Optional nit: add brief doc comments to variants to improve generated TS docs.
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (2)
149-160: Type union extension is fine; consider tightening generics to dropany.Adding
PostDeletionBehaviourto the select types is correct. Optionally, makeSelectSettingItemandrenderRecordingSelectgeneric to eliminate theanyin options/onChange and prevent cross-setting value mix-ups.Example:
type SelectValue = | MainWindowRecordingStartBehaviour | PostStudioRecordingBehaviour | PostDeletionBehaviour | number; type SelectSettingItem<T extends SelectValue = SelectValue> = { label: string; type: "select"; description: string; value: T; onChange: (value: T) => void | Promise<void>; }; function renderRecordingSelect<T extends SelectValue>( label: string, description: string, getValue: () => T, onChange: (value: T) => void, options: { text: string; value: T }[] ) { /* ... */ }
448-461: Option values look correct; align label with behavior and earlier terminology.
- Values
"exit"and"reopenRecordingWindow"match the intended behaviors. Good.- The label “Hide Cap” might be misleading if the behavior actually closes the main window (per backend summary). Consider “Close window” for consistency with the “Main window recording start behaviour” options (“Close”, “Minimise”).
Optional text tweak:
- { text: "Hide Cap", value: "exit" }, + { text: "Close window", value: "exit" },Please also confirm that “exit” does not quit the entire app on any OS (especially macOS), but only closes/hides the main window, matching the backend’s “close main window” behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src-tauri/src/general_settings.rs(3 hunks)apps/desktop/src-tauri/src/recording.rs(3 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(5 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)crates/recording/src/studio_recording.rs(1 hunks)packages/ui-solid/src/auto-imports.d.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/ui-solid/src/auto-imports.d.ts
🔇 Additional comments (6)
apps/desktop/src/utils/tauri.ts (1)
363-363: TS settings shape aligns with Rust serde(camelCase).The optional postDeletionBehaviour property is correctly added and matches backend naming via serde(camelCase). No action required.
crates/recording/src/studio_recording.rs (1)
324-329: Good cleanup: stop cursor actor when pipeline finishes.Prevents a dangling cursor actor when the pipeline completes cleanly. Looks correct and side-effect free.
Minor check: this path does not persist cursor events (unlike Pause/Stop which call shutdown()). If that’s intentional for this completion path, we’re good. If not, consider reusing the shutdown helper for symmetry.
apps/desktop/src-tauri/src/recording.rs (1)
9-10: Import looks correct.PostDeletionBehaviour is used later in delete_recording; no issues.
apps/desktop/src-tauri/src/general_settings.rs (2)
90-92: Backwards compatibility: serde(default) on new field is correct.Existing stores won’t break and will default to Exit. Good addition.
132-133: Default implementation updated — consistent with enum default.Explicitly setting post_deletion_behaviour: Exit maintains clarity and avoids relying solely on enum default. LGTM.
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (1)
324-329: Render helper type update LGTM.Extending
getValue’s return type to includePostDeletionBehaviouris consistent with the new setting.
Provides the choice for the recording window to reopen after deleting an in progress recording
Summary by CodeRabbit
New Features
Bug Fixes
Chores