fix: stop cancelled confirm dialogs from running actions - #312
Merged
Merged
Conversation
Escape or a backdrop click during an action closed the dialog. Material then nulls componentInstance, and the next loading.set() threw. The modal now sets disableClose while loading is set, and wrappers that touch componentInstance after an await use optional chaining. Also rename ConfirmRepsonse to ConfirmResponse, type reason as 'done' | undefined (it is undefined on dismiss), and move the receiptToTsv JSDoc back above its function. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
openConfirmModal always resolves to an object, so `if (!details) return` never fired. Cancel, Escape and backdrop clicks still ran the delete in domains (application), edge, build list, brokers, signage plugins and trigger reorder. All callers now use `details.reason !== 'done'`. Also close the modal on every path so no spinner is left stuck: - trigger reorder now closes on success and error, with a reorder icon - metadata removal now closes the modal after confirming - api keys, storage, upload library and resource imports close in a finally block when the request throws Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Select and trigger-settings dialogs resolve with reason 'action', not 'done', so the stricter check skipped the action every time. Dialogs that race afterClosed() can also resolve with undefined. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Without it the confirm buttons stay live during the request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
This branch was successfully deployed
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.
Problem
openConfirmModalalways returns an object. Six callers checkedif (!details) return, which never stops the action. Cancel, Escape or a backdrop click still deleted the item:Escape or a backdrop click during the request also closed the dialog. Material then clears
componentInstance, so the laterloading.set()call threw.This is part of a stack of 6 PRs from a review of the whole app. Merge them in order: confirm-cancel, security, data loss, broken features, async errors, cleanup. Each PR targets the branch before it, so the diff shows only its own changes.
Fix
openConfirmModalnow checkdetails.reason !== 'done'. Select dialogs that resolve with'action'check for that reason.disableClosewhile it is loading. The receipt view can still be closed.ConfirmRepsonsetoConfirmResponseand typereasonas optional.Checks
tsc,bun run lint,bunx vitest run(955 tests) pass. New tests cover a dismissed modal anddisableClosewhile loading. Both fail without the fix.Made by Claude Opus 5.5 (1M context) in Claude Code (T3 Code).
🤖 Generated with Claude Code