From 56beeb720dc35405dfa5f59baea321ddff8d7326 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 21 Aug 2026 15:17:14 +0200 Subject: [PATCH] =?UTF-8?q?fix(quest):=20QT5=20=E2=80=94=20the=20rows=20we?= =?UTF-8?q?re=20unclickable,=20and=20nothing=20showed=20which=20was=20sele?= =?UTF-8?q?cted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in the panel as committed, both found by checking the code against the authored data rather than by a test. The authored row template is a Type-3 generic container, which resolves through DatWidgetFactory's fallback arm to UiDatElement — whose constructor sets ClickThrough = true ("generic decoration; behavioral widgets opt back in"). Binding OnClick without clearing that compiles, reads correctly, and produces a list in which nothing can be selected: every click sails past the row. The skills page had already met this and left the precedent; I did not follow it. And there was no selection highlight at all, so even once clicking worked the player could not tell which row the detail pane was describing. UiTemplateListBox has no selection mechanism of its own, so the page opts in directly and re-PAINTS the highlight after a rebuild — a rebuild discards the row objects, so remembering the selection is not enough to keep it visible. The tests for both initially passed while the bugs were live, because the fixture's row root was a UiPanel and its text started white. A UiPanel is not click-through, so the first test was vacuous; white-on-white made the highlight unobservable. The fixture now builds the same UiDatElement production does and authors a non-white colour. This is the third time this campaign a fixture that did not match the real widget hid a real defect. Live mount confirmed against the installed dats in this session's client run: "[UI] retail journal panel from LayoutDesc importer (0x2100006E slot 0x10000559)" with no bind failure. Co-Authored-By: Claude Opus 5 --- .../Layout/JournalContractsPageController.cs | 42 +++++- .../JournalContractsPageControllerTests.cs | 136 +++++++++++++++++- 2 files changed, 175 insertions(+), 3 deletions(-) diff --git a/src/AcDream.App/UI/Layout/JournalContractsPageController.cs b/src/AcDream.App/UI/Layout/JournalContractsPageController.cs index e148f2e4..1791ec74 100644 --- a/src/AcDream.App/UI/Layout/JournalContractsPageController.cs +++ b/src/AcDream.App/UI/Layout/JournalContractsPageController.cs @@ -72,7 +72,16 @@ public sealed class JournalContractsPageController private readonly UiText? _description; private readonly UiText? _timedValue; + /// + /// Retail's own selection highlight for a listbox row. UiTemplateListBox + /// has no generic selection mechanism (its class doc), so the page opts in + /// directly — the same precedent + /// set. + /// + private static readonly Vector4 SelectedNameColor = Vector4.One; + private readonly List _rowContractIds = []; + private readonly List<(uint ContractId, UiText? Name, Vector4 Unselected)> _rows = []; private long _renderedRevision = -1; private uint _selectedContractId; @@ -138,6 +147,7 @@ public sealed class JournalContractsPageController _selectedContractId = 0u; _rowContractIds.Clear(); + _rows.Clear(); _list?.FlushPreservingScroll(); foreach (ContractTracker tracker in contracts) @@ -152,7 +162,8 @@ public sealed class JournalContractsPageController ContractEntry entry = catalog.Lookup(tracker.ContractId); - if (UiElement.FindDescendant(row, RowNameId) is UiText name) + var name = UiElement.FindDescendant(row, RowNameId) as UiText; + if (name is not null) SetText(name, entry.ContractName); if (UiElement.FindDescendant(row, RowStatusId) is UiText status) { @@ -161,11 +172,23 @@ public sealed class JournalContractsPageController tracker.ReceivedAt, entry, now)); } + // Captured AFTER SetText, which never touches DefaultColor (it is + // read lazily inside the provider closure), so this is the row's + // own authored colour to restore on deselect. + _rows.Add((tracker.ContractId, name, name?.DefaultColor ?? Vector4.One)); + uint captured = tracker.ContractId; if (row is UiDatElement clickable) + { + // A generic Type-3 container is click-THROUGH by default, so + // without this the handler below never fires and the list looks + // dead. UiDatElement carries the opt-in seam for exactly this. + clickable.ClickThrough = false; clickable.OnClick = () => Select(captured); + } } + ApplySelectionHighlight(); RefreshDetail(); } @@ -173,9 +196,26 @@ public sealed class JournalContractsPageController public void Select(uint contractId) { _selectedContractId = contractId; + ApplySelectionHighlight(); RefreshDetail(); } + /// + /// Re-painted rather than merely remembered: a rebuild discards the old row + /// objects, so a preserved selection has to be applied to the new ones. + /// + private void ApplySelectionHighlight() + { + foreach ((uint contractId, UiText? name, Vector4 unselected) in _rows) + { + if (name is not null) + { + name.DefaultColor = + contractId == _selectedContractId ? SelectedNameColor : unselected; + } + } + } + private void RefreshDetail() { ContractCatalog catalog = _bindings.Catalog(); diff --git a/tests/AcDream.App.Tests/UI/Layout/JournalContractsPageControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/JournalContractsPageControllerTests.cs index 49b20d81..cfd1587e 100644 --- a/tests/AcDream.App.Tests/UI/Layout/JournalContractsPageControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/JournalContractsPageControllerTests.cs @@ -27,11 +27,20 @@ public sealed class JournalContractsPageControllerTests private const uint DescriptionId = 0x100005DEu; private const uint TimedValueId = 0x100005E3u; + /// + /// The authored row name/status colour. Deliberately NOT white: the + /// selection highlight paints white, so a fixture that starts white cannot + /// observe the highlight at all. + /// + private static readonly System.Numerics.Vector4 AuthoredTextColor = + new(0.8f, 0.8f, 0.8f, 1f); + private static UiText Text(uint id) => new() { DatElementId = id, Width = 200f, Height = 18f, + DefaultColor = AuthoredTextColor, }; /// @@ -71,7 +80,15 @@ public sealed class JournalContractsPageControllerTests return (page, list); } - /// One row: a name text and a status text, as retail authors. + /// + /// One row: a name text and a status text, as retail authors. + /// + /// + /// The root is a because that is what the real + /// Type-3 template resolves to through DatWidgetFactory's generic fallback + /// arm — and its click-through default is the entire point of one of the + /// tests below. A UiPanel here would make that test pass vacuously. + /// private static UiElement? RowTemplate(uint layoutId, uint elementId) { if (layoutId != JournalContractsPageController.RowTemplateLayoutId @@ -80,7 +97,15 @@ public sealed class JournalContractsPageControllerTests return null; } - var row = new UiPanel { Width = 270f, Height = 16f }; + var row = new UiDatElement( + new ElementInfo + { + Id = JournalContractsPageController.RowTemplateElementId, + Type = 3, + Width = 270, + Height = 20, + }, + static _ => (0u, 0, 0)); row.AddChild(Text(RowNameId)); row.AddChild(Text(RowStatusId)); return row; @@ -353,6 +378,113 @@ public sealed class JournalContractsPageControllerTests Assert.Equal("Done (5m 0s to Repeat)", TextOf(page, StatusValueId)); } + [Fact] + public void RowsOptOutOfClickThroughOrTheListIsDead() + { + // The authored row template is a Type-3 generic container, and those + // are click-THROUGH by default — the click sails past the row to + // whatever is behind it and the handler never runs. Binding OnClick + // without clearing ClickThrough compiles, looks right, and produces a + // list you cannot select anything in. + (UiElement page, _) = BuildPage(); + using var state = new RuntimeContractState(); + Track(state, 0x10u, stage: 2u); + + Bind(page, state, Catalog(Entry(0x10u, "First"))); + + UiDatElement row = Assert.IsAssignableFrom(Assert.Single( + UiElement.FindDescendant(page, ListId)!.Children.SelectMany(Flatten), + e => e is UiDatElement { OnClick: not null })); + Assert.False(row.ClickThrough); + } + + [Fact] + public void ClickingARowSelectsIt() + { + (UiElement page, _) = BuildPage(); + using var state = new RuntimeContractState(); + Track(state, 0x10u, stage: 2u); + Track(state, 0x20u, stage: 2u); + + JournalContractsPageController controller = Bind(page, state, Catalog( + Entry(0x10u, "First"), Entry(0x20u, "Second", description: "Second desc."))); + Assert.Equal(0x10u, controller.SelectedContractId); + + UiDatElement second = Assert.IsAssignableFrom( + UiElement.FindDescendant(page, ListId)!.Children + .SelectMany(Flatten) + .Where(e => e is UiDatElement { OnClick: not null }) + .ElementAt(1)); + second.OnClick!(); + + Assert.Equal(0x20u, controller.SelectedContractId); + Assert.Equal("Second desc.", TextOf(page, DescriptionId)); + } + + [Fact] + public void TheSelectedRowIsHighlightedAndTheOthersAreNot() + { + // Without this the player cannot tell which row the detail pane is + // describing. + (UiElement page, _) = BuildPage(); + using var state = new RuntimeContractState(); + Track(state, 0x10u, stage: 2u); + Track(state, 0x20u, stage: 2u); + + JournalContractsPageController controller = Bind(page, state, Catalog( + Entry(0x10u, "First"), Entry(0x20u, "Second"))); + + UiText[] names = UiElement.FindDescendant(page, ListId)!.Children + .SelectMany(Flatten) + .OfType() + .Where(t => t.DatElementId == RowNameId) + .ToArray(); + Assert.Equal(2, names.Length); + + // Row 0 opens selected (no display contract, so the first row stands). + System.Numerics.Vector4 selectedColor = names[0].DefaultColor; + System.Numerics.Vector4 unselectedColor = names[1].DefaultColor; + Assert.NotEqual(unselectedColor, selectedColor); + + controller.Select(0x20u); + + // The highlight MOVES: the old row goes back to its authored colour + // rather than both rows ending up lit. + Assert.Equal(unselectedColor, names[0].DefaultColor); + Assert.Equal(selectedColor, names[1].DefaultColor); + } + + [Fact] + public void TheHighlightSurvivesARebuild() + { + // A rebuild discards the row objects, so a remembered selection has to + // be re-PAINTED onto the new ones rather than merely kept. + (UiElement page, _) = BuildPage(); + using var state = new RuntimeContractState(); + Track(state, 0x10u, stage: 2u); + Track(state, 0x20u, stage: 2u); + + JournalContractsPageController controller = Bind(page, state, Catalog( + Entry(0x10u, "First"), Entry(0x20u, "Second"))); + controller.Select(0x20u); + + Track(state, 0x30u, stage: 2u); // forces a rebuild + controller.Tick(); + + Assert.Equal(0x20u, controller.SelectedContractId); + UiText selected = UiElement.FindDescendant(page, ListId)!.Children + .SelectMany(Flatten) + .OfType() + .Where(t => t.DatElementId == RowNameId) + .ElementAt(1); + UiText unselected = UiElement.FindDescendant(page, ListId)!.Children + .SelectMany(Flatten) + .OfType() + .Where(t => t.DatElementId == RowNameId) + .ElementAt(0); + Assert.NotEqual(unselected.DefaultColor, selected.DefaultColor); + } + [Fact] public void AProgressCounterRendersThroughTheAuthoredFormat() {