diff --git a/TrayApp.Test/TrayApplicationTests.cs b/TrayApp.Test/TrayApplicationTests.cs index 8164323..bfe0e62 100644 --- a/TrayApp.Test/TrayApplicationTests.cs +++ b/TrayApp.Test/TrayApplicationTests.cs @@ -122,4 +122,119 @@ public void Dispose_IsIdempotent() // call throws, and the lifetime's Exit handler has usually disposed it already by then. Assert.AreEqual(0, stops); } + + [TestMethod] + public void TrayIconClick_WithTheToggleDisabled_DoesNotRunTheToolsSetter() + { + int setterCalls = 0; + bool available = false; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => available) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + + // What painting the menu would have read. IsEnabled is a cache of the last refresh, so without this + // the item still carries its default of enabled and the assertion would pass for the wrong reason. + menu.Refresh(); + + host.OnTrayIconClicked(this, EventArgs.Empty); + + // The menu shows this greyed out, so a click on the icon has to agree with it. The native menu entry + // is refused by the toolkit; the icon is the path with nothing in the way. + Assert.AreEqual(0, setterCalls); + } + + [TestMethod] + public void TrayIconClick_WithTheToggleEnabled_RunsTheToolsSetter() + { + int setterCalls = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => true) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + menu.Refresh(); + + host.OnTrayIconClicked(this, EventArgs.Empty); + + // The fast path still has to work. Guarding it is worth nothing if it guards everything. + Assert.AreEqual(1, setterCalls); + } + + [TestMethod] + public void TrayIconClick_BecomingEnabledAgain_RunsTheToolsSetter() + { + int setterCalls = 0; + bool available = false; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => available) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + + menu.Refresh(); + host.OnTrayIconClicked(this, EventArgs.Empty); + + available = true; + menu.Refresh(); + host.OnTrayIconClicked(this, EventArgs.Empty); + + // Refused while the tool said it was unavailable, served once it said otherwise. The guard reads the + // refreshed state rather than latching on the first answer. + Assert.AreEqual(1, setterCalls); + } + + [TestMethod] + public void Activate_WithTheItemDisabled_DoesNotRunTheToolsSetter() + { + int setterCalls = 0; + bool available = true; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Toggle("Keep awake", () => false, _ => setterCalls++, isEnabled: () => available) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + + menu.Refresh(); + available = false; + menu.Refresh(); + + TrayMenuItem toggle = menu.Items.First(item => item is TrayToggleItem); + host.Activate(toggle); + + // The backstop behind the native menu. The toolkit will not raise Click for a greyed-out item, but + // it greys it out as of the last refresh, and a click already on its way when the tool became + // unavailable still arrives here. NativeMenuItem exposes Click with no way to raise it, so this + // calls the handler's target rather than the event. + Assert.AreEqual(0, setterCalls); + } + + [TestMethod] + public void TrayIconClick_WithNoToggleAtAll_DoesNothing() + { + int commandCalls = 0; + + TrayAppDefinition app = TrayAppBuilder.Create("demo") + .DisplayName("Demo") + .Command("Two", () => commandCalls++) + .Build(); + + using TrayApplication host = Host(app, out TrayMenu menu); + menu.Refresh(); + + host.OnTrayIconClicked(this, EventArgs.Empty); + + // The fast path is for toggles. A menu of commands must not have one picked for it, and quit least + // of all. + Assert.AreEqual(0, commandCalls); + } } diff --git a/TrayApp/Tray/TrayApplication.cs b/TrayApp/Tray/TrayApplication.cs index b3f1118..e0b315d 100644 --- a/TrayApp/Tray/TrayApplication.cs +++ b/TrayApp/Tray/TrayApplication.cs @@ -176,11 +176,25 @@ private void BuildNativeMenu() } } - private void OnTrayIconClicked(object? sender, EventArgs e) + /// + /// Flips the first toggle the user could have clicked in the menu. + /// + /// + /// A disabled toggle is skipped, because this is the only activation path with nothing between the + /// click and the tool's setter. The menu path is projected onto s that + /// the toolkit greys out and refuses to raise Click for, so the tray icon is where a tool's + /// isEnabled predicate would otherwise be silently ignored - the menu would show the action + /// as unavailable while a click on the icon ran it anyway. + /// + /// Internal so that it can be driven without a platform: everything above this reads menu state + /// that populates with no display attached. + /// + /// + internal void OnTrayIconClicked(object? sender, EventArgs e) { foreach ((TrayMenuItem model, _) in entries) { - if (model is TrayToggleItem) + if (model is TrayToggleItem { IsEnabled: true }) { Activate(model); return; @@ -192,8 +206,30 @@ private void OnTrayIconClicked(object? sender, EventArgs e) private void OnExit(object? sender, ControlledApplicationLifetimeExitEventArgs e) => Dispose(); - private void Activate(TrayMenuItem item) + /// + /// Runs one item's action and brings the menu back in line with it. + /// + /// + /// The enabled check here is the backstop rather than the guard that matters. A native menu item is + /// greyed out at the last refresh, so the toolkit already refuses to raise Click for it - but + /// "at the last refresh" is the gap: a tool's state can change between the paint and the click, and + /// nothing repaints a menu the user is already looking at. is + /// the value that painting used, so this agrees with what the user saw rather than second-guessing + /// it. + /// + /// Internal for the same reason as : the native Click that + /// reaches this cannot be raised from a test, since exposes the event + /// and no way to fire it, so the backstop is exercised by calling it. + /// + /// + /// The item to activate. + internal void Activate(TrayMenuItem item) { + if (!item.IsEnabled) + { + return; + } + bool succeeded = menu.Activate(item); if (succeeded && item is TrayToggleItem toggle)