From b85522b2c2f6177bba101984dc4c1deb45b6399e Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Thu, 18 Jun 2026 18:00:06 +0200 Subject: [PATCH] fix(ui): popup menu uses the shared scroll indicator, not raw ^/v glyphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PopupMenu drew its own unscaled ^/v text markers instead of the track+thumb indicator the rest of the UI uses, so it didn't show position/ proportion and looked inconsistent on e-ink. drawScrollIndicator(Px) assumed a full-width list (anchored to d.width()), which doesn't hold for a centred, narrower popup box — added a right_x-aware overload so it can anchor to the box's own edge instead. Co-Authored-By: Claude Sonnet 4.6 --- examples/companion_radio/ui-new/PopupMenu.h | 24 ++++++++------------- examples/companion_radio/ui-new/icons.h | 22 +++++++++++++++++-- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/examples/companion_radio/ui-new/PopupMenu.h b/examples/companion_radio/ui-new/PopupMenu.h index 48d248a4..4cfc00b4 100644 --- a/examples/companion_radio/ui-new/PopupMenu.h +++ b/examples/companion_radio/ui-new/PopupMenu.h @@ -1,6 +1,7 @@ #pragma once #include #include +#include "icons.h" // scalable scroll indicator (track + thumb), matches the rest of the UI // Generic scrollable popup menu overlay. // Caller owns item string lifetimes — addItem() stores const char* pointers. @@ -63,7 +64,10 @@ struct PopupMenu { if (_scroll < 0) _scroll = 0; bool can_scroll = (_count > vis); - int arrow_w = can_scroll ? (cw + 2) : 0; // right gutter for ^/v markers + // Right gutter for the scroll indicator column, plus a couple px of + // clearance from the box border (full-screen lists sit flush against the + // screen edge instead, so they don't need this extra margin). + int arrow_w = can_scroll ? (scrollIndicatorColWidth(display) + 2) : 0; // Box width: widest of the title / all items, plus padding and the arrow // gutter; clamped to the screen with a sane minimum. @@ -111,20 +115,10 @@ struct PopupMenu { display.setColor(DisplayDriver::LIGHT); } - // Scroll markers in the reserved gutter, drawn on top of their row and - // inverted on the selection bar so they stay visible. - if (can_scroll) { - int ax = bx + bw - arrow_w; - if (_scroll > 0) { - display.setColor(_scroll == _sel ? DisplayDriver::DARK : DisplayDriver::LIGHT); - display.setCursor(ax, list_y + 1); display.print("^"); - } - if (_scroll + vis < _count) { - int last = _scroll + vis - 1; - display.setColor(last == _sel ? DisplayDriver::DARK : DisplayDriver::LIGHT); - display.setCursor(ax, list_y + (vis - 1) * item_h + 1); display.print("v"); - } - } + // Same proportional track + thumb indicator the rest of the UI uses, + // anchored to the box's own right edge (a couple px clear of the border) + // instead of the screen edge since this box floats centred on screen. + drawScrollIndicator(display, bx + bw - 2, list_y, vis * item_h, _count, vis, _scroll); display.setColor(DisplayDriver::LIGHT); return 50; } diff --git a/examples/companion_radio/ui-new/icons.h b/examples/companion_radio/ui-new/icons.h index 2983f12c..df743cb5 100644 --- a/examples/companion_radio/ui-new/icons.h +++ b/examples/companion_radio/ui-new/icons.h @@ -285,7 +285,11 @@ inline int scrollIndicatorReserve(DisplayDriver& d, int total, int visible) { // total_px : total extent of the whole list (items or pixels) // view_px : extent currently visible // scroll_px : extent scrolled past (offset of the first visible item) -inline void drawScrollIndicatorPx(DisplayDriver& d, int top_y, int track_h, +// right_x : column origin's right edge, in screen pixels (the indicator +// occupies [right_x - col, right_x)). Full-screen lists pass +// d.width(); a narrower container (e.g. a centred popup) passes its +// own right edge so the indicator lands inside the box, not the screen. +inline void drawScrollIndicatorPx(DisplayDriver& d, int right_x, int top_y, int track_h, long total_px, long view_px, long scroll_px) { if (total_px <= view_px || track_h <= 0) return; // whole list fits — no indicator const long span = total_px - view_px; // > 0 here @@ -295,7 +299,7 @@ inline void drawScrollIndicatorPx(DisplayDriver& d, int top_y, int track_h, const int s = miniIconScale(d); const int col = 5 * s; // triangle / column width const int th = 3 * s; // triangle height - const int x = d.width() - col; // indicator column origin + const int x = right_x - col; // indicator column origin // Caps are static end-markers, always drawn flush at the very top / bottom of // the track; the thumb travels in the fixed band between them. (They mark the @@ -330,6 +334,13 @@ inline void drawScrollIndicatorPx(DisplayDriver& d, int top_y, int track_h, miniIconDrawHalo(d, x, top_y + track_h - th, ICON_SCROLL_DOWN); } +// Convenience overload anchored to the screen's right edge — the common case +// for full-width lists. +inline void drawScrollIndicatorPx(DisplayDriver& d, int top_y, int track_h, + long total_px, long view_px, long scroll_px) { + drawScrollIndicatorPx(d, d.width(), top_y, track_h, total_px, view_px, scroll_px); +} + // Uniform-height wrapper: one item = one unit. Drop-in replacement for // DisplayDriver::drawScrollArrows. // total : total number of items @@ -340,6 +351,13 @@ inline void drawScrollIndicator(DisplayDriver& d, int top_y, int track_h, drawScrollIndicatorPx(d, top_y, track_h, total, visible, first); } +// right_x variant — see drawScrollIndicatorPx's right_x for why a narrower +// container (e.g. a popup) needs this instead of the screen-edge default. +inline void drawScrollIndicator(DisplayDriver& d, int right_x, int top_y, int track_h, + int total, int visible, int first) { + drawScrollIndicatorPx(d, right_x, top_y, track_h, total, visible, first); +} + // Scrollable item-list skeleton shared by the list screens. Computes the visible // window from the font metrics, keeps `sel` in view, reserves the scroll-column, // draws each visible row through `row`, then the indicator. The callback