fix: replace window.confirm with ConfirmDialog component (#997)

[Xenon010101]

Description\nReplaces `window.confirm` with the existing `ConfirmDialog` component for a consistent design experience when resetting the roadmap.\n\n## Changes\n- Added `showResetConfirm` state to control dialog visibility\n- Imported `ConfirmDialog` component\n- Removed native `window.confirm` call\n\nCloses #997

Summary by CodeRabbit

• Refactor
  • Updated the confirmation experience when resetting roadmap progress with an improved dialog interface.

[github-actions]

Hi @Xenon010101, thanks for contributing to InternHack! 🎉

I have automatically:

• 👤 Assigned this PR to you.
• 🏷️ Applied the gssoc:approved label.

Our workflows will now analyze your changes to classify:

• 📈 PR Difficulty: level:*
• 🧩 PR Type: type:*
• 🌟 PR Quality: quality:*

Tip

Ensure your PR description references the issue it resolves (e.g. Closes #123). This allows the bot to inherit any additional labels from that issue!

Happy coding! 🚀

[coderabbitai]

📝 Walkthrough

Walkthrough

FirstPRRoadmapPage replaces the native browser window.confirm with a custom ConfirmDialog component when users attempt to reset their completed roadmap steps, introducing state management to control dialog visibility and refactoring the reset logic into dialog handlers.

Changes

ConfirmDialog-driven reset flow

Layer / File(s)	Summary
Replace native confirm with ConfirmDialog for reset flow client/src/module/student/opensource/FirstPRRoadmapPage.tsx	ConfirmDialog import is added, showResetConfirm state tracks dialog visibility, and the "Start over" button now opens the dialog; onConfirm clears the completed set and removes persisted progress from localStorage, while onCancel simply closes the dialog.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Sachinchaurasiya360/InternHack#204: Introduces the reusable ConfirmDialog component that this PR uses to replace the native window.confirm for step reset confirmation.

Suggested labels

good first issue, level:beginner, type:feature

Poem

🐰 A dialog so styled, no more native pop,
The roadmap reset now has modern flair,
Confirmed with grace, the progress falls away,
Design consistency at last takes care!
🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: replacing window.confirm with ConfirmDialog component for better design consistency.
Description check	✅ Passed	The description covers the key changes and includes the related issue. However, it lacks detail on testing methodology and does not include screenshots/UI evidence as required by the template.
Linked Issues check	✅ Passed	The PR successfully replaces window.confirm with ConfirmDialog component as required by #997, implementing the custom confirmation flow using existing UI patterns.
Out of Scope Changes check	✅ Passed	All changes are scoped to replacing window.confirm with ConfirmDialog in FirstPRRoadmapPage.tsx. No unrelated modifications or scope creep detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

client/src/module/student/opensource/FirstPRRoadmapPage.tsx (1)

163-175: ⚡ Quick win

Consider moving ConfirmDialog outside the AnimatePresence block.

The ConfirmDialog is currently nested inside the AnimatePresence block (lines 138–180) that unmounts when allDone becomes false. When the user confirms the reset, both setCompleted(new Set()) and setShowResetConfirm(false) execute together, causing the parent banner and dialog to unmount simultaneously.

While this doesn't cause functional issues, best practice is to render dialogs at the page level (outside conditional/animated blocks) to keep their lifecycle independent of the content they're confirming.

♻️ Recommended placement

Move the ConfirmDialog outside the AnimatePresence block to the page level, for example right before the closing </div> at line 275:

       </div>
+
+      <ConfirmDialog
+        open={showResetConfirm}
+        title="Reset all steps?"
+        description="This will remove your progress and reset all completed steps."
+        confirmLabel="Reset"
+        cancelLabel="Cancel"
+        onConfirm={() => {
+          setCompleted(new Set());
+          try { localStorage.removeItem(STORAGE_KEY); } catch { /**/ }
+          setShowResetConfirm(false);
+        }}
+        onCancel={() => setShowResetConfirm(false)}
+      />
     </div>
   );
 }

And remove it from inside the completion banner:

               <button
                 onClick={() => setShowResetConfirm(true)}
                 className="text-sm text-lime-700 dark:text-lime-400 border border-lime-400 px-3 py-0.5 rounded-lg font-medium"
               >
                 Start over
               </button>
-              <ConfirmDialog
-                open={showResetConfirm}
-                title="Reset all steps?"
-                description="This will remove your progress and reset all completed steps."
-                confirmLabel="Reset"
-                cancelLabel="Cancel"
-                onConfirm={() => {
-                  setCompleted(new Set());
-                  try { localStorage.removeItem(STORAGE_KEY); } catch { /**/ }
-                  setShowResetConfirm(false);
-                }}
-                onCancel={() => setShowResetConfirm(false)}
-              />
             </div>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@client/src/module/student/opensource/FirstPRRoadmapPage.tsx` around lines 163
- 175, The ConfirmDialog is rendered inside the AnimatePresence block so it
unmounts together with the banner; move the ConfirmDialog JSX out of the
AnimatePresence block to the page-level container (render it alongside the main
page <div> rather than inside the banner), keeping the same props and handlers
(open={showResetConfirm}, onConfirm that calls setCompleted(new Set()),
localStorage.removeItem(STORAGE_KEY) and setShowResetConfirm(false), and
onCancel={() => setShowResetConfirm(false)}); remove the in-banner instance so
the dialog's lifecycle is independent of allDone toggles and banner unmounting.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@client/src/module/student/opensource/FirstPRRoadmapPage.tsx`:
- Around line 163-175: The ConfirmDialog is rendered inside the AnimatePresence
block so it unmounts together with the banner; move the ConfirmDialog JSX out of
the AnimatePresence block to the page-level container (render it alongside the
main page <div> rather than inside the banner), keeping the same props and
handlers (open={showResetConfirm}, onConfirm that calls setCompleted(new Set()),
localStorage.removeItem(STORAGE_KEY) and setShowResetConfirm(false), and
onCancel={() => setShowResetConfirm(false)}); remove the in-banner instance so
the dialog's lifecycle is independent of allDone toggles and banner unmounting.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81f138d6-916e-4f60-9b28-c6b5194b619e

📥 Commits

Reviewing files that changed from the base of the PR and between e15a36c and bbd11aa.

📒 Files selected for processing (1)

• client/src/module/student/opensource/FirstPRRoadmapPage.tsx

[Sachinchaurasiya360]

Code Review — PR #1410: Replace window.confirm with ConfirmDialog

Hi @Xenon010101, the ConfirmDialog enhancements (Escape key, loading state, confirmVariant, children) are well implemented. One coordination issue to flag.

---

🟡 Merge conflict: PRs #1405, #1406, #1409, and this PR all apply identical changes to ConfirmDialog.tsx

All four PRs modify ConfirmDialog.tsx with the exact same diff (Escape key listener, loading prop, confirmVariant prop, children support, description made optional). Whichever merges second, third, and fourth will have conflicts on this file.

The cleanest resolution: merge this PR (or whichever improves the base component) first, then the other three PRs only need to rebase and drop the ConfirmDialog.tsx changes from their diff since the base already has them.

Please coordinate with @YAXH64 on PRs #1405, #1406, and #1409 before merging.

---

✅ Everything else looks good

• Escape key handler is properly cleaned up via removeEventListener in the effect return
• loading ? undefined : onCancel on the backdrop click correctly prevents dismiss while loading
• Focus on cancelRef is correct — default focus should be on the safe/cancel action
• role="alertdialog" with aria-modal and aria-labelledby/aria-describedby is proper accessibility