fix: do not activate a disabled toggle from a tray icon click [patch]

[matt-edmondson]

Fixes #7.

OnTrayIconClicked took the first TrayToggleItem in the menu and called Activate on it without reading IsEnabled:

if (model is TrayToggleItem)
{
    Activate(model);
    return;
}

The menu path never needed that check because the toolkit supplies it: BuildNativeMenu wires native.Click += …, and a greyed-out NativeMenuItem does not raise Click. The tray icon is the one path from a click to the tool's setter with nothing in the way, so a tool's isEnabled predicate was honoured in the menu and silently ignored on the icon — the entry showed unavailable while a left-click ran it anyway.

Measured: with the guards reverted, TrayIconClick_WithTheToggleDisabled_DoesNotRunTheToolsSetter records the setter running once against an expected zero.

The change

Two guards, both small:

1. OnTrayIconClicked — if (model is TrayToggleItem { IsEnabled: true }), exactly as the issue proposes.
2. Activate — returns early for a disabled item. This is the defence-in-depth the issue raises as optional, and it earns its place on a case the toolkit gating cannot cover: a native item is greyed out as of the last refresh, and nothing repaints a menu the user already has open, so a tool whose state changes between the paint and the click still reaches Activate. IsEnabled is the value the paint used, so the guard agrees with what the user saw rather than re-reading the predicate and disagreeing with the screen.

Both OnTrayIconClicked and Activate are internal, joining NativeMenu and HasStarted, so they can be driven with no display — which is how the rest of this class is already tested. Neither is public, so the package surface is unchanged.

TrayMenu.Activate is deliberately not touched: its bool return documents "the item's action completed without throwing", and making it also mean "was disabled" would conflate two different answers in a public API.

One thing worth your call

The guard sits inside the loop, per the issue's suggested line, so the click now activates the first enabled toggle. With two toggles where the first is disabled, that means the icon flips the second one rather than doing nothing. For the single-toggle case they are identical. If you would rather the fast path stay pinned to "the first switch" and do nothing when it is unavailable, that is moving the return out one level — say the word and I will change it.

Quit is unaffected

Worth confirming, since the Activate guard applies to commands too: TrayMenu constructs QuitItem with getEnabled: null, so its IsEnabled is always true. TrayIconClick_WithNoToggleAtAll_DoesNothing pins that a toggle-free menu has nothing picked for it.

Tests

Five added to TrayApplicationTests, which previously only covered menu projection and disposal. Verified by reverting each guard in turn and re-running: three fail against the old implementation.

Test	Old implementation
TrayIconClick_WithTheToggleDisabled_DoesNotRunTheToolsSetter	fails — setter ran once, expected zero
TrayIconClick_BecomingEnabledAgain_RunsTheToolsSetter	fails — setter ran twice, expected once
Activate_WithTheItemDisabled_DoesNotRunTheToolsSetter	fails without the Activate guard — setter ran once, expected zero
TrayIconClick_WithTheToggleEnabled_RunsTheToolsSetter	passes — guards the fast path still working
TrayIconClick_WithNoToggleAtAll_DoesNothing	passes — regression guard

Each test calls menu.Refresh() before clicking. IsEnabled is a cache of the last refresh and defaults to true, so without it the disabled test would have passed for the wrong reason.

Activate_WithTheItemDisabled_… was added after SonarCloud failed the gate at 66.7% coverage on new code: every other test drives OnTrayIconClicked, which skips a disabled toggle before Activate is reached, so the backstop was never executed. The alternative was deleting the guard to raise the number, which would have traded the fix for the metric. NativeMenuItem exposes Click with no way to raise it, so the native path cannot be driven from a test — hence calling Activate directly. Checked against a local cobertura report that every executable line this PR adds is now hit; the two uncovered lines remaining in the file (OnQuitRequested, OnExit) are pre-existing and untouched by this diff.

Full suite: 112 passed, 0 failed, 1 skipped (the pre-existing non-Linux DesktopSession case). Solution builds with 0 warnings, 0 errors.

🤖 Generated with Claude Code

https://claude.ai/code/session_018VSTy8Ye7JXqeFqvhrmnRt

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud