✨ feat(linux): background export with system notification#337
Open
dongvv-2538 wants to merge 3 commits intosiddharthvaddem:mainfrom
Open
✨ feat(linux): background export with system notification#337dongvv-2538 wants to merge 3 commits intosiddharthvaddem:mainfrom
dongvv-2538 wants to merge 3 commits intosiddharthvaddem:mainfrom
Conversation
When the user closes the editor window while an MP4 or GIF export is running, a native dialog offers three choices: - Continue in Background: hides the window and lets the export finish - Cancel Export & Close: cancels the job and closes - Cancel: keeps the window open On completion the file is auto-saved to the Downloads folder and a system notification fires with the filename. Clicking the notification reveals the file in the file manager. Changes: - electron/main.ts: track editorIsExporting; intercept close to show the export-in-progress dialog; send background-export-ready and cancel-export-and-close IPC events - electron/ipc/handlers.ts: add save-exported-video-to-path (silent save) and send-export-notification (Electron Notification) handlers - electron/preload.ts: expose setIsExporting, saveExportedVideoToPath, sendExportNotification, onBackgroundExportReady, onCancelExportAndClose, exportCancelledDone - electron/electron-env.d.ts: TypeScript declarations for new API methods - src/components/video-editor/VideoEditor.tsx: add backgroundExportDirRef; notify main process of exporting state; branch save paths for background mode; register onBackgroundExportReady / onCancelExportAndClose listeners - i18n (en/es/zh-CN): add exportInBackground translation keys
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds background-export support across layers: new renderer↔preload APIs, IPC handlers to save files and send notifications (with GIF optimization), main-process close handling for active exports, renderer changes to auto-save to Downloads and handle cancellation, i18n strings, and gifsicle packaging. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c2686c023
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c2686c023
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 514-525: The IPC handler "save-exported-video-to-path" currently
writes arbitrary renderer-supplied paths; restrict it to the user's downloads
folder by resolving and validating the destination in main. In the handler
(ipcMain.handle "save-exported-video-to-path") derive the downloads root via
app.getPath("downloads"), extract/sanitize the incoming filename (e.g.,
path.basename and strip path separators), construct the destination with
path.join(downloadsRoot, safeFilename), then path.resolve and confirm the
resolved path begins with the downloadsRoot; if not, reject and return {
success: false, error: 'invalid path' }. Keep the file write to fs.writeFile
only after this validation and return the success/path result.
In `@electron/main.ts`:
- Around line 311-323: The "Cancel Export & Close" branch currently returns
early and force-closes the window, bypassing the existing unsaved-changes
safeguard; remove the early "return" so execution falls through into the
existing unsaved-changes flow and modify the
ipcMain.once("export-cancelled-done", ...) handler (and the fallback timeout) to
clear the fallback but not call forceCloseEditorWindow(win) directly—allow the
normal unsaved-changes logic to prompt/save/discard the document after the
renderer signals export cancellation (keep the fallback only to clear or
escalate if the renderer never responds).
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1149-1159: Ensure the background-save path is deterministic and
surface failures by checking and awaiting backgroundExportDirRef.current before
attempting background save in the VideoEditor export logic: if
backgroundExportDirRef.current is undefined, wait for or derive a deterministic
fallback directory (e.g., a known Downloads path) rather than falling through to
saveExportedVideo; wrap calls to saveExportedVideoToPath and
sendExportNotification in try/catch and on failure fall back to calling
saveExportedVideo (which shows the save dialog) and surface the error to the
user (e.g., via existing UI error handler), updating the branches around
backgroundExportDirRef.current, saveExportedVideoToPath, and
sendExportNotification (also apply same fixes to the similar block at the other
location).
- Around line 474-480: The exportCancelledDone() IPC call is emitted immediately
after calling exporterRef.current.cancel(), which can let main close the window
before handleExport() finishes; instead, change the cancel flow so
exportCancelledDone() is sent only after the exporter has fully unwound.
Specifically, update the useEffect cancel handler to await a
cancellation-completion signal from the exporter (e.g., make
exporterRef.current.cancel() return a Promise or expose a completion
event/promise like exporterRef.current.onCancelled / exporterRef.current.done)
and call window.electronAPI.exportCancelledDone() only after that promise/event
resolves; ensure this cooperates with handleExport()’s await exporter.export()
and its finally block so state cleanup runs before notifying main.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e331cd9-80e5-44ef-939f-1e4664e2ce3d
📒 Files selected for processing (8)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/main.tselectron/preload.tssrc/components/video-editor/VideoEditor.tsxsrc/i18n/locales/en/dialogs.jsonsrc/i18n/locales/es/dialogs.jsonsrc/i18n/locales/zh-CN/dialogs.json
- handlers.ts: validate save-exported-video-to-path destination is within the Downloads folder (path.resolve + startsWith check) to prevent path traversal; derive safe filename with path.basename; write to the resolved destination and return it in the success response - main.ts: 'Cancel Export & Close' now calls win.close() after the renderer ACKs cancellation instead of force-closing, allowing the normal unsaved-changes guard to run; fallback timeout cleans up the listener instead of force-closing - VideoEditor.tsx: wrap background save/notify (GIF and MP4) in try/catch; on failure clear backgroundExportDirRef and fall back to the normal save dialog so the user can still save their export - VideoEditor.tsx: defer exportCancelledDone() until after cancel() has propagated (via a zero-timeout microtask flush) so the main process does not race ahead of handleExport's finally-block cleanup
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
electron/main.ts (1)
315-326:⚠️ Potential issue | 🟠 MajorKeep the timeout path inside the normal close flow.
Line 325 still jumps straight to
forceCloseEditorWindow(win). If the renderer never ACKs within 5 seconds, this bypasses the unsaved-changes guard at Lines 333-365 and can drop dirty project state without offering Save / Discard / Cancel. Re-enter the normalwin.close()path here instead of force-closing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/main.ts` around lines 315 - 326, The fallback timeout currently calls forceCloseEditorWindow(win) which bypasses the normal unsaved-changes flow; instead, change the timeout handler to remove the "export-cancelled-done" listener, clear the fallback timeout, and call win.close() so the normal close/unsaved-changes handling (the logic tied to win.close() and the unsaved-changes check) runs; update or remove any flags/listeners as needed so this re-entrancy does not loop (use the existing onCancelled and editor-exporting state rather than forceCloseEditorWindow).src/components/video-editor/VideoEditor.tsx (2)
475-485:⚠️ Potential issue | 🟠 MajorDon't ACK cancellation before
handleExport()has actually unwound.Lines 482-484 send
exportCancelledDone()after a timer tick, and Line 1483 clearssetIsExporting(false)immediately aftercancel().electron/main.tstreats those signals as “safe to close”, butawait exporter.export()can still be in flight and itsfinallymay not have run yet. Please drive both signals from a real cancellation-complete promise/event, not a microtask delay.Also applies to: 1480-1484
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 475 - 485, The code currently calls exporter.cancel() and then uses a timer tick to call window.electronAPI.exportCancelledDone(), which can race with handleExport's finally that clears state (setIsExporting(false)); instead, change the flow so exportCancelledDone() is invoked only after a real "cancellation complete" promise/event from the exporter or from handleExport. Concretely: update the exporter API or handleExport to expose a cancellation-completion promise or event (e.g., exporter.cancel() returns a Promise or emit/exporter.on('cancelled') when fully unwound), then in the onCancelExportAndClose handler await that cancellation-complete signal before calling window.electronAPI.exportCancelledDone() and before clearing isExporting; replace the setTimeout/microtask trick with awaiting that true completion to avoid races between exporter.cancel(), handleExport, and exportCancelledDone().
1157-1160:⚠️ Potential issue | 🟠 MajorWait for the Downloads path before deciding between background-save and dialog-save.
Lines 1157-1159 and 1324-1326 snapshot
backgroundExportDirRef.currentonce, then Lines 1190 and 1357 immediately fall back tosaveExportedVideo()if it's still null. If the export finishes just beforeonBackgroundExportReady()runs, this can open the normal save dialog while the editor is hidden. Gate this branch on a promise/latch for the ready IPC, or another deterministic Downloads path, instead of a best-effort ref read.Also applies to: 1189-1190, 1324-1327, 1356-1357
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 1157 - 1160, The current logic reads backgroundExportDirRef.current once and immediately falls back to saveExportedVideo() if null, which races with the IPC that supplies the Downloads path; change this to await a deterministic readiness signal instead of a best-effort ref read: introduce or reuse a promise/latch that onBackgroundExportReady resolves (e.g., backgroundExportReadyPromise), then in the branches that currently check backgroundExportDirRef.current (the blocks that build fullPath and the fallbacks to saveExportedVideo()), await that promise (with a short timeout if needed) and only treat a missing Downloads path as final after the promise settles; update all places referencing backgroundExportDirRef.current and the immediate calls to saveExportedVideo() to first await the readiness signal so the dialog isn’t opened while the editor is hidden.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1161-1188: The try/catch currently couples the file write and
notification so a notification failure triggers the fallback save; change the
logic in the block using saveExportedVideoToPath / sendExportNotification so you
first call await window.electronAPI.saveExportedVideoToPath(arrayBuffer,
fullPath) and handle its failure by entering the fallback that calls
window.electronAPI.saveExportedVideo(...) (clearing
backgroundExportDirRef.current, setting setUnsavedExport, etc.); if the write
succeeds, then call window.electronAPI.sendExportNotification("GIF",
saveResult.path) inside its own try/catch and treat notification errors as
non-fatal (log them but do not fall back to manual save or change
setUnsavedExport/handleExportSaved), and apply the same refactor to the
analogous block with saveExportedVideoToPath/sendExportNotification at lines
~1328-1355 so only actual write failures trigger the manual save flow.
---
Duplicate comments:
In `@electron/main.ts`:
- Around line 315-326: The fallback timeout currently calls
forceCloseEditorWindow(win) which bypasses the normal unsaved-changes flow;
instead, change the timeout handler to remove the "export-cancelled-done"
listener, clear the fallback timeout, and call win.close() so the normal
close/unsaved-changes handling (the logic tied to win.close() and the
unsaved-changes check) runs; update or remove any flags/listeners as needed so
this re-entrancy does not loop (use the existing onCancelled and
editor-exporting state rather than forceCloseEditorWindow).
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 475-485: The code currently calls exporter.cancel() and then uses
a timer tick to call window.electronAPI.exportCancelledDone(), which can race
with handleExport's finally that clears state (setIsExporting(false)); instead,
change the flow so exportCancelledDone() is invoked only after a real
"cancellation complete" promise/event from the exporter or from handleExport.
Concretely: update the exporter API or handleExport to expose a
cancellation-completion promise or event (e.g., exporter.cancel() returns a
Promise or emit/exporter.on('cancelled') when fully unwound), then in the
onCancelExportAndClose handler await that cancellation-complete signal before
calling window.electronAPI.exportCancelledDone() and before clearing
isExporting; replace the setTimeout/microtask trick with awaiting that true
completion to avoid races between exporter.cancel(), handleExport, and
exportCancelledDone().
- Around line 1157-1160: The current logic reads backgroundExportDirRef.current
once and immediately falls back to saveExportedVideo() if null, which races with
the IPC that supplies the Downloads path; change this to await a deterministic
readiness signal instead of a best-effort ref read: introduce or reuse a
promise/latch that onBackgroundExportReady resolves (e.g.,
backgroundExportReadyPromise), then in the branches that currently check
backgroundExportDirRef.current (the blocks that build fullPath and the fallbacks
to saveExportedVideo()), await that promise (with a short timeout if needed) and
only treat a missing Downloads path as final after the promise settles; update
all places referencing backgroundExportDirRef.current and the immediate calls to
saveExportedVideo() to first await the readiness signal so the dialog isn’t
opened while the editor is hidden.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1a151d7-4a07-4d7c-bb76-a975d6a674de
📒 Files selected for processing (3)
electron/ipc/handlers.tselectron/main.tssrc/components/video-editor/VideoEditor.tsx
✅ Files skipped from review due to trivial changes (1)
- electron/ipc/handlers.ts
- Install gifsicle npm package and unpack binary via asarUnpack so it is accessible outside the asar archive in packaged builds - Add optimizeGifBuffer() in IPC handlers that runs gifsicle -O3 --lossy=40 --colors 256 after gif.js encoding; integrated into both save-exported-video and save-exported-video-to-path handlers - Resolve gifsicle binary path at runtime (app.getAppPath in dev, process.resourcesPath in packaged) to avoid import.meta.url crash when Vite bundles the main process - Disable FloydSteinberg dithering in gif.js (dither noise destroys LZW compression) and set quality:1 for accurate palette sampling - Add Small (480p) size preset and 5/10 FPS frame rate options to give users file-size-friendly export choices - Change default GIF frame rate from 15 FPS to 10 FPS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the user closes the editor window while an MP4 or GIF export is running, a native dialog now intercepts the close and offers three choices:
When a background export completes, the file is auto-saved to the Downloads folder and a system notification fires showing the filename. Clicking the notification reveals the file in the file manager.
Changes
Electron (main process)
electron/main.ts: addededitorIsExportingflag; IPC listener forset-is-exporting; editor windowclosehandler now shows the export-in-progress dialog and sendsbackground-export-ready/cancel-export-and-closeIPC events accordinglyelectron/ipc/handlers.ts: two new handlers —save-exported-video-to-path(silent file write, no dialog) andsend-export-notification(ElectronNotificationwith click-to-reveal); addedNotificationto electron importsPreload bridge
electron/preload.ts: exposed six newelectronAPImethods:setIsExporting,saveExportedVideoToPath,sendExportNotification,onBackgroundExportReady,onCancelExportAndClose,exportCancelledDoneelectron/electron-env.d.ts: TypeScript declarations for all six new methodsRenderer
src/components/video-editor/VideoEditor.tsx:backgroundExportDirRefref to hold the Downloads path when running headlesslywindow.electronAPI.setIsExporting(true/false)around every export to keep main process in synconBackgroundExportReadylistener (stores the downloads dir) andonCancelExportAndCloselistener (cancels the exporter and ACKs)backgroundExportDirRef.current— if set, they auto-save viasaveExportedVideoToPath+sendExportNotificationinstead of showing the save dialoghandleCancelExportalso notifies the main process immediatelyi18n
en/dialogs.json,es/dialogs.json,zh-CN/dialogs.json: newexportInBackgroundtranslation namespaceTesting
Summary by CodeRabbit
New Features
Changes