fix: close GitHub dialog box (vibe-kanban) (#663)

* 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.
This commit is contained in:
Gabriel Gordon-Hall
2025-09-10 09:59:14 +01:00
committed by GitHub
parent d87f6d7113
commit 2c8eb0ca21

View File

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