fix(quest): QT5 — the rows were unclickable, and nothing showed which was selected
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 <noreply@anthropic.com>
This commit is contained in:
parent
ec6eeb120d
commit
56beeb720d
2 changed files with 175 additions and 3 deletions
|
|
@ -72,7 +72,16 @@ public sealed class JournalContractsPageController
|
||||||
private readonly UiText? _description;
|
private readonly UiText? _description;
|
||||||
private readonly UiText? _timedValue;
|
private readonly UiText? _timedValue;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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
|
||||||
|
/// <see cref="CharacterCreationSkillsPage"/> set.
|
||||||
|
/// </summary>
|
||||||
|
private static readonly Vector4 SelectedNameColor = Vector4.One;
|
||||||
|
|
||||||
private readonly List<uint> _rowContractIds = [];
|
private readonly List<uint> _rowContractIds = [];
|
||||||
|
private readonly List<(uint ContractId, UiText? Name, Vector4 Unselected)> _rows = [];
|
||||||
|
|
||||||
private long _renderedRevision = -1;
|
private long _renderedRevision = -1;
|
||||||
private uint _selectedContractId;
|
private uint _selectedContractId;
|
||||||
|
|
@ -138,6 +147,7 @@ public sealed class JournalContractsPageController
|
||||||
_selectedContractId = 0u;
|
_selectedContractId = 0u;
|
||||||
|
|
||||||
_rowContractIds.Clear();
|
_rowContractIds.Clear();
|
||||||
|
_rows.Clear();
|
||||||
_list?.FlushPreservingScroll();
|
_list?.FlushPreservingScroll();
|
||||||
|
|
||||||
foreach (ContractTracker tracker in contracts)
|
foreach (ContractTracker tracker in contracts)
|
||||||
|
|
@ -152,7 +162,8 @@ public sealed class JournalContractsPageController
|
||||||
|
|
||||||
ContractEntry entry = catalog.Lookup(tracker.ContractId);
|
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);
|
SetText(name, entry.ContractName);
|
||||||
if (UiElement.FindDescendant(row, RowStatusId) is UiText status)
|
if (UiElement.FindDescendant(row, RowStatusId) is UiText status)
|
||||||
{
|
{
|
||||||
|
|
@ -161,11 +172,23 @@ public sealed class JournalContractsPageController
|
||||||
tracker.ReceivedAt, entry, now));
|
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;
|
uint captured = tracker.ContractId;
|
||||||
if (row is UiDatElement clickable)
|
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);
|
clickable.OnClick = () => Select(captured);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
ApplySelectionHighlight();
|
||||||
RefreshDetail();
|
RefreshDetail();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -173,9 +196,26 @@ public sealed class JournalContractsPageController
|
||||||
public void Select(uint contractId)
|
public void Select(uint contractId)
|
||||||
{
|
{
|
||||||
_selectedContractId = contractId;
|
_selectedContractId = contractId;
|
||||||
|
ApplySelectionHighlight();
|
||||||
RefreshDetail();
|
RefreshDetail();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
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()
|
private void RefreshDetail()
|
||||||
{
|
{
|
||||||
ContractCatalog catalog = _bindings.Catalog();
|
ContractCatalog catalog = _bindings.Catalog();
|
||||||
|
|
|
||||||
|
|
@ -27,11 +27,20 @@ public sealed class JournalContractsPageControllerTests
|
||||||
private const uint DescriptionId = 0x100005DEu;
|
private const uint DescriptionId = 0x100005DEu;
|
||||||
private const uint TimedValueId = 0x100005E3u;
|
private const uint TimedValueId = 0x100005E3u;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
private static readonly System.Numerics.Vector4 AuthoredTextColor =
|
||||||
|
new(0.8f, 0.8f, 0.8f, 1f);
|
||||||
|
|
||||||
private static UiText Text(uint id) => new()
|
private static UiText Text(uint id) => new()
|
||||||
{
|
{
|
||||||
DatElementId = id,
|
DatElementId = id,
|
||||||
Width = 200f,
|
Width = 200f,
|
||||||
Height = 18f,
|
Height = 18f,
|
||||||
|
DefaultColor = AuthoredTextColor,
|
||||||
};
|
};
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|
@ -71,7 +80,15 @@ public sealed class JournalContractsPageControllerTests
|
||||||
return (page, list);
|
return (page, list);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>One row: a name text and a status text, as retail authors.</summary>
|
/// <summary>
|
||||||
|
/// One row: a name text and a status text, as retail authors.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// The root is a <see cref="UiDatElement"/> 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.
|
||||||
|
/// </remarks>
|
||||||
private static UiElement? RowTemplate(uint layoutId, uint elementId)
|
private static UiElement? RowTemplate(uint layoutId, uint elementId)
|
||||||
{
|
{
|
||||||
if (layoutId != JournalContractsPageController.RowTemplateLayoutId
|
if (layoutId != JournalContractsPageController.RowTemplateLayoutId
|
||||||
|
|
@ -80,7 +97,15 @@ public sealed class JournalContractsPageControllerTests
|
||||||
return null;
|
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(RowNameId));
|
||||||
row.AddChild(Text(RowStatusId));
|
row.AddChild(Text(RowStatusId));
|
||||||
return row;
|
return row;
|
||||||
|
|
@ -353,6 +378,113 @@ public sealed class JournalContractsPageControllerTests
|
||||||
Assert.Equal("Done (5m 0s to Repeat)", TextOf(page, StatusValueId));
|
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<UiDatElement>(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<UiDatElement>(
|
||||||
|
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<UiText>()
|
||||||
|
.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<UiText>()
|
||||||
|
.Where(t => t.DatElementId == RowNameId)
|
||||||
|
.ElementAt(1);
|
||||||
|
UiText unselected = UiElement.FindDescendant(page, ListId)!.Children
|
||||||
|
.SelectMany(Flatten)
|
||||||
|
.OfType<UiText>()
|
||||||
|
.Where(t => t.DatElementId == RowNameId)
|
||||||
|
.ElementAt(0);
|
||||||
|
Assert.NotEqual(unselected.DefaultColor, selected.DefaultColor);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void AProgressCounterRendersThroughTheAuthoredFormat()
|
public void AProgressCounterRendersThroughTheAuthoredFormat()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue