fix(web): wire saved-template delete in New Project flow#2296
fix(web): wire saved-template delete in New Project flow#2296nehaprasad-dev wants to merge 2 commits into
Conversation
The delete button in From Template was a no-op because onDeleteTemplate never reached the modal. Forward the callback through EntryShell and NewProjectModal, confirm before DELETE, and show errors on failure.
lefarcen
added
size/M
lefarcen
left a comment
lefarcen
left a comment
There was a problem hiding this comment.
Hi @nehaprasad-dev! Thanks for tying this directly back to #2237 — the user-facing delete flow and failure case are clear.
Quick PR-description cleanup before review continues: could you update the body into the current template shape so the Why, What users will see, Surface area, and Validation sections are recognized cleanly? You already have most of the right content; the main thing is to use the Surface area checklist format and mark UI / i18n keys there so reviewers can scope the change quickly.
Once that metadata is cleaned up, @mrcfps has been requested for the code review.
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@nehaprasad-dev Thanks for wiring the saved-template delete flow back up through the New Project modal. I found one keyboard interaction regression that would be good to tighten up before merge.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| onClick={(e) => e.stopPropagation()} | ||
| role="alertdialog" | ||
| aria-modal="true" | ||
| data-testid="template-delete-confirm-dialog" |
There was a problem hiding this comment.
Opening this nested confirmation inside NewProjectModal introduces an Escape-key regression: while this dialog is open, pressing Escape still hits the parent modal's document-level handler and closes the entire New Project flow instead of just dismissing the delete confirmation. That drops any in-progress form state the user has already entered, so the new safety prompt becomes destructive for keyboard users. Please intercept Escape while confirmTarget is set (or temporarily suppress the parent handler) so Escape cancels only this dialog.
Capture Escape while the nested template-delete dialog is open so the parent modal handler does not close the whole flow and discard form data.
|
@mrcfps please check the updates |
lefarcen
added
size/L
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@nehaprasad-dev I re-checked the saved-template delete flow end to end on the latest head: the callback is now wired through the modal, the nested confirmation keeps Escape scoped to the confirm instead of closing New Project, and the updated panel/modal tests cover the success, failure, and keyboard-dismiss paths. Thanks for tightening up both the fix and the follow-up regression coverage.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.3 participants
fix : #2237
why
What users will see
Surface area
[x] UI
[x] i18n keys
Screenshots
Bug fix verification
Validation