From 67aba8c386d6dde87eb54d816ae6b16a622a37ce Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 7 Sep 2026 15:42:25 +0200 Subject: [PATCH] fix: plain popup scrollbar draws retail chrome, not a flat bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner live-client report 2026-09-07: "For scrollable dropdown or the meta window we use the same assets as we do in for example chat or inventory window." The plain-style popup's scrollable-overflow scrollbar (DrawScrollablePopupPlain / DrawPopupScrollbarPlain in UiMenu.cs) drew a home-made flat 1px track + flat thumb instead of the gold track + up/down arrow buttons + thumb the chat SpewBox and inventory UiItemList already use through RetailScrollbarChrome. The owner only ever objected to the retail ROW art (checkmark glyph, gradient panel) — the bar itself was never in scope for the plain-row fix, so this change touches only the scrollbar draw call and leaves the plain row rendering untouched. DrawScrollablePopupPlain now calls the existing DrawPopupScrollbar helper (the same procedural sprite-chrome draw VendorUiController/ ConfigOptionsPageController already use) whenever a SpriteResolve is wired, falling back to the old flat DrawPopupScrollbarPlain only for a hand-built UiMenu with no resolver at all. New RetailScrollbarChrome.ApplyToMenuPopup(UiMenu) wires the same vertical skin ids (Track/Up/Down/ThumbTop/Mid/Bot Normal) the chat/inventory scrollbar uses onto a menu's own ScrollTrackSprite/etc properties. Mutation shown to fail first: UiMenuPlainStyleTests's Plain_OpenPopup_ScrollableOverflow_DrawsPlainTrackAndFlatThumb_NoDatArt and Plain_ScrollablePopup_ContentFits_DrawsTrackWithNoThumb asserted resolveCalls==0 and an all-fill scrollbar — both failed (6 resolve calls, 6 sprite quads instead of 0) against the new DrawPopupScrollbar call before being rewritten to Plain_OpenPopup_ScrollableOverflow_DrawsRetailScrollbarChrome_RowsStayPlain and Plain_ScrollablePopup_ContentFits_DrawsNoScrollbarAtAll, which pin the new sprite-chrome behavior (6 resolved ids on overflow: track, up, down, thumb top/mid/bottom; 3 on content-fits: track+up+down, no thumb; 0 on a menu built with no resolver) while re-asserting the rows are still plain fills with zero retail row-sprite quads. Retail's own RetailButtonArt=true popup path (DrawGridPopup/DrawScrollablePopup) is untouched — its regression golden (Retail_OpenPopup_DrawIsByteForByteUnchanged_RegressionGolden) still passes byte-for-byte. Co-Authored-By: Claude Fable 5.1 --- src/AcDream.App/UI/RetailScrollbarChrome.cs | 21 +++++++ src/AcDream.App/UI/UiMenu.cs | 41 +++++++++---- .../UI/UiMenuPlainStyleTests.cs | 60 ++++++++++++++----- 3 files changed, 96 insertions(+), 26 deletions(-) diff --git a/src/AcDream.App/UI/RetailScrollbarChrome.cs b/src/AcDream.App/UI/RetailScrollbarChrome.cs index 0dc3623d..5c427027 100644 --- a/src/AcDream.App/UI/RetailScrollbarChrome.cs +++ b/src/AcDream.App/UI/RetailScrollbarChrome.cs @@ -84,6 +84,27 @@ internal static class RetailScrollbarChrome bar.ThumbBotPressedSprite = ThumbBotPressed; } + /// + /// Wires the retail vertical skin onto a popup's own + /// procedural scrollbar properties ( + /// etc). Owner live-client report 2026-09-07 ("For scrollable dropdown or + /// the meta window we use the same assets as we do in for example chat or + /// inventory window"): the popup's own DrawPopupScrollbar draws a + /// simpler Normal-only chrome (no hover/pressed states — matching how + /// VendorUiController/ConfigOptionsPageController already + /// wire these exact ids), so only the Normal-state constants are needed + /// here. + /// + internal static void ApplyToMenuPopup(UiMenu menu) + { + menu.ScrollTrackSprite = Track; + menu.ScrollThumbTopSprite = ThumbTopNormal; + menu.ScrollThumbSprite = ThumbMidNormal; + menu.ScrollThumbBottomSprite = ThumbBotNormal; + menu.ScrollUpSprite = UpNormal; + menu.ScrollDownSprite = DownNormal; + } + /// Wires the full retail horizontal skin onto . /// The leading () slot is the LEFT edge. internal static void ApplyHorizontal(UiScrollbar bar) diff --git a/src/AcDream.App/UI/UiMenu.cs b/src/AcDream.App/UI/UiMenu.cs index 95a03f66..f183310b 100644 --- a/src/AcDream.App/UI/UiMenu.cs +++ b/src/AcDream.App/UI/UiMenu.cs @@ -884,9 +884,19 @@ public sealed class UiMenu : UiElement } /// Plain counterpart of — same - /// -sliced single column, plain - /// selected/hover row fills, and a plain scrollbar - /// () instead of the sprite chrome. + /// -sliced single column and plain + /// selected/hover row fills, but the SCROLLBAR itself draws retail's own + /// chrome (owner live-client report 2026-09-07: "For scrollable dropdown + /// or the meta window we use the same assets as we do in for example + /// chat or inventory window") via the shared + /// helper — the exact ids + /// wires onto etc, the SAME sprite ids the + /// chat SpewBox/inventory scrollbar uses. Rows + /// stay plain by design (the owner accepted the flat dark list; only the + /// bar was objectionable). A menu built with no + /// at all (a hand-built test/legacy fixture) falls back to the fully + /// flat rather than silently + /// drawing nothing. private void DrawScrollablePopupPlain(UiRenderContext ctx) { ConfigurePopupScroll(); @@ -918,17 +928,26 @@ public sealed class UiMenu : UiElement avail ? PlainTextColor : TextColorGhosted); } - DrawPopupScrollbarPlain(ctx, inX + ColumnWidth, inY); + if (SpriteResolve is { } resolve) + DrawPopupScrollbar(ctx, resolve, inX + ColumnWidth, inY); + else + DrawPopupScrollbarPlain(ctx, inX + ColumnWidth, inY); } /// - /// Plain counterpart of : a 1px-bordered - /// track and a flat thumb, both in — no DAT - /// thumb/track/arrow-button art at all. Shares the exact same - /// geometry (so the thumb's drawn - /// position matches 's hit-test - /// math), but draws no separate up/down button glyphs — plain mode has no - /// art for them and the click regions already work through geometry alone + /// NO-RESOLVER FALLBACK ONLY (see 's + /// own doc comment — the 2026-09-07 owner directive moved the normal + /// plain-popup scrollbar to retail's own chrome via + /// ). This draws a 1px-bordered track and + /// a flat thumb, both in — no DAT + /// thumb/track/arrow-button art at all — for the rare case a + /// is built with true but no + /// at all (a hand-built test/legacy fixture). + /// Shares the exact same geometry (so + /// the thumb's drawn position matches + /// 's hit-test math), but + /// draws no separate up/down button glyphs — plain mode has no art for + /// them and the click regions already work through geometry alone /// ( is unchanged). /// private void DrawPopupScrollbarPlain(UiRenderContext ctx, float x, float y) diff --git a/tests/AcDream.App.Tests/UI/UiMenuPlainStyleTests.cs b/tests/AcDream.App.Tests/UI/UiMenuPlainStyleTests.cs index 66c5bf68..98d09738 100644 --- a/tests/AcDream.App.Tests/UI/UiMenuPlainStyleTests.cs +++ b/tests/AcDream.App.Tests/UI/UiMenuPlainStyleTests.cs @@ -359,8 +359,17 @@ public sealed class UiMenuPlainStyleTests Assert.Equal(RetailChromeSprites.Border + UiMenu.PlainPadding, glyphSeg.Verts[0], 3); } + // ── Owner live-client report 2026-09-07 ("For scrollable dropdown or the + // meta window we use the same assets as we do in for example chat or + // inventory window"): the plain popup's SCROLLBAR now draws retail's own + // chrome (the exact sprite ids RetailScrollbarChrome wires onto + // chat/inventory's own bar) — only the ROWS stayed plain. These two tests + // used to pin a fully flat/untextured scrollbar; they now pin the + // opposite: real sprite draws for the bar, untouched plain fills for the + // rows, and no visible bar at all once the content fits. + [Fact] - public void Plain_OpenPopup_ScrollableOverflow_DrawsPlainTrackAndFlatThumb_NoDatArt() + public void Plain_OpenPopup_ScrollableOverflow_DrawsRetailScrollbarChrome_RowsStayPlain() { int resolveCalls = 0; var menu = MakePopupMenu(retailButtonArt: false, itemCount: 12, rowsPerColumn: 5, scrollable: true, @@ -373,27 +382,39 @@ public sealed class UiMenuPlainStyleTests var segs = renderer.DebugSpriteSegmentVerts; Assert.True(menu.PopupScroll.HasOverflow); - Assert.Equal(0, resolveCalls); - Assert.Equal(0, QuadCount(segs, menu.ScrollTrackSprite)); - Assert.Equal(0, QuadCount(segs, menu.ScrollThumbSprite)); + // Track + up + down + thumb top/mid/bottom = 6 resolved sprite ids — + // the SAME chrome ids the chat/inventory scrollbar resolves through + // the same SpriteResolve seam, no longer the flat DrawFill-only path. + Assert.Equal(6, resolveCalls); + Assert.Equal(1, QuadCount(segs, menu.ScrollTrackSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollUpSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollDownSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollThumbTopSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollThumbSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollThumbBottomSprite)); float outerTop = menu.Height; float inX = RetailChromeSprites.Border, inY = outerTop + RetailChromeSprites.Border; - float scrollbarX = inX + PlainColumnWidth; + // The rows are untouched by the chrome swap: still a plain fill, no + // DAT row/checkbox art at all (RetailButtonArt=false's own contract). Assert.True(HasFillQuad(segs, inX, inY, PlainColumnWidth, PlainRowHeight, menu.PlainSelectedColor), - "expected visible row 0 (selected/current) filled with PlainSelectedColor"); - Assert.True(HasFillQuad(segs, scrollbarX, inY, menu.ScrollbarWidth, 5 * PlainRowHeight, menu.PlainBackgroundColor), - "expected the scrollbar track background fill"); + "expected visible row 0 (selected/current) still filled with PlainSelectedColor"); + Assert.Equal(0, QuadCount(segs, menu.ItemHighlightSprite)); + Assert.Equal(0, QuadCount(segs, menu.ItemNormalSprite)); - // popup bg(1)+outline(4) + selected row(1) + scrollbar bg(1)+outline(4) + thumb(1) = 12. - Assert.Equal(12, QuadCount(segs, 0u)); + // popup bg(1)+outline(4) + selected row(1) = 6 untextured quads; + // the scrollbar itself no longer contributes any (it is all sprite + // draws now). + Assert.Equal(6, QuadCount(segs, 0u)); } [Fact] - public void Plain_ScrollablePopup_ContentFits_DrawsTrackWithNoThumb() + public void Plain_ScrollablePopup_ContentFits_DrawsNoScrollbarAtAll() { - var menu = MakePopupMenu(retailButtonArt: false, itemCount: 3, rowsPerColumn: 5, scrollable: true); + int resolveCalls = 0; + var menu = MakePopupMenu(retailButtonArt: false, itemCount: 3, rowsPerColumn: 5, scrollable: true, + countResolveCall: n => resolveCalls += n); OpenAndHover(menu); var (renderer, ctx) = MakeContext(200f, 200f); @@ -402,9 +423,18 @@ public sealed class UiMenuPlainStyleTests Assert.False(menu.PopupScroll.HasOverflow); - // popup bg(1)+outline(4) + scrollbar bg(1)+outline(4) = 10, no thumb quad - // (nothing selected/hovered here either). - Assert.Equal(10, QuadCount(segs, 0u)); + // Content-fits still draws the track + up/down buttons (retail's own + // proportion-0x88-defaults-to-1.0 rule — a content-fits bar shows a + // full-track thumb elsewhere in this class), but no thumb: 3 resolves. + Assert.Equal(3, resolveCalls); + Assert.Equal(1, QuadCount(segs, menu.ScrollTrackSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollUpSprite)); + Assert.Equal(1, QuadCount(segs, menu.ScrollDownSprite)); + Assert.Equal(0, QuadCount(segs, menu.ScrollThumbSprite)); + + // popup bg(1)+outline(4) = 5 untextured quads (nothing + // selected/hovered here either, and the scrollbar draws no fills). + Assert.Equal(5, QuadCount(segs, 0u)); } [Fact]