From fd521f7816d8281e006946d1d33b6bde38e4934e Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 6 Sep 2026 20:49:43 +0200 Subject: [PATCH] =?UTF-8?q?fix(vt):=20list=20column=20fix=20round=201/11?= =?UTF-8?q?=20=E2=80=94=20text=20column=20optional=20onclick?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit None of VTank's eight HudList instances relies on row selection (PluginCore.cs Monsters/Items/Meta/Route/Buffs/Consumables cell handlers) — every real text cell is an action target. Add an optional onclick="{Action}" to : when present, a click fires it with the row index INSTEAD of selecting; when absent, the original select-the-row behavior is unchanged. BuildListColumn now takes the column's own index (used for its onclick throw message and threaded through for the rest of the fix round's column-context messages); the unknown- throw also picked up the index as a side effect of that signature change. New tests (MarkupListColumnsTests): the onclick attribute binds and falls back correctly when absent, a malformed onclick throws at Build, and a click actually reaches the bound callback instead of firing SelectionChanged — each shown to fail first (missing TextClicked member / no 4-arg Text() overload before the production change). Co-Authored-By: Claude Fable 5.1 --- src/AcDream.App/UI/MarkupDocument.cs | 33 ++++++++-- src/AcDream.App/UI/UiMarkupList.cs | 14 ++-- src/AcDream.App/UI/UiMarkupListColumn.cs | 16 ++++- .../UI/MarkupListColumnsTests.cs | 65 +++++++++++++++++++ 4 files changed, 119 insertions(+), 9 deletions(-) diff --git a/src/AcDream.App/UI/MarkupDocument.cs b/src/AcDream.App/UI/MarkupDocument.cs index 43d2bc442..f39bfc410 100644 --- a/src/AcDream.App/UI/MarkupDocument.cs +++ b/src/AcDream.App/UI/MarkupDocument.cs @@ -553,7 +553,7 @@ public static class MarkupDocument if (listUsesColumns) { list.Columns = listChildren - .Select(columnEl => BuildListColumn(columnEl, binding, icons)) + .Select((columnEl, index) => BuildListColumn(columnEl, binding, icons, index)) .ToList(); } else @@ -994,7 +994,7 @@ public static class MarkupDocument /// stays null (draws nothing) rather than ever pointing at a null resolver. /// private static UiMarkupListColumn BuildListColumn( - XElement columnEl, object binding, IMarkupIconResolver? icons) + XElement columnEl, object binding, IMarkupIconResolver? icons, int index) { string? type = (string?)columnEl.Attribute("type"); float width = F(columnEl, "width"); @@ -1008,7 +1008,21 @@ public static class MarkupDocument Func>? colorsSource = colorsAttr is null ? null : BindUintList(colorsAttr, binding, "column colors"); - return UiMarkupListColumn.Text(width, textSource, colorsSource); + // Fix round finding 1: optional onclick — a text cell that + // declares one fires it with the row index instead of + // selecting; one that doesn't keeps the original + // select-on-click behavior. Same "resolve if binding-shaped, + // throw only if malformed" rule as the list's own onchange + // above — omitting the attribute entirely is fine. + string? textOnClickAttr = (string?)columnEl.Attribute("onclick"); + Action? textOnClick = BindIntAction(textOnClickAttr, binding); + if (textOnClickAttr is not null && textOnClick is null) + { + throw new FormatException( + $"{ColumnContext(index, "text", "onclick")} did not resolve to an " + + $"Action property on {binding.GetType().Name}"); + } + return UiMarkupListColumn.Text(width, textSource, colorsSource, textOnClick); } case "check": { @@ -1033,10 +1047,21 @@ public static class MarkupDocument } default: throw new FormatException( - $"unknown (expected text, check, or icon)"); + $"column[{index}] has unknown type=\"{type}\" (expected text, check, or icon)"); } } + /// + /// Fix round finding 5: every column-attribute throw message identifies + /// the offending column by position and declared type + /// (column[2] type="check" values) rather than the generic + /// "column values" the initial slice used — a plugin author with + /// several columns of the same type needs the index to find which + /// one is wrong. + /// + private static string ColumnContext(int index, string type, string attribute) => + $"column[{index}] type=\"{type}\" {attribute}"; + private static bool IsBinding(string value) => value.Length > 2 && value[0] == '{' && value[^1] == '}'; diff --git a/src/AcDream.App/UI/UiMarkupList.cs b/src/AcDream.App/UI/UiMarkupList.cs index 378f71db9..ad877e25c 100644 --- a/src/AcDream.App/UI/UiMarkupList.cs +++ b/src/AcDream.App/UI/UiMarkupList.cs @@ -360,10 +360,16 @@ public sealed class UiMarkupList : UiElement switch (columns[c].Kind) { case UiMarkupListColumnKind.Text: - // Text-column click selects — the list's own - // selected/onchange, unchanged in meaning from the - // single-column list. - SelectionChanged?.Invoke(index); + // Fix round finding 1: a text column with its own + // onclick fires THAT instead of selecting — none of + // VTank's eight lists actually uses row selection, every + // real text cell is an action target. A text column + // without onclick keeps the original select-the-row + // behavior (the list's own selected/onchange). + if (columns[c].TextClicked is { } onTextClick) + onTextClick(index); + else + SelectionChanged?.Invoke(index); break; case UiMarkupListColumnKind.Check: // A click in a check/icon column fires that column's own diff --git a/src/AcDream.App/UI/UiMarkupListColumn.cs b/src/AcDream.App/UI/UiMarkupListColumn.cs index 02de24891..470bf32ef 100644 --- a/src/AcDream.App/UI/UiMarkupListColumn.cs +++ b/src/AcDream.App/UI/UiMarkupListColumn.cs @@ -46,6 +46,18 @@ public sealed class UiMarkupListColumn /// column draws with the list's . /// public Func>? ColorsSource { get; init; } + /// + /// Fix round finding 1: optional onclick="{Action<int>}" on a + /// text column. Fired with the ROW INDEX on a click anywhere in the cell + /// INSTEAD of selecting the row, when present. Null (the default — no + /// onclick attribute at all) keeps today's original behavior: a + /// click in this cell selects the row and fires the list's own + /// onchange, exactly as before this fix. None of VTank's eight + /// lists actually uses row selection — every real text cell is an action + /// target — but the select-on-click default stays for any acdream markup + /// that already relies on it. + /// + public Action? TextClicked { get; init; } // ── check ───────────────────────────────────────────────────────────── /// <column type="check" values="{IReadOnlyList<bool>}">. @@ -79,12 +91,14 @@ public sealed class UiMarkupListColumn public static UiMarkupListColumn Text( float width, Func> textSource, - Func>? colorsSource) => new() + Func>? colorsSource, + Action? onClick = null) => new() { Kind = UiMarkupListColumnKind.Text, Width = width, TextSource = textSource, ColorsSource = colorsSource, + TextClicked = onClick, }; public static UiMarkupListColumn Check( diff --git a/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs b/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs index 616e3eabd..73d542a65 100644 --- a/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs +++ b/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs @@ -324,6 +324,71 @@ public sealed class MarkupListColumnsTests Assert.Throws(() => MarkupDocument.Build(xml, binding, Sprite)); } + // ── Fix round: text column optional onclick (fix item 1) ──────────────── + + [Fact] + public void TextColumn_OptionalOnclickAttribute_BindsAndDoesNotBreakWithoutIt() + { + var binding = new ThreeColumnBinding(); + const string withOnclick = + "" + + "" + + " " + + ""; + var panelWith = MarkupDocument.Build(withOnclick, binding, Sprite); + var listWith = Assert.IsType(panelWith.Children[0]); + Assert.NotNull(listWith.Columns![0].TextClicked); + + const string withoutOnclick = + "" + + "" + + " " + + ""; + var panelWithout = MarkupDocument.Build(withoutOnclick, binding, Sprite); + var listWithout = Assert.IsType(panelWithout.Children[0]); + Assert.Null(listWithout.Columns![0].TextClicked); + } + + [Fact] + public void TextColumn_MalformedOnclickAttribute_ThrowsAtBuild() + { + var binding = new ThreeColumnBinding(); + const string xml = + "" + + "" + + " " + + ""; + + var ex = Assert.Throws(() => MarkupDocument.Build(xml, binding, Sprite)); + Assert.Contains("column[0] type=\"text\" onclick", ex.Message); + } + + [Fact] + public void ClickInTextColumn_WithOnclick_FiresItInsteadOfSelecting() + { + var clicked = new List(); + var selections = new List(); + var list = new UiMarkupList + { + Width = 60f, Height = 200f, RowHeight = 20f, + SelectedIndexSource = () => -1, + SelectionChanged = row => selections.Add(row), + Columns = new[] + { + UiMarkupListColumn.Text( + 60f, () => new[] { "a", "b", "c" }, null, row => clicked.Add(row)), + }, + }; + var (_, ctx) = MakeContext(200f, 200f); + list.DrawSelfAndChildren(ctx); + + // Row 1 (y = 20..40). + list.OnEvent(new UiEvent { Type = UiEventType.MouseDown, Data1 = 10, Data2 = 25 }); + + Assert.Equal(new[] { 1 }, clicked); + Assert.Empty(selections); + } + // ── Draw-level: column x-offsets, check glyph, icon, clipping ──────────── private static (TextRenderer renderer, UiRenderContext ctx) MakeContext(float w, float h)