Save Theme Button After Change Won't Work - #282
Conversation
Save button got stuck as a static 'Saved' confirmation because the auto-close timeout checked a stale closure of saveSuccess (always false, so it never fired) and no code cleared saveSuccess on subsequent edits; fixed by tracking success in a ref for the timeout and resetting saveSuccess whenever the user edits the space again.
Walkthrough
ChangesSpace editor save-state flow
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Greptile SummaryThe PR resets saved-state feedback when theme or name values change and replaces the stale timeout closure with a ref. The shared ref leaves overlapping save timeouts unable to distinguish which save they belong to.
Confidence Score: 4/5The overlapping-save timeout race should be fixed before merging because an older timeout can close the editor during a newer save confirmation. The new mutable ref fixes the stale closure for a single save but conflates multiple save generations, allowing an earlier uncancelled timeout to observe a later success and close prematurely. Files Needing Attention: src/renderer/src/components/settings/sections/spaces/space-editor.tsx Important Files Changed
Reviews (1): Last reviewed commit: "Save Theme Button After Change Won't Wor..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (saveSuccessRef.current) { | ||
| onClose(); | ||
| } |
There was a problem hiding this comment.
When the user saves, edits, and successfully saves again within 1.5 seconds, the first uncancelled timeout reads the shared saveSuccessRef value written by the second save and closes the editor less than 1.5 seconds after the latest save.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/components/settings/sections/spaces/space-editor.tsx`:
- Around line 68-73: Update handleSave and both edit handlers to use an
edit-generation counter: capture the current generation when handleSave starts,
increment it whenever Lines 35 or 109 process an edit, and only set
saveSuccessRef/current success state or invoke onClose from the save completion
timeout when the captured generation still matches. Ensure each timeout is tied
to its originating save so an older request cannot overwrite newer edits or
close the editor prematurely.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 9d1fa668-6abf-43f3-82fa-5143c5c87e35
📒 Files selected for processing (1)
src/renderer/src/components/settings/sections/spaces/space-editor.tsx
| saveSuccessRef.current = true; | ||
| setSaveSuccess(true); | ||
|
|
||
| // Auto-close after short delay | ||
| // Auto-close after short delay, unless the user started editing again in the meantime | ||
| setTimeout(() => { | ||
| if (saveSuccess) { | ||
| if (saveSuccessRef.current) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not let an older save overwrite a newer edit.
If the user edits the space while flow.spaces.updateSpace(...) is pending, Lines 35 or 109 set saveSuccessRef.current to false. When the request resolves, Line 68 unconditionally sets it back to true, and Lines 72-73 can call onClose() while editedSpace contains unsaved changes.
The shared boolean also does not identify which save owns the timeout. A timeout from an earlier save can observe true from a later save and close the editor too early.
Capture an edit generation when handleSave starts. Increment it in both edit handlers. Apply the success state and close only when the captured generation still matches.
Proposed fix
const saveSuccessRef = useRef(false);
+const editGenerationRef = useRef(0);
const updateEditedSpace = (updates: Partial<Space>) => {
+ editGenerationRef.current += 1;
setEditedSpace((prev) => ({ ...prev, ...updates }));
const handleSave = async () => {
+ const saveGeneration = editGenerationRef.current;
setIsSaving(true);
setSaveSuccess(false);
try {
...
await flow.spaces.updateSpace(space.profileId, space.id, updatedFields);
onSpacesUpdate();
+ if (saveGeneration !== editGenerationRef.current) {
+ return;
+ }
saveSuccessRef.current = true;
setSaveSuccess(true);
setTimeout(() => {
- if (saveSuccessRef.current) {
+ if (saveGeneration === editGenerationRef.current && saveSuccessRef.current) {
onClose();
}
}, 1500);
}
const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+ editGenerationRef.current += 1;
setEditedSpace({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/src/components/settings/sections/spaces/space-editor.tsx` around
lines 68 - 73, Update handleSave and both edit handlers to use an
edit-generation counter: capture the current generation when handleSave starts,
increment it whenever Lines 35 or 109 process an edit, and only set
saveSuccessRef/current success state or invoke onClose from the save completion
timeout when the captured generation still matches. Ensure each timeout is tied
to its originating save so an older request cannot overwrite newer edits or
close the editor prematurely.
1 participant
Fixes #281
Save button got stuck as a static 'Saved' confirmation because the auto-close timeout checked a stale closure of saveSuccess (always false, so it never fired) and no code cleared saveSuccess on subsequent edits; fixed by tracking success in a ref for the timeout and resetting saveSuccess whenever the user edits the space again.
Testing: Repo has no test suite (confirmed no test files/dirs); verified by tracing the exact stale-closure bug against the reported repro steps and confirming the edited file still parses/formats cleanly via npx prettier --check (no syntax errors) -- did not run bun install/full typecheck as it was not needed to validate this narrow logic fix.
Summary by CodeRabbit