fix(chat): correct delete-message dialog dismiss race and restore destructive styling - #2035
Merged
Conversation
…tructive styling - AlertDialogAction: accept optional variant so destructive confirm buttons render with danger styling - ChatPage delete dialog: use variant="destructive" on confirm, @click.capture to beat DialogClose event ordering - MessageToolbar: give the trash button destructive/70 text and destructive hover palette - useMessageActions: expose toast/t to the composable; show a destructive error toast when deleteMessage rejects instead of silent console.error - Test: cover the delete-failure toast path in useMessageActions.test.ts
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughMessage deletion now uses destructive confirmation styling, capture-phase click handling, and localized destructive toasts when deletion fails in writable session views. ChangesMessage deletion flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ChatPage
participant useMessageActions
participant sessionClient
participant toast
participant t
ChatPage->>useMessageActions: confirmMessageDelete()
useMessageActions->>sessionClient: deleteMessage()
sessionClient-->>useMessageActions: deletion error
useMessageActions->>t: translate delete error strings
t-->>useMessageActions: localized title and description
useMessageActions->>toast: show destructive error toast
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Collaborator
|
LGTM. |
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
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.
Summary
Fixes two long-standing UX issues with the message delete action:
Dialog dismiss race —
AlertDialogActionwraps reka-uiDialogClosewhich callsrootContext.onOpenChange(false)on click, closing the dialog. Our@clickhandler ran after that close, by which timependingDeleteMessageIdwas already cleared, soconfirmMessageDeletereturned early and no deletion happened.Destructive styling — The delete confirm button used default primary colors; the trash icon in the toolbar used muted-foreground. Both now use the destructive color palette.
Changes
AlertDialogAction.vuevariantprop, forwarded tobuttonVariants({ variant })ChatPage.vuevariant="destructive"+@click.captureto beat DialogClose event orderingMessageToolbar.vuetext-destructive/70 hover:text-destructive hover:bg-destructive/10useMessageActions.tstoast/toptions; show destructive error toast on delete failure instead of silent console.erroruseMessageActions.test.tsVerification
pnpm exec vitest run test/renderer/features/chat-page/composables/useMessageActions.test.ts test/renderer/components/ChatPage.test.ts— 99 passedpnpm run format/pnpm run i18n/pnpm run lint— all cleanSummary by CodeRabbit
New Features
Bug Fixes
Tests