Add Confirmation Pop-up to Prevent Accidental Exception Deletions #10283
Conversation
WalkthroughThe pull request implements confirmation dialogs for deleting schedule exceptions, removing availability entries, and deleting schedule templates across various components. The previous direct delete actions are replaced with dialogs that require user confirmation before proceeding. Additionally, new localization keys are added to support confirmation messages, enhancing user interaction and preventing accidental deletions. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.
|
Jacobjeevan
added
the
waiting for related PR
|
@modamaan what is the status on the PR |
|
@rithviknishad #10048 Has work started on the ConfirmDialog? |
|
@Jacobjeevan, can we use the existing |
Jacobjeevan
removed
question
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx (3)
141-141: Consider using consistent state variable naming.The state variables for managing dialog visibility use different naming patterns:
isDialogOpenin ScheduleTemplateEditoropenDialogin AvailabilityEditorConsider using consistent naming across both components for better maintainability.
- const [openDialog, setOpenDialog] = useState(false); + const [isDialogOpen, setDialogOpen] = useState(false);Also applies to: 332-332
266-302: Consider disabling the confirm button during deletion.The confirm button should be disabled while deletion is in progress to prevent accidental double-clicks.
<AlertDialogAction + disabled={isDeleting} onClick={() => { deleteTemplate(); setDialogOpen(false); }} > {t("confirm")} </AlertDialogAction>
395-431: Remove redundant onClick handler in AlertDialogTrigger.The
onClickhandler in the trigger button is redundant asAlertDialogTriggeralready handles opening the dialog.<Button variant="ghost" size="icon" - onClick={() => setOpenDialog(true)} disabled={isDeleting} className="text-red-600 hover:text-red-700 hover:bg-red-50" >Also, consider disabling the confirm button during deletion to prevent double-clicks:
<AlertDialogAction + disabled={isDeleting} onClick={() => { deleteAvailability(); setOpenDialog(false); }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
public/locale/en.json(2 hunks)src/pages/Scheduling/ScheduleExceptions.tsx(4 hunks)src/pages/Scheduling/components/CreateScheduleTemplateSheet.tsx(4 hunks)src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/Scheduling/components/CreateScheduleTemplateSheet.tsx
- src/pages/Scheduling/ScheduleExceptions.tsx
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx (1)
18-29: LGTM!The new imports for Alert and AlertDialog components are well-organized and necessary for implementing the confirmation dialogs.
District.Admin._.confirm-Popup.Chrome.2025-02-04.21-48-48.mp4 |
…modamaan/care_feAmaan into issues/10259/confirmation-popup
| type="button" | ||
| variant="secondary" | ||
| size="sm" | ||
| className="text-gray-600 hover:text-gray-900" |
There was a problem hiding this comment.
let's not modify the colors, also use danger variant for destructive actions
| className="text-gray-600 hover:text-gray-900" |
| variant="ghost" | ||
| size="icon" | ||
| onClick={() => setOpenDialog(true)} | ||
| disabled={isDeleting} | ||
| className="text-red-600 hover:text-red-700 hover:bg-red-50" |
There was a problem hiding this comment.
let's not modify classNames of buttons. looses the point of having variants and consistent designs
Proposed Changes
Vinu.TV._.CARE.-.confirm.dialog.2025-01-31.01-44-05.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes