Repository navigation
fix(BindCommand): disable the button while its command cannot execute - #32
Merged
Merged
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause.
BindCommandpassedmvvm.BindCommandasetEnabledthat only swappedButton.Style(toButtonSecondaryand back). Its doc gave the reason, "aButtonhas no boolean disabled field", which is no longer true:Base.Disabled()exists,Button.OnEventignores every event while it is set, and the toolkit's focus walk skips disabled widgets. So a button whose command could not run still took clicks, Enter/Space and keyboard focus, and the host's ownOnClickhandler (composed beforeExecute) still ran. Both claimward apps (Linux and Windows) wrapBindCommandin a localbindCommandthat setsDisabledby hand for this reason.Fix.
setEnabledalso setsb.Disabled()to!CanExecute. The Style greying is kept as it was, so a bound button looks exactly as it did and the apps' workaround can be dropped without a visual change. The returned unbind now restores the button's priorStyleandDisabledas well asOnClick, so unbinding leaves no disabled or secondary-styled button behind, and rebinding no longer captures a greyed Style as the "original".Test.
TestBindCommandDisablesTheButtonputs the button in atoolkit.VBoxand drives it through the container. Before the fix it fails withfired: clicks=2 runs=0(click + Enter reached the host handler),Tab focused a button whose command cannot executeandnot Disabled. After the fix it passes, including re-enabling and the unbind restore, for both a button that was enabled before binding and one that was already disabled.Sabotage (each still passes
go vet): dropping theDisabled().SetinsetEnabledfails the test; dropping the restore in unbind failsunbind left the button Disabled.Locally: gofmt clean,
go vet ./...,go test ./...(100.0% statement coverage),go test -race.README: the binding table and the
BindCommandparagraph described the Style-only behaviour and the missing disabled field; both are updated.Toolkit dependency. mvvmtk depends on
github.com/go-widgets/toolkit(go.mod: v0.321.2). The fix uses onlyBase.Disabled()and the disabled handling already in v0.321.2 (the new test passes against it), so it needs no toolkit bump and does not depend on go-widgets/toolkit#482.Semver: patch. No exported API is added or changed; it is a behaviour fix (a button whose command cannot execute is now inert, and unbind restores Style and Disabled).
Fixes #31
🤖 Generated with Claude Code