🎨 Palette: Replace confirm dialog with AlertDialog#87
🎨 Palette: Replace confirm dialog with AlertDialog#87Krosebrook wants to merge 1 commit intomainfrom
Conversation
- Replaced `window.confirm` with `@/components/ui/alert-dialog` for song deletion confirmation.
- Added `aria-label` to the delete button ("Delete song") and share button ("Share song").
- Improved accessibility and visual consistency with the design system.
Co-authored-by: Krosebrook <214532761+Krosebrook@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
✨ 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 |
Summary of ChangesHello @Krosebrook, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the user experience and accessibility on the Song Details page. It transitions from using a generic browser confirmation prompt to a custom Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/pages/SongDetails.tsx">
<violation number="1" location="client/src/pages/SongDetails.tsx:210">
P2: Disable the AlertDialog delete action while the delete mutation is pending to prevent duplicate delete requests from rapid double-clicks.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| </AlertDialogHeader> | ||
| <AlertDialogFooter> | ||
| <AlertDialogCancel>Cancel</AlertDialogCancel> | ||
| <AlertDialogAction onClick={handleDelete} className="bg-destructive text-destructive-foreground hover:bg-destructive/90">Delete</AlertDialogAction> |
There was a problem hiding this comment.
P2: Disable the AlertDialog delete action while the delete mutation is pending to prevent duplicate delete requests from rapid double-clicks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/pages/SongDetails.tsx, line 210:
<comment>Disable the AlertDialog delete action while the delete mutation is pending to prevent duplicate delete requests from rapid double-clicks.</comment>
<file context>
@@ -174,16 +184,33 @@ export default function SongDetails() {
+ </AlertDialogHeader>
+ <AlertDialogFooter>
+ <AlertDialogCancel>Cancel</AlertDialogCancel>
+ <AlertDialogAction onClick={handleDelete} className="bg-destructive text-destructive-foreground hover:bg-destructive/90">Delete</AlertDialogAction>
+ </AlertDialogFooter>
+ </AlertDialogContent>
</file context>
| <AlertDialogAction onClick={handleDelete} className="bg-destructive text-destructive-foreground hover:bg-destructive/90">Delete</AlertDialogAction> | |
| <AlertDialogAction onClick={handleDelete} disabled={isDeleting} className="bg-destructive text-destructive-foreground hover:bg-destructive/90">Delete</AlertDialogAction> |
There was a problem hiding this comment.
Pull request overview
This PR updates the Song Details page UX by replacing the native browser confirm() delete prompt with the app’s Radix/shadcn AlertDialog, and adds accessibility labels to icon-only actions.
Changes:
- Replaced the delete confirmation
confirm()flow with anAlertDialogconfirmation modal. - Added
aria-labelto the Share icon button (and to the Delete icon button in the new dialog trigger).
Comments suppressed due to low confidence (1)
client/src/pages/SongDetails.tsx:165
- This icon-only button now has an
aria-label, but the decorative icon should be markedaria-hidden="true"to avoid screen readers announcing both the label and the icon glyph/name.
aria-label="Share song"
data-testid="button-share"
>
<Share2 className="w-4 h-4 md:w-5 md:h-5" />
</Button>
| <AlertDialogCancel>Cancel</AlertDialogCancel> | ||
| <AlertDialogAction onClick={handleDelete} className="bg-destructive text-destructive-foreground hover:bg-destructive/90">Delete</AlertDialogAction> |
There was a problem hiding this comment.
AlertDialogAction remains clickable while the delete mutation is pending. Since handleDelete doesn’t guard against repeated calls, users can trigger multiple delete requests by clicking "Delete" multiple times after the dialog opens. Disable the action (and optionally cancel) while isDeleting is true, and/or add a guard in handleDelete to no-op when already pending.
| <AlertDialogCancel>Cancel</AlertDialogCancel> | |
| <AlertDialogAction onClick={handleDelete} className="bg-destructive text-destructive-foreground hover:bg-destructive/90">Delete</AlertDialogAction> | |
| <AlertDialogCancel disabled={isDeleting}>Cancel</AlertDialogCancel> | |
| <AlertDialogAction | |
| onClick={isDeleting ? undefined : handleDelete} | |
| disabled={isDeleting} | |
| className="bg-destructive text-destructive-foreground hover:bg-destructive/90" | |
| > | |
| {isDeleting ? "Deleting..." : "Delete"} | |
| </AlertDialogAction> |
| aria-label="Delete song" | ||
| data-testid="button-delete-song" | ||
| > | ||
| <Trash2 className="w-4 h-4 md:w-5 md:h-5" /> |
There was a problem hiding this comment.
This icon-only destructive button has an aria-label, but the Trash2 icon should be aria-hidden="true" so assistive tech doesn’t announce the icon in addition to the button label.
| <Trash2 className="w-4 h-4 md:w-5 md:h-5" /> | |
| <Trash2 className="w-4 h-4 md:w-5 md:h-5" aria-hidden="true" /> |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully replaces the native browser confirmation dialog with a custom AlertDialog component, improving the user experience and visual consistency. Additionally, aria-label attributes have been added to icon-only buttons, which is a good step towards improving accessibility. The changes are well-implemented and align with modern web development practices.
I am having trouble creating individual review comments. Click here to see my feedback.
client/src/pages/SongDetails.tsx (44-50)
The if (confirm("Are you sure? This cannot be undone.")) block has been removed, which is correct given the introduction of the AlertDialog. However, it's important to ensure that the handleDelete function is only called when the user explicitly confirms the deletion in the new AlertDialog to prevent accidental deletions.
client/src/pages/SongDetails.tsx (161)
Adding aria-label="Share song" is a good practice for accessibility, especially for icon-only buttons. This helps screen readers convey the purpose of the button to users.
client/src/pages/SongDetails.tsx (195)
The aria-label="Delete song" attribute is correctly added to the delete button, enhancing accessibility for screen reader users by providing a clear description of the button's action.
client/src/pages/SongDetails.tsx (205)
The AlertDialogDescription correctly uses song.title to provide a more specific and user-friendly confirmation message. This improves the user experience by clearly stating which song will be deleted.
client/src/pages/SongDetails.tsx (210)
The AlertDialogAction for deletion correctly calls handleDelete when confirmed. The styling className="bg-destructive text-destructive-foreground hover:bg-destructive/90" also visually reinforces the destructive nature of the action, which is good for UX.
This change improves the UX of the Song Details page by replacing the native browser confirmation dialog with a custom
AlertDialogcomponent. This provides a more integrated and visually consistent experience. Additionally,aria-labelattributes were added to icon-only buttons to improve accessibility for screen readers.Verified with Playwright (mocked backend) and static analysis (
pnpm check).PR created automatically by Jules for task 5101894663903765602 started by @Krosebrook
Summary by cubic
Replaced the native confirm on the Song Details page with the design-system AlertDialog for delete confirmation. Added aria-labels to the Share and Delete icon buttons to improve accessibility.
Written for commit 59fd886. Summary will update on new commits.