erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(D.2b): UiMenu — controller owns Selected (review fix for Task 4)

Browse source 
Review caught a behavior divergence: the generic UiMenu auto-set its own
Selected on any enabled pick, while the controller's EnabledProvider keeps the
null-payload specials (Squelch / Tell-to-Selected) enabled/white like retail.
So a special-item click set Selected=null and shifted the highlight onto the
deferred placeholders — and the menu tests masked it by using a different
(specials-disabled) gate than the controller ships.

Fix: clean MVC contract mirroring retail UIElement_Menu::NewSelection — the
widget REPORTS the pick via OnSelect; the controller OWNS Selected (it sets it
only for talk-channel payloads). A special-item click now fires OnSelect(null),
the controller ignores it, and the active channel + highlight stay put —
observably identical to the pre-generalization widget, and extensible for when
Squelch lands. Tests realigned to the controller's gate (specials white) and to
the controller-owns-Selected contract.

Full suite: 403 passed, 2 skipped, 0 failed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-06-16 17:27:30 +02:00
parent 955f7a69a8
commit 67e5b8cff2
3 changed files with 34 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

5
src/AcDream.App/UI/Layout/ChatWindowController.cs
View file

@ -266,8 +266,13 @@ public sealed class ChatWindowController
menu.Items = System.Array.ConvertAll(ChannelItems,
t => new UiMenu.MenuItem(t.Label, (object?)t.Channel));
menu.Selected = (object?)c._activeChannel;
// Specials (Squelch / Tell-to-Selected, null payload) render WHITE/enabled like
// retail; only the talk-CHANNEL items grey when unavailable.
menu.EnabledProvider = p => p is not ChatChannelKind ch || ChannelAvailable(ch);
menu.ButtonLabelProvider = () => ChannelButtonLabel(c._activeChannel);
// The widget reports the pick; the controller owns Selected. Only a talk-channel
// payload updates the active channel + highlight — the null-payload specials are
// deferred no-ops (see the chat re-drive deferred list) and leave selection intact.
menu.OnSelect = p =>
{
if (p is ChatChannelKind ch) { c._activeChannel = ch; menu.Selected = p; }

9
src/AcDream.App/UI/UiMenu.cs
View file

@ -223,8 +223,13 @@ public sealed class UiMenu : UiElement
if (row >= 0 && row < RowsPerColumn && idx >= 0 && idx < Items.Count
&& (EnabledProvider?.Invoke(Items[idx].Payload) ?? true))
{
Selected = Items[idx].Payload;
OnSelect?.Invoke(Selected);
// The widget REPORTS the pick; the controller owns Selected (it sets
// Selected only for payloads it acts on). This mirrors retail
// UIElement_Menu::NewSelection delegating to the owner rather than
// self-selecting — so a deferred/no-op item (e.g. the Squelch /
// Tell-to-Selected specials, null payload) leaves the current
// selection + highlight unchanged when the controller ignores it.
OnSelect?.Invoke(Items[idx].Payload);
}
}
_open = false;

36
tests/AcDream.App.Tests/UI/UiMenuTests.cs
View file

@ -31,12 +31,14 @@ public class UiMenuTests
new("Tell to Olthoi Chat", (object?)ChatChannelKind.Olthoi),
};
// Availability gate identical to ChatWindowController.ChannelAvailable.
// Availability gate identical to ChatWindowController's EnabledProvider: the null-payload
// specials (Squelch/Tell-to-Selected) are ENABLED/white like retail; only talk-CHANNEL
// items grey when unavailable. (The widget reports any enabled pick via OnSelect; the
// controller decides whether to update Selected, so specials are inert no-ops anyway.)
private static bool ChannelAvailable(object? p)
=> p is ChatChannelKind ch
? ch is ChatChannelKind.Say or ChatChannelKind.General
or ChatChannelKind.Trade or ChatChannelKind.Lfg
: false; // null-payload (Squelch/Tell-to-Selected) = inert
=> p is not ChatChannelKind ch
|| ch is ChatChannelKind.Say or ChatChannelKind.General
or ChatChannelKind.Trade or ChatChannelKind.Lfg;
private UiMenu MakeMenu() => new UiMenu
{
@ -96,7 +98,8 @@ public class UiMenuTests
Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 10, 5))); // open
object? fired = null;
menu.OnSelect = p => fired = p;
// Mirror the controller: the widget reports the pick, the controller sets Selected.
menu.OnSelect = p => { fired = p; if (p is ChatChannelKind) menu.Selected = p; };
// "Chat to All" (Say) is index 2 = left col, row 2: y in [-85,-68). Say is available.
Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 10, -76)));
@ -111,7 +114,8 @@ public class UiMenuTests
Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 10, 5))); // open
object? fired = null;
menu.OnSelect = p => fired = p;
// Mirror the controller: the widget reports the pick, the controller sets Selected.
menu.OnSelect = p => { fired = p; if (p is ChatChannelKind) menu.Selected = p; };
// "Tell to Trade Chat" (Trade) is index 11 = right col (lx>=191), row 4: y in [-51,-34).
Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 200, -42)));
@ -120,17 +124,21 @@ public class UiMenuTests
}
[Fact]
public void Select_SpecialItem_DoesNotFire()
public void Select_SpecialItem_FiresNull_LeavesSelectionUnchanged()
{
var menu = MakeMenu();
var menu = MakeMenu(); // Selected = Say
Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 10, 5))); // open
int fired = 0;
menu.OnSelect = _ => fired++;
// "Squelch (ignore)" is index 0 = left col, row 0 (null payload): y in [-119,-102).
// null payload → ChannelAvailable returns false → inert.
// Mirror the controller: only channel payloads update Selected; the null-payload
// specials are deferred no-ops that leave the active channel + highlight unchanged.
bool fired = false; object? firedPayload = "sentinel";
menu.OnSelect = p => { fired = true; firedPayload = p; if (p is ChatChannelKind) menu.Selected = p; };
// "Squelch (ignore)" is index 0 = left col, row 0 (null payload), white/enabled.
Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 10, -110)));
Assert.Equal(0, fired);
Assert.True(fired); // the pick IS reported...
Assert.Null(firedPayload); // ...with the special's null payload
Assert.Equal(ChatChannelKind.Say, menu.Selected); // ...but selection is unchanged (deferred no-op)
}
[Fact]