fix: prompt for unsaved changes in new projects#312
fix: prompt for unsaved changes in new projects#312LocNguyenSGU wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b3e839711
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (lastSavedSnapshot === null) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Avoid marking untouched new projects as dirty
Returning true whenever lastSavedSnapshot is null makes every newly opened recording/session look dirty immediately, even before the user edits anything. In this flow currentProjectSnapshot is populated as soon as media loads, so hasUnsavedChanges is sent to the main process as true and the close handler will always show the unsaved-changes prompt for untouched projects. This is a regression from the previous behavior and creates a persistent false-positive save prompt on close for new projects.
Useful? React with 👍 / 👎.
|
@LocNguyenSGU Ci checks are failing. Can you take another look. |
|
Hi @LocNguyenSGU Thanks for working on this. After taking a closer look at #321 I think I am going to probably have those changes in for fixing this. It sets a baseline snapshot on load means you can distinguish "user hasn't changed anything" from "never saved" without special-casing null. I feel like this is better placed in the persistence layer than the videoeditor. |
Summary
Fix the editor dirty-state logic so new projects that have never been saved still trigger the unsaved-changes prompt after edits.
Reproduction
A newly loaded recording/session resets both
currentProjectPathandlastSavedSnapshottonull. After making edits, the previous dirty-state rule still returned false because it requiredcurrentProjectPath.Patch Summary
Risks
Low. The change is limited to dirty-state calculation and its test coverage.
Test Coverage
src/components/video-editor/VideoEditor.test.tsSummary by CodeRabbit
Tests
Refactor