From 36a08284c4ac0a0801e941cc88970d2d32299acb Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 5 Oct 2026 15:36:19 +0200 Subject: [PATCH] fix(BindCommand): disable the button while its command cannot execute BindCommand reflected CanExecute only by swapping the button's Style, on the stated ground that a Button has no disabled flag. It has one now (Base.Disabled), and the swap alone left a button whose command cannot run taking clicks, Enter and keyboard focus; the host's own OnClick handler, composed before Execute, still ran. Disabled is now bound to !CanExecute, beside the unchanged Style greying, and unbind restores OnClick, Style and the prior Disabled state. Fixes #31 Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 10 ++++---- bind.go | 23 +++++++++++++------ bind_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 217d499..79ffad0 100644 --- a/README.md +++ b/README.md @@ -41,12 +41,14 @@ field and callback names filled in — no business logic. Every helper returns a | `BindListItems[T](*ListBox, *ObservableList[T], project, invalidate)` | `Items` | list → widget | | `BindDropDownOptions[T](*DropDown, *ObservableList[T], project, invalidate)` | `Options` | list → widget | | `BindViews[T](*ViewSwitcher, *ObservableList[T], project, invalidate)` | `Views` | list → widget | -| `BindCommand(*Button, *Command, invalidate)` | `OnClick` + `Style` greying | command | +| `BindCommand(*Button, *Command, invalidate)` | `OnClick` + `Disabled` + `Style` greying | command | | `BindTree[T](*TreeTable, *ObservableList[T], project, invalidate)` | `Root` forest | list → widget | -`BindCommand` reflects `CanExecute` by greying the button — swapping its `Style` -to `ButtonSecondary` when the command cannot execute and restoring the original -`Style` when it can — because a `Button` has no boolean "disabled" field. +`BindCommand` binds the button's `Disabled` state to `!CanExecute`: a button whose +command cannot run takes no click, no Enter/Space and no keyboard focus. It also +greys the button as it always has — swapping its `Style` to `ButtonSecondary` +when the command cannot execute and restoring the original `Style` when it can. +The returned unbind puts `OnClick`, `Style` and `Disabled` back as they were. `BindTree` rebuilds a `TreeTable.Root` (`[]*TreeTableNode` forest) from the list; the caller's `project` owns each node's `Cells`/`Children`. diff --git a/bind.go b/bind.go index f6c6fd9..d738133 100644 --- a/bind.go +++ b/bind.go @@ -241,25 +241,34 @@ func BindViews[T any](v *toolkit.ViewSwitcher, l *mvvm.ObservableList[T], projec // ── Command binding ───────────────────────────────────────────────────────── // BindCommand wires a Command to a Button: it composes Execute into the -// button's OnClick and reflects executability by greying the button — swapping -// its Style to ButtonSecondary when the command cannot execute and restoring -// the original Style when it can (a Button has no boolean "disabled" field, so -// this backend-specific greying is how CanExecute surfaces). invalidate (may be -// nil) requests a repaint on each executability change. The returned unbind -// restores the prior OnClick and detaches. +// button's OnClick and binds the button's Disabled state to !CanExecute, so a +// button whose command cannot run neither fires on a click or Enter nor takes +// keyboard focus (the toolkit's focus walk skips disabled widgets). It also +// keeps the greying it has always applied -- the Style swaps to +// ButtonSecondary while the command cannot execute and back to the original +// Style when it can -- so a bound button looks exactly as before. invalidate +// (may be nil) requests a repaint on each executability change. The returned +// unbind restores the prior OnClick, Style and Disabled state and detaches. func BindCommand(b *toolkit.Button, c *mvvm.Command, invalidate func()) (unbind func()) { orig := b.Style + origDisabled := b.Disabled().Get() setEnabled := func(enabled bool) { if enabled { b.Style = orig } else { b.Style = toolkit.ButtonSecondary } + b.Disabled().Set(!enabled) if invalidate != nil { invalidate() } } - return mvvm.BindCommand(c, &b.OnClick, setEnabled) + detach := mvvm.BindCommand(c, &b.OnClick, setEnabled) + return func() { + detach() + b.Style = orig + b.Disabled().Set(origDisabled) + } } // ── Tree binding (ObservableList → TreeTable forest) ──────────────────────── diff --git a/bind_test.go b/bind_test.go index 2e58e56..4338d82 100644 --- a/bind_test.go +++ b/bind_test.go @@ -377,3 +377,68 @@ func TestBindTree(t *testing.T) { } u2() } + +// TestBindCommandDisablesTheButton: a button whose command cannot execute is +// really disabled, not merely restyled -- a click and Enter do not reach +// OnClick, and Tab does not focus it -- and comes back when the command can run +// again. Before #31 only the Style changed, so the button still took clicks +// (which Execute then ignored, but the host's own handler ran) and focus. +func TestBindCommandDisablesTheButton(t *testing.T) { + allowed := mvvm.NewObservable(false) + runs := 0 + cmd := mvvm.NewCommand(func() { runs++ }, allowed.Get) + mvvm.BindCanExecute(cmd, allowed) + + b := toolkit.NewButton("Connect", nil) + clicks := 0 + b.OnClick = func() { clicks++ } + root := toolkit.NewVBox() + root.Append(b) + root.SetBounds(toolkit.Rect{X: 0, Y: 0, W: 100, H: 30}) + unbind := BindCommand(b, cmd, nil) + + root.OnEvent(toolkit.Event{Kind: toolkit.EventClick, X: 50, Y: 15}) + b.OnEvent(toolkit.Event{Kind: toolkit.EventKeyDown, Code: "Enter"}) + if clicks != 0 || runs != 0 { + t.Errorf("a button whose command cannot execute fired: clicks=%d runs=%d", clicks, runs) + } + root.OnEvent(toolkit.Event{Kind: toolkit.EventKeyDown, Code: "Tab"}) + if b.Focused() { + t.Error("Tab focused a button whose command cannot execute") + } + if !b.Disabled().Get() { + t.Fatal("a button whose command cannot execute is not Disabled") + } + clicks = 0 + b.SetFocused(false) + + allowed.Set(true) + if b.Disabled().Get() { + t.Fatal("the button stayed Disabled once the command can execute") + } + root.OnEvent(toolkit.Event{Kind: toolkit.EventKeyDown, Code: "Tab"}) + if !b.Focused() { + t.Fatal("Tab did not focus the button once its command can execute") + } + root.OnEvent(toolkit.Event{Kind: toolkit.EventClick, X: 50, Y: 15}) + if clicks != 1 || runs != 1 { + t.Fatalf("an enabled button's click: clicks=%d runs=%d, want 1/1", clicks, runs) + } + + // Unbind restores the Disabled state the button had before binding, even + // when the command cannot execute at that moment. + allowed.Set(false) + unbind() + if b.Disabled().Get() { + t.Fatal("unbind left the button Disabled") + } + if b.Style != toolkit.ButtonDefault { + t.Fatalf("unbind left Style=%v, want the original Default", b.Style) + } + pre := toolkit.NewButton("x", nil) + pre.Disabled().Set(true) + BindCommand(pre, mvvm.NewCommand(func() {}, nil), nil)() + if !pre.Disabled().Get() { + t.Fatal("unbind re-enabled a button that was Disabled before binding") + } +}