From 560c1e2d4cbff24217bf99e4f17ed1fe7074dc09 Mon Sep 17 00:00:00 2001 From: Kieran Klukas Date: Mon, 15 Jun 2026 17:44:42 -0400 Subject: [PATCH] feat(dialog): track last closed dialog in open with grace --- internal/ui/dialog/dialog.go | 27 ++++++++++++++ internal/ui/dialog/overlay_test.go | 58 ++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/internal/ui/dialog/dialog.go b/internal/ui/dialog/dialog.go index dc5ce3913d..0b82784866 100644 --- a/internal/ui/dialog/dialog.go +++ b/internal/ui/dialog/dialog.go @@ -69,6 +69,13 @@ type Overlay struct { // dialog was opened via OpenDialogWithGrace. graceOpenedAt time.Time graceLastInputAt time.Time + + // Track recently closed dialog IDs so that reopening the same + // dialog type can skip the grace period. This prevents rapid + // successive dialogs (e.g. multiple permission prompts) from + // each eating a keystroke. + lastClosedID string + lastClosedAt time.Time } // NewOverlay creates a new [Overlay] instance. @@ -106,9 +113,21 @@ func (d *Overlay) OpenDialog(dialog Dialog) { // comes first. Use this for dialogs that open asynchronously (e.g. // permission prompts) where in-flight keystrokes from a previously // focused component could act on the dialog before the user sees it. +// +// If the same dialog ID was just closed (within reopenGraceWindow), +// the grace period is skipped — the user is already focused on this +// dialog type and rapid successive prompts should not eat keystrokes. func (d *Overlay) OpenDialogWithGrace(dialog Dialog) { now := time.Now() d.dialogs = append(d.dialogs, dialog) + + // Skip grace when reopening the same dialog type immediately. + if dialog.ID() == d.lastClosedID && now.Sub(d.lastClosedAt) < reopenGraceWindow { + d.graceOpenedAt = time.Time{} + d.graceLastInputAt = time.Time{} + return + } + d.graceOpenedAt = now d.graceLastInputAt = now } @@ -146,7 +165,15 @@ func (d *Overlay) CloseFrontDialog() { d.removeDialog(len(d.dialogs) - 1) } +// reopenGraceWindow is how long after closing a dialog we consider +// a reopen of the same dialog ID to be "immediate" and skip grace. +const reopenGraceWindow = 500 * time.Millisecond + func (d *Overlay) removeDialog(idx int) { + if idx == len(d.dialogs)-1 { + d.lastClosedID = d.dialogs[idx].ID() + d.lastClosedAt = time.Now() + } d.dialogs = append(d.dialogs[:idx], d.dialogs[idx+1:]...) // Clear grace state when the front dialog changes. if idx == len(d.dialogs) { diff --git a/internal/ui/dialog/overlay_test.go b/internal/ui/dialog/overlay_test.go index a610a11220..e581860aa3 100644 --- a/internal/ui/dialog/overlay_test.go +++ b/internal/ui/dialog/overlay_test.go @@ -129,3 +129,61 @@ func TestOverlay_NonKeyMessagesPassDuringGrace(t *testing.T) { o.Update(tea.MouseWheelMsg{}) require.Len(t, d.received, 1, "non-key messages should pass through during grace") } + +// TestOverlay_ReopenSameDialogSkipsGrace verifies that when a dialog is +// closed and immediately reopened with the same ID (e.g. successive +// permission prompts), the grace period is skipped so the user can +// continue interacting without lost keystrokes. +func TestOverlay_ReopenSameDialogSkipsGrace(t *testing.T) { + t.Parallel() + + d1 := &stubDialog{id: "permissions"} + o := NewOverlay() + o.OpenDialogWithGrace(d1) + + // Close and immediately reopen with the same ID. + o.CloseDialog("permissions") + d2 := &stubDialog{id: "permissions"} + o.OpenDialogWithGrace(d2) + + // Keys should reach the new dialog immediately — no grace. + o.Update(keyMsg('a')) + require.Len(t, d2.received, 1, "reopened dialog with same ID should skip grace") +} + +// TestOverlay_ReopenDifferentDialogKeepsGrace verifies that reopening a +// different dialog ID still gets the normal grace period. +func TestOverlay_ReopenDifferentDialogKeepsGrace(t *testing.T) { + t.Parallel() + + d1 := &stubDialog{id: "permissions"} + o := NewOverlay() + o.OpenDialogWithGrace(d1) + o.CloseDialog("permissions") + + d2 := &stubDialog{id: "other"} + o.OpenDialogWithGrace(d2) + + o.Update(keyMsg('a')) + require.Empty(t, d2.received, "different dialog ID should still have grace period") +} + +// TestOverlay_ReopenAfterWindowExpiresKeepsGrace verifies that if enough +// time passes between close and reopen, the grace period applies normally. +func TestOverlay_ReopenAfterWindowExpiresKeepsGrace(t *testing.T) { + t.Parallel() + + d1 := &stubDialog{id: "permissions"} + o := NewOverlay() + o.OpenDialogWithGrace(d1) + o.CloseDialog("permissions") + + // Backdate the close time so it falls outside the reopen window. + o.lastClosedAt = time.Now().Add(-reopenGraceWindow - time.Millisecond) + + d2 := &stubDialog{id: "permissions"} + o.OpenDialogWithGrace(d2) + + o.Update(keyMsg('a')) + require.Empty(t, d2.received, "reopened dialog after window expires should have grace") +}