From 2c8eb0ca21fbdc9ce77dfa1c04c7c8812c052d4c Mon Sep 17 00:00:00 2001
From: Gabriel Gordon-Hall <gabriel@bloop.ai>
Date: Wed, 10 Sep 2025 09:59:14 +0100
Subject: [PATCH] fix: close GitHub dialog box (vibe-kanban) (#663)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* I reviewed the dialog and fixed its close handling to align with our other NiceModal dialogs.

What I changed
- Dialog onOpenChange: Use hide, not resolve. Replaced `onOpenChange={modal.resolve}` with `onOpenChange={(open) => !open && modal.hide()}` so the X button actually hides the modal.
- Close/Skip buttons: Hide after resolve. Updated all buttons to call `modal.resolve(...); modal.hide();` so clicking Close/Skip reliably closes.
- Auto-close on success: After device flow success, added `modal.hide()` (and return value for resolve) to ensure the dialog closes immediately when login completes.

Why this fixes it
- `modal.resolve()` alone doesn’t necessarily hide the modal; NiceModal’s `hide()` should be called to close. Other dialogs (e.g., ProvidePatDialog) already follow this pattern.

Files touched
- `frontend/src/components/dialogs/auth/GitHubLoginDialog.tsx`

Quick test suggestions
- Start GitHub login, complete authorization. The dialog should auto-close after success.
- If it shows “Successfully connected!”, both the X and Close buttons should now close it.
- Try “Skip” in both pre-login and device-code steps; it should close.

Want me to run a frontend type check or spin up dev to verify the UI flow?

* Implemented the clean fix while keeping the confirmation view.

What I changed
- Dialog close behavior: `onOpenChange` now both resolves and hides, returning true if authenticated and false otherwise.
  - Before: only hid the dialog (or previously only resolved).
  - Now: `onOpenChange={(open) => { if (!open) { modal.resolve(isAuthenticated); modal.hide(); }}}`
- Success flow: Removed auto-resolve/hide on device flow success.
  - Now it just `await reloadSystem()` and switches to the “Successfully connected!” view. The user closes it via the Close button.
- Buttons: Kept previous button fixes.
  - Close (success): `modal.resolve(true); modal.hide();`
  - Skip (intro/device): `modal.resolve(false); modal.hide();`

Why this is correct
- Aligns with NiceModal patterns: close actions call resolve + hide, so any awaiting code proceeds and the modal actually closes.
- Keeps the confirmation view: success doesn’t auto-close; the user now sees “Successfully connected!” and closes it explicitly.

Notes
- App flows that await `NiceModal.show('github-login')` will still proceed once the user closes (X/overlay/Close).
- If you want, I can also cancel polling when the dialog closes to avoid background calls; say the word and I’ll add `setPolling(false)` on close.
---
 .../dialogs/auth/GitHubLoginDialog.tsx        | 32 ++++++++++++++++---
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/frontend/src/components/dialogs/auth/GitHubLoginDialog.tsx b/frontend/src/components/dialogs/auth/GitHubLoginDialog.tsx
index a7950fc0..b1f48437 100644
--- a/frontend/src/components/dialogs/auth/GitHubLoginDialog.tsx
+++ b/frontend/src/components/dialogs/auth/GitHubLoginDialog.tsx
@@ -59,7 +59,6 @@ const GitHubLoginDialog = NiceModal.create(() => {
               setDeviceState(null);
               setError(null);
               await reloadSystem();
-              modal.resolve();
               break;
             case DevicePollStatus.AUTHORIZATION_PENDING:
               timer = setTimeout(poll, deviceState.interval * 1000);
@@ -124,7 +123,15 @@ const GitHubLoginDialog = NiceModal.create(() => {
   };
 
   return (
-    <Dialog open={modal.visible} onOpenChange={modal.resolve}>
+    <Dialog
+      open={modal.visible}
+      onOpenChange={(open) => {
+        if (!open) {
+          modal.resolve(isAuthenticated ? true : false);
+          modal.hide();
+        }
+      }}
+    >
       <DialogContent>
         <DialogHeader>
           <div className="flex items-center gap-3">
@@ -155,7 +162,13 @@ const GitHubLoginDialog = NiceModal.create(() => {
               </CardContent>
             </Card>
             <DialogFooter>
-              <Button onClick={() => modal.resolve()} className="w-full">
+              <Button
+                onClick={() => {
+                  modal.resolve(true);
+                  modal.hide();
+                }}
+                className="w-full"
+              >
                 Close
               </Button>
             </DialogFooter>
@@ -229,7 +242,13 @@ const GitHubLoginDialog = NiceModal.create(() => {
             )}
 
             <DialogFooter>
-              <Button variant="outline" onClick={() => modal.resolve()}>
+              <Button
+                variant="outline"
+                onClick={() => {
+                  modal.resolve(false);
+                  modal.hide();
+                }}
+              >
                 Skip
               </Button>
             </DialogFooter>
@@ -282,7 +301,10 @@ const GitHubLoginDialog = NiceModal.create(() => {
             <DialogFooter className="gap-3 flex-col sm:flex-row">
               <Button
                 variant="outline"
-                onClick={() => modal.resolve()}
+                onClick={() => {
+                  modal.resolve(false);
+                  modal.hide();
+                }}
                 className="flex-1"
               >
                 Skip