diff --git a/gova.go b/gova.go index e4755b8..b068f2f 100644 --- a/gova.go +++ b/gova.go @@ -50,12 +50,35 @@ func provideOverlays(s *Scope, a fyne.App, w fyne.Window, bridge *fyneBridge.Fyn fyneBridge.ShowAlert(w, title, message, specs) }) }, - showSheet: func(content View, onDismiss func()) { + showSheet: func(content View, onDismiss func()) func() { spec := toSpec(content.viewNode()) + var ( + mu sync.Mutex + hideFn func() + canceled bool + ) fyne.Do(func() { mounted := bridge.Mount(spec) - fyneBridge.ShowSheet(w, mounted.Object, onDismiss) + hide := fyneBridge.ShowSheet(w, mounted.Object, onDismiss) + mu.Lock() + if canceled { + mu.Unlock() + hide() + return + } + hideFn = hide + mu.Unlock() }) + return func() { + mu.Lock() + canceled = true + h := hideFn + hideFn = nil + mu.Unlock() + if h != nil { + fyne.Do(h) + } + } }, setTheme: func(t *Theme) { tc := fyneBridge.ThemeConfig{ diff --git a/internal/fyne/overlay.go b/internal/fyne/overlay.go index 27e0234..7d0fa1c 100644 --- a/internal/fyne/overlay.go +++ b/internal/fyne/overlay.go @@ -43,12 +43,13 @@ func ShowAlert(win fyne.Window, title, message string, actions []AlertActionSpec d.Show() } -func ShowSheet(win fyne.Window, content fyne.CanvasObject, onDismiss func()) { +func ShowSheet(win fyne.Window, content fyne.CanvasObject, onDismiss func()) func() { wrapped := container.New(layout.NewCustomPaddedLayout(10, 10, 10, 10), content) d := dialog.NewCustom("", "Close", wrapped, win) d.SetOnClosed(onDismiss) d.Resize(fyne.NewSize(400, 300)) d.Show() + return d.Hide } type AlertActionSpec struct { diff --git a/overlay.go b/overlay.go index 0fdb622..dd73c59 100644 --- a/overlay.go +++ b/overlay.go @@ -1,9 +1,11 @@ package gova +import "sync" + type ActionStyle int const ( - ActionDefault ActionStyle = iota + ActionDefault ActionStyle = iota ActionCancel ActionDestructive ) @@ -20,7 +22,7 @@ var overlayStoreKey = &StoreKey[*overlayFuncs]{Default: nil} type overlayFuncs struct { showAlert func(title, message string, actions []AlertAction) - showSheet func(content View, onDismiss func()) + showSheet func(content View, onDismiss func()) func() setTheme func(theme *Theme) } @@ -37,19 +39,40 @@ func UseAlert(s *Scope) func(title, message string, actions ...AlertAction) { } // UseSheet returns (show, dismiss) functions for presenting a sheet overlay. +// dismiss programmatically closes the most recently shown sheet; it is a +// no-op if no sheet is currently presented (either never shown, already +// dismissed by the user, or already dismissed programmatically). func UseSheet(s *Scope) (show func(content View), dismiss func()) { store := UseStore(s, overlayStoreKey) fns := store.Get() - var dismissFn func() + var ( + mu sync.Mutex + cancel func() + ) show = func(content View) { - if fns != nil && fns.showSheet != nil { - fns.showSheet(content, func() { dismissFn = nil }) + if fns == nil || fns.showSheet == nil { + return } + // onDismiss fires when the sheet is closed by any path + // (user-driven Close button or programmatic cancel) so the + // next dismiss() call doesn't try to cancel a dead sheet. + c := fns.showSheet(content, func() { + mu.Lock() + cancel = nil + mu.Unlock() + }) + mu.Lock() + cancel = c + mu.Unlock() } dismiss = func() { - if dismissFn != nil { - dismissFn() + mu.Lock() + c := cancel + cancel = nil + mu.Unlock() + if c != nil { + c() } } return show, dismiss diff --git a/overlay_test.go b/overlay_test.go new file mode 100644 index 0000000..fd42e0f --- /dev/null +++ b/overlay_test.go @@ -0,0 +1,130 @@ +package gova + +import ( + "context" + "testing" +) + +// fakeOverlay records calls and exercises the cancel-handle contract that +// the real Fyne-backed overlayFuncs implement. Each call to showSheet +// returns a fresh cancel closure; invoking it counts toward cancelCount +// and fires the onDismiss callback (mirroring Fyne's CustomDialog, which +// calls SetOnClosed on programmatic Hide). +type fakeOverlay struct { + showCount int + cancelCount int + lastOnDismiss func() +} + +func (f *fakeOverlay) funcs() *overlayFuncs { + return &overlayFuncs{ + showSheet: func(content View, onDismiss func()) func() { + f.showCount++ + f.lastOnDismiss = onDismiss + closed := false + return func() { + if closed { + return + } + closed = true + f.cancelCount++ + if onDismiss != nil { + onDismiss() + } + } + }, + } +} + +func TestUseSheetDismissInvokesCancel(t *testing.T) { + scope := newScope(context.Background(), nil) + fake := &fakeOverlay{} + Provide(scope, overlayStoreKey, fake.funcs()) + + show, dismiss := UseSheet(scope) + + show(Text("first")) + if fake.showCount != 1 { + t.Fatalf("expected 1 showSheet call, got %d", fake.showCount) + } + + dismiss() + if fake.cancelCount != 1 { + t.Fatalf("dismiss did not invoke cancel hook: cancelCount=%d (UseSheet must wire dismiss to the cancel closure returned by showSheet)", fake.cancelCount) + } +} + +func TestUseSheetDismissBeforeShowIsNoOp(t *testing.T) { + scope := newScope(context.Background(), nil) + fake := &fakeOverlay{} + Provide(scope, overlayStoreKey, fake.funcs()) + + _, dismiss := UseSheet(scope) + // Calling dismiss before any show should not panic and should not + // invoke any cancel hook (there is nothing to cancel). + dismiss() + if fake.cancelCount != 0 { + t.Fatalf("dismiss before show should be a no-op; cancelCount=%d", fake.cancelCount) + } +} + +func TestUseSheetDismissIsIdempotent(t *testing.T) { + scope := newScope(context.Background(), nil) + fake := &fakeOverlay{} + Provide(scope, overlayStoreKey, fake.funcs()) + + show, dismiss := UseSheet(scope) + show(Text("x")) + dismiss() + dismiss() + dismiss() + + if fake.cancelCount != 1 { + t.Fatalf("repeated dismiss should not re-cancel: cancelCount=%d", fake.cancelCount) + } +} + +func TestUseSheetUserCloseClearsHandle(t *testing.T) { + // Simulates the user clicking the sheet's Close button: Fyne fires + // the onDismiss callback directly, without going through the cancel + // hook. A subsequent programmatic dismiss() must then be a no-op. + scope := newScope(context.Background(), nil) + fake := &fakeOverlay{} + Provide(scope, overlayStoreKey, fake.funcs()) + + show, dismiss := UseSheet(scope) + show(Text("x")) + + if fake.lastOnDismiss == nil { + t.Fatal("expected showSheet to receive an onDismiss callback") + } + fake.lastOnDismiss() // user closed the sheet themselves + + dismiss() + if fake.cancelCount != 0 { + t.Fatalf("dismiss after user-close should be no-op; cancelCount=%d", fake.cancelCount) + } +} + +func TestUseSheetReshowGetsFreshCancel(t *testing.T) { + scope := newScope(context.Background(), nil) + fake := &fakeOverlay{} + Provide(scope, overlayStoreKey, fake.funcs()) + + show, dismiss := UseSheet(scope) + + show(Text("first")) + dismiss() + if fake.cancelCount != 1 { + t.Fatalf("first dismiss did not cancel: cancelCount=%d", fake.cancelCount) + } + + show(Text("second")) + if fake.showCount != 2 { + t.Fatalf("expected 2 showSheet calls after reshow; got %d", fake.showCount) + } + dismiss() + if fake.cancelCount != 2 { + t.Fatalf("dismiss after reshow did not cancel new sheet: cancelCount=%d", fake.cancelCount) + } +}