Add editor autosave recovery#538
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds local autosave/recovery to VideoEditor (localStorage draft read/validate/write, restore/discard dialog, passes saved snapshot into applyLoadedProject) and refactors useScreenRecorder to memoize safeHideCountdownOverlay and include it in effect dependencies. ChangesVideoEditor Recovery Draft
useScreenRecorder Countdown Overlay Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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. Review rate limit: 4/8 reviews remaining, refill in 30 minutes.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: 0084ca1113
ℹ️ 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".
| if (!currentProjectMedia || !currentProjectSnapshot || !hasUnsavedChanges) { | ||
| clearRecoveryDraft(); | ||
| return; |
There was a problem hiding this comment.
This effect deletes the persisted recovery payload whenever the currently loaded project has no unsaved changes, which happens immediately on startup for a normal saved project. Because pendingRecoveryDraft is only kept in memory after load, a restart/crash before the user clicks Restore will permanently drop the recoverable edits from localStorage. The clear path should be gated so existing recovery data is retained until explicit restore/discard.
Useful? React with 👍 / 👎.
| /> | ||
| <Dialog | ||
| open={Boolean(recoveryDraft)} | ||
| onOpenChange={(open) => !open && handleDiscardRecoveryDraft()} |
There was a problem hiding this comment.
The dialog’s onOpenChange treats any close action as a hard discard, and this DialogContent includes the default close affordances (escape, outside click, and the top-right close button). That means a user can lose the only recovery draft by accidentally dismissing the modal instead of intentionally pressing the Discard action. Closing should not delete recovery data unless the user explicitly confirms discard.
Useful? React with 👍 / 👎.
![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
🤖 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 388-463: Autosave and the unsaved-changes flag miss updates to
webcamSizePreset and cursorHighlight because hasUnsavedChanges is derived from
currentProjectSnapshot which doesn't include those fields; update the snapshot /
hashing logic (where currentProjectSnapshot or lastSavedSnapshot is
created—e.g., the createProjectData or snapshot serialization/hash function) to
include webcamSizePreset and cursorHighlight so changes to those controls update
currentProjectSnapshot, flip hasUnsavedChanges, and trigger the
autosave/editorState behavior shown in the VideoEditor useEffect.
- Around line 388-463: The effect currently calls clearRecoveryDraft() whenever
hasUnsavedChanges is false which deletes the persisted draft while the restore
dialog may still be open; update the condition so we no longer clear the
localStorage draft merely because the live editor is "clean" — instead only
clear when there is truly no project context (e.g. !currentProjectMedia ||
!currentProjectSnapshot) or when the draft belongs to a different project
(compare draft.projectPath to currentProjectPath) or when the user explicitly
dismisses the restore dialog; modify the useEffect in VideoEditor (the block
referencing hasUnsavedChanges and clearRecoveryDraft) to remove
hasUnsavedChanges as the sole trigger and add a guard for an explicit
restore/discard state (or a projectPath mismatch) before calling
clearRecoveryDraft.
- Around line 2241-2265: The recovery Dialog is currently rendered after an
early `if (error)` return so it never appears when startup errors occur; to fix,
render the <Dialog> (controlled by `Boolean(recoveryDraft)`) alongside or above
the error UI so it mounts even when `error` is set — move the Dialog block out
of the post-error return (or include it inside the error rendering branch) and
keep its handlers `handleDiscardRecoveryDraft` and `handleRestoreRecoveryDraft`
and props (`open`, `onOpenChange`) intact so users can restore/discard drafts
while on the error screen.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19238c15-c0fe-4eaa-b572-3c6a6f4b2ea4
📒 Files selected for processing (2)
src/components/video-editor/VideoEditor.tsxsrc/hooks/useScreenRecorder.ts
| <Dialog | ||
| open={Boolean(recoveryDraft)} | ||
| onOpenChange={(open) => !open && handleDiscardRecoveryDraft()} | ||
| > | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <DialogTitle>Restore unsaved edits?</DialogTitle> | ||
| <DialogDescription> | ||
| OpenScreen recovered unsaved editor changes from{" "} | ||
| {recoveryDraft | ||
| ? new Date(recoveryDraft.savedAt).toLocaleString() | ||
| : "a previous session"} | ||
| . | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <DialogFooter> | ||
| <Button type="button" variant="secondary" onClick={handleDiscardRecoveryDraft}> | ||
| Discard | ||
| </Button> | ||
| <Button type="button" onClick={handleRestoreRecoveryDraft}> | ||
| Restore | ||
| </Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> |
There was a problem hiding this comment.
Keep the recovery dialog reachable on startup failures.
This modal is mounted after the existing if (error) early return above, so any startup path that sets an error will skip the restore/discard prompt entirely. That’s the worst time to hide it — users can’t recover the draft if they’re stuck on the error screen.
♻️ Possible shape
- if (error) {
+ if (error && !recoveryDraft) {
return (
<div className="flex items-center justify-center h-screen bg-background">
<div className="text-destructive">{error}</div>
</div>
);
}🤖 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 2241 - 2265, The
recovery Dialog is currently rendered after an early `if (error)` return so it
never appears when startup errors occur; to fix, render the <Dialog> (controlled
by `Boolean(recoveryDraft)`) alongside or above the error UI so it mounts even
when `error` is set — move the Dialog block out of the post-error return (or
include it inside the error rendering branch) and keep its handlers
`handleDiscardRecoveryDraft` and `handleRestoreRecoveryDraft` and props (`open`,
`onOpenChange`) intact so users can restore/discard drafts while on the error
screen.
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Refactor