Skip to content

feat: Add exportFolder to user preferences#512

Open
AbhinRustagi wants to merge 3 commits intosiddharthvaddem:mainfrom
AbhinRustagi:feature/remember-last-export-folder
Open

feat: Add exportFolder to user preferences#512
AbhinRustagi wants to merge 3 commits intosiddharthvaddem:mainfrom
AbhinRustagi:feature/remember-last-export-folder

Conversation

@AbhinRustagi
Copy link
Copy Markdown

@AbhinRustagi AbhinRustagi commented May 1, 2026

Description

Adds exportFolder as a key to user preferences, and stores the location of the last saved video, and prefills it in the next export action.

Type of Change

  • New Feature

Related Issue(s)

Closes #503

Screenshots / Video

Screenshot (if applicable):

![Screenshot Description](path/to/screenshot.png)

Video (if applicable):

<video src="path/to/video.mp4" controls width="600"></video>

Testing

  1. Build and launch the app.
  2. Export a video; in the save dialog, navigate to a folder other than ~/Downloads and save.
  3. Export again — the save dialog should open in the folder chosen in step 2.
  4. Quit and relaunch the app, then export — the dialog should still open in that folder (preference is persisted via
    localStorage).
  5. Rename or delete that folder on disk, then export again — the dialog should fall back to ~/Downloads without erroring.
  6. On Windows, repeat steps 2–3 to confirm parentDirectoryOf handles \ separators.

Checklist

  • I have performed a self-review of my code.
  • I have added any necessary screenshots or videos.
  • I have linked related issue(s) and updated the changelog if applicable.

Thank you for contributing!

Summary by CodeRabbit

  • New Features
    • Export save dialog now remembers and defaults to your last used export folder for smoother exports.
    • If the saved folder is missing or invalid, the dialog falls back to the system downloads folder.
  • Tests
    • Added unit tests covering parent-directory detection across POSIX and Windows path formats.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16f458b7-1cb4-404e-976f-8b5790591200

📥 Commits

Reviewing files that changed from the base of the PR and between c407276 and b801c1c.

📒 Files selected for processing (2)
  • src/lib/userPreferences.test.ts
  • src/lib/userPreferences.ts
✅ Files skipped from review due to trivial changes (1)
  • src/lib/userPreferences.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/userPreferences.ts

📝 Walkthrough

Walkthrough

Threads an optional exportFolder through renderer → preload → IPC → main. Main validates the folder exists before using it as the save-dialog default (falls back to downloads). VideoEditor persists the parent directory into user preferences and forwards it on subsequent exports.

Changes

Cohort / File(s) Summary
Electron IPC & types
electron/electron-env.d.ts, electron/preload.ts, electron/ipc/handlers.ts
Added optional exportFolder?: string to the renderer API and IPC handler. Main handler now validates exportFolder with fs.stat, uses it as the dialog default when valid, and falls back to downloads otherwise.
Video editor integration
src/components/video-editor/VideoEditor.tsx
Export flows now pass the persisted exportFolder to electronAPI.saveExportedVideo. On save completion, the editor derives and persists the parent directory of the saved file.
User preferences & util
src/lib/userPreferences.ts, src/lib/userPreferences.test.ts
Added `exportFolder: string

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • siddharthvaddem

Poem

saved where? oh right — it remembers.
last folder lives, tidy as a drawer.
dialogs nod, exports find home,
tiny memory, big relief. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and concisely describes the main feature being added: storing exportFolder in user preferences.
Description check ✅ Passed Description covers all key sections: clear purpose, motivation (closes #503), type of change marked, testing steps provided, and checklist completed.
Linked Issues check ✅ Passed PR fully implements issue #503 requirements: remembers last export folder, persists via localStorage, and falls back to ~/Downloads when folder is missing.
Out of Scope Changes check ✅ Passed All changes directly support the export folder feature: TypeScript definitions, IPC handlers, preload bridge, VideoEditor integration, user preferences storage, and path utilities.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/lib/userPreferences.ts (1)

89-98: 💤 Low value

looks good overall, one tiny edge case

the dual-separator handling is solid for cross-platform. one minor nit: for a Windows root-level save like C:\file.mp4, this returns "C:" rather than "C:\". technically C: means "current directory on drive C" while C:\ means root, but this is such a rare edge case (who saves exports to root?) and the handler's fs.stat check would validate it anyway.

lowkey fine to ship as-is, just wanted to call it out.

🔧 optional: handle Windows drive root edge case
 export function parentDirectoryOf(filePath: string): string | null {
 	const lastSep = Math.max(filePath.lastIndexOf("/"), filePath.lastIndexOf("\\"));
 	if (lastSep <= 0) return null;
-	return filePath.slice(0, lastSep);
+	const parent = filePath.slice(0, lastSep);
+	// Windows drive letter without trailing slash (e.g. "C:") - add the backslash
+	if (/^[a-zA-Z]:$/.test(parent)) {
+		return parent + "\\";
+	}
+	return parent;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/userPreferences.ts` around lines 89 - 98, The parentDirectoryOf
function returns "C:" for Windows root saves like "C:\file.mp4" because it
slices up to the separator index (2) instead of including the backslash; update
parentDirectoryOf to detect a Windows drive-root pattern (e.g., /^[A-Za-z]:\\/
or check filePath[1]==':' and filePath[2]=='\\') when lastSep === 2 and return
the three-character root ("C:\\"), otherwise keep the existing behavior (use
lastSep to slice). Ensure you reference the parentDirectoryOf function and
handle both "/" and "\\" separators as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/userPreferences.ts`:
- Around line 89-98: The parentDirectoryOf function returns "C:" for Windows
root saves like "C:\file.mp4" because it slices up to the separator index (2)
instead of including the backslash; update parentDirectoryOf to detect a Windows
drive-root pattern (e.g., /^[A-Za-z]:\\/ or check filePath[1]==':' and
filePath[2]=='\\') when lastSep === 2 and return the three-character root
("C:\\"), otherwise keep the existing behavior (use lastSep to slice). Ensure
you reference the parentDirectoryOf function and handle both "/" and "\\"
separators as before.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2734a685-11c3-429b-96aa-986b5784f5c9

📥 Commits

Reviewing files that changed from the base of the PR and between 884021c and c407276.

📒 Files selected for processing (5)
  • electron/electron-env.d.ts
  • electron/ipc/handlers.ts
  • electron/preload.ts
  • src/components/video-editor/VideoEditor.tsx
  • src/lib/userPreferences.ts

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c40727672f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/lib/userPreferences.ts Outdated
@makaradam makaradam mentioned this pull request May 2, 2026
4 tasks
@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, saveExportedVideo can return an error field from the main-process IPC handler, but the renderer-visible Window.electronAPI.saveExportedVideo type omits it. This makes the API contract inaccurate and prevents typed callers from accessing error details on failure paths.

Severity: remediation recommended | Category: correctness

How to fix: Add error to return type

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

window.electronAPI.saveExportedVideo's TypeScript declaration is missing the error?: string field even though the IPC handler returns it on failure.

Issue Context

The main-process IPC handler for save-exported-video includes error: String(error) in its catch block return value.

Fix Focus Areas

  • electron/electron-env.d.ts[74-82]
  • electron/ipc/handlers.ts[881-887]

Suggested change

Update the return type in electron/electron-env.d.ts to include error?: string (and optionally update any renderer handling to display/log it when present).


Found by Qodo code review

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: remember last exporting folder

2 participants