Skip to content

Fix: menu lists can overflow the rendered window - #155

Open
Nidrax wants to merge 2 commits into
trunkfrom
claude/issue-138-20260826-1305
Open

Fix: menu lists can overflow the rendered window#155
Nidrax wants to merge 2 commits into
trunkfrom
claude/issue-138-20260826-1305

Conversation

@Nidrax

@Nidrax Nidrax commented Aug 26, 2026

Copy link
Copy Markdown
Member

Make overflowing menus scroll instead of running off-screen:

  • Cap popup/submenu window height to the display, so long menus no longer overflow the screen.
  • ui::Menu now hides items that don't fit and shows scroll indicators (using the combobox_arrow_up_selected/combobox_arrow_down_selected skin parts) when it needs to scroll. Scrolling works via clicking the arrows, the mouse wheel, or keyboard up/down navigation.
  • Replaced the hand-drawn submenu right-arrow triangle with the combobox_arrow_right skin parts.

Generated with Claude Code

Fixes #138

Cap popup/submenu window height to the display and let ui::Menu hide
items outside the visible window, showing combobox_arrow_up/down
skin parts as scroll indicators (clickable, mouse-wheel and keyboard
navigation all scroll). Also replace the hand-drawn submenu triangle
with the combobox_arrow_right skin parts.

Fixes #138

Co-authored-by: Daniel Praźmo <4753308+Nidrax@users.noreply.github.com>
@Nidrax

Nidrax commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

@claude

Scrolling doesn't work as intended. While it is possible to use touchpad/mouse wheel to scroll the menus, it is very clunky (doesn't always work, looks like an 'active' menu item has to be hovered on for the scrolling to work). Instead when the menu overflows the rendered view I wanted to display a fixed "scroller" items on with upward/downward arrows that would autoscroll the menu items list while mouse is hovered over them. Also implement bounds checking to not let the user scroll further beyond the point than when the last item on the list is in view.

@claude

claude Bot commented Aug 26, 2026

Copy link
Copy Markdown

Claude finished @Nidrax's task in 15m 41s —— View job


Done

  • Gather context (read src/ui/menu.h, src/ui/menu.cpp, src/app/ui/skin/skin_theme.cpp)
  • Understand current scrolling implementation and why hover/wheel scrolling is unreliable
  • Design fixed-arrow autoscroll: hovering the up/down arrow zone continuously scrolls while the mouse stays there (via a timer), instead of one-shot click/wheel scroll
  • Add/verify bounds checking so scrolling stops exactly when the first/last item is in view
  • Implement changes in src/ui/menu.h / src/ui/menu.cpp
  • Build and sanity-check (ninja besprited) — builds clean, no warnings in the touched files
  • Commit and push

What was wrong: the old implementation only scrolled on kMouseDownMessage (click each arrow) or kMouseWheelMessage. The wheel path was unreliable because the toolkit only routes wheel/move events to whichever widget the mouse last moved over (mouse_widget in Manager) — if the popup opened under a stationary cursor, or the cursor hadn't crossed a highlighted item yet, wheel events never reached the menu at all, which matches the "only works when an item is hovered" symptom. Mouse-wheel scrolling also didn't check m_hasMoreBelow, so it could be spun past the last item into blank trailing space.

What changed (src/ui/menu.cpp / src/ui/menu.h):

  • Menu now owns a Timer (m_scrollTimer) plus m_autoScrollDirection. On kMouseMoveMessage/kMouseDownMessage, if the cursor is inside the fixed m_scrollUpBounds/m_scrollDownBounds arrow strips, startAutoScroll() kicks off continuous scrolling on a 90ms tick (kMenuAutoScrollInterval) for as long as the cursor stays there — no click needed. Moving off the arrow (or a kMouseLeaveMessage) calls stopAutoScroll().
  • scrollBy() now refuses to advance past either end: up is bounded by m_scrollTopIndex <= 0, down is bounded by m_hasMoreBelow (from the last layoutItems() pass), so it can no longer scroll past the point where the last/first item is fully in view — this also fixed the mouse-wheel over-scroll bug.
  • The auto-scroll timer itself re-checks those same bounds each tick and stops itself once the end is reached.
  • Mouse-wheel scrolling is left in place as a secondary option (now bounds-checked); keyboard up/down navigation was already unaffected since it scrolls via ensureVisible().

Verified ninja besprited builds clean end-to-end (including menu.cpp and skin_theme.cpp) with no new warnings.

Branch: claude/issue-138-20260826-1305

Replace click/wheel-based menu scrolling, which relied on the mouse
having already moved over the popup to register, with a Timer-driven
auto-scroll that starts as soon as the cursor hovers a scroll arrow
and keeps scrolling until it leaves or the end of the list is
reached. Also fixes scrollBy() so wheel/timer scrolling can't advance
past the point where the last (or first) item is fully in view.

Co-authored-by: Daniel Praźmo <4753308+Nidrax@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu items lists can overflow the rendered window

1 participant