Repository navigation
fix(ui): clamp menu content to available height so long lists scroll - #54391
ehealthwares wants to merge 3 commits into
Conversation
The composer agent/model selects open upward, and the menu surface had no height constraint, so a long panel rendered past the top of the window and was clipped by the app: the agents above the cut were unreachable, with no scrollbar and no wheel scrolling. corvu's popper already publishes the available height as --kb-popper-content-available-height on the content element, so clamp to it and let the surface scroll. Without the variable the declaration is ignored and the surface keeps its natural height. The select-v2 surfaces override overflow back to hidden and keep their own listbox scrolling. Adds a LongList story reproducing a 32-agent picker. Refs anomalyco#54374
github-actions
Bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I ran the #54374 reproduction (31 primary agents, 600px-tall window, web UI built from each checkout) against the base and this branch, and the fix works. On the base the agent menu opens 928px tall with its top at −547px, 20 of 33 agents are off-screen, and the wheel and keyboard do nothing. With this change it is capped at the popper's available height (372px, top at 9px), the wheel and End key scroll it, and the selected agent is visible when it opens. packages/ui type-checks.
Public API: unchanged (CSS and a Storybook story only).
One gap: menus with appearance="standard" keep overflow: hidden from their more specific rule. They now get the height cap but can't scroll, so long standard menus still hide items (see inline comment).
| popper reports as available, so the list scrolls instead of rendering | ||
| past the window edge where it gets clipped and unreachable. */ | ||
| max-height: var(--kb-popper-content-available-height); | ||
| overflow-y: auto; |
There was a problem hiding this comment.
This works for the default compact appearance (I checked it in the composer agent menu). But [data-component="menu-v2-content"][data-appearance="standard"] further down sets overflow: hidden with higher specificity, which overrides this overflow-y. standard menus (servers registry, MCP dialog, windows titlebar menu, project view, side region) now get capped at the available height but can't scroll, so overflowing items are still unreachable. Could the standard rule use overflow-x: hidden (or drop the overflow declaration) so both appearances scroll?
There was a problem hiding this comment.
Addressed in 52cd8bb — the standard appearance rule now uses overflow-x: hidden so the vertical axis follows the base rule. Re-verified both appearances in Chromium (clamped, overflow-y: auto, overflow-x: hidden, last item reachable) and updated the PR description against the template.
There was a problem hiding this comment.
Thanks. I checked 52cd8bb: the standard rule now sets only overflow-x: hidden, so it no longer overrides the base rule's overflow-y: auto and long standard menus can scroll. That covers my comment. The LongList story's change to agent-28 also fixes the initial value I mentioned.
|
|
||
| export const LongList = { | ||
| render: () => { | ||
| const [agent, setAgent] = createSignal("ehealthwares-marketing") |
There was a problem hiding this comment.
The initial value ehealthwares-marketing matches none of the generated agent-N items, so the story opens with nothing selected. Use one of the generated ids (e.g. a deep one like agent-28), so the story also shows the selected item scrolling into view.
There was a problem hiding this comment.
Good catch — that was my placeholder for the story's current agent and it matched none of the generated ids, so the story opened with nothing selected (and it needlessly carried my own org name into an open-source tree). Changed to agent-28, a deep item, so the story opens with the selected agent scrolled into view.
There was a problem hiding this comment.
Thanks, I confirmed in the diff: LongList now starts on agent-28, so the story opens with a deep item selected.
Menu surfaces with appearance="standard" set overflow: hidden, which beat the clamp applied to the base content rule, so long standard menus (servers registry, MCP dialog, windows titlebar menu, project view, side region) were capped at the available height but could not scroll — the overflow was still unreachable. Switch that declaration to overflow-x: hidden so the rounded border still clips while the vertical axis follows the content rule. Also select one of the generated ids in the LongList story so the selected agent is the one scrolled into view. Refs anomalyco#54374
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #54374
Type of change
What does this PR do?
The composer agent select opens upward from the footer, and the menu surface had no height constraint. corvu's popper computes the available space and publishes it as
--kb-popper-content-available-heighton the content element, but nothing consumed it, so a menu with more agents than fit between the footer and the top of the window rendered past the window edge and was clipped by the app:max-height: var(--kb-popper-content-available-height)plusoverflow-y: auto(andoverscroll-behavior: contain) on[data-component="menu-v2-content"]makes the surface clamp to that space and scroll. With the variable absent the declaration is ignored, so nothing changes for menus that are not popper-positioned.appearance="standard"menus declareoverflow: hidden, which beat the newoverflow-y; that is nowoverflow-x: hidden, so the rounded border still clips while the vertical axis follows the base rule.select-v2surfaces keep their own higher-specificityoverflow: hiddenand continue scrolling through their inner listbox, which stays capped at12rem.Because the focused item is inside the scroll container, the browser scrolls the current agent into view when the menu opens, so a long list no longer opens on a selection that is off-screen.
The
LongListstory renders a 32-item picker to reproduce and inspect the clamp.How did you verify your code works?
Verified the compiled stylesheet against a 32-item menu in Chromium (Playwright,
chromium-1217):max-heightresolves to the popper's available height andoverflow-ycomputes toauto; the panel is taller than the clamp and the wheel scrolls it; focusing a deep item scrolls it into view;select-v2still computeshiddenoverflow with a12remscrollable listbox; without the variablemax-heightcomputes tonone. Re-ran the same set forappearance="standard"(clamped,overflow-y: auto,overflow-x: hidden, last item reachable).lightningcsscompiles both files without errors.I could not run the repo's Storybook/component-test harness here (no Bun install), which is why the verification is a fixture harness rather than
packages/appcoverage. The bot's review comment on this PR has real before/after screenshots from the web UI built from this branch, which I did not reproduce.Screenshots / recordings
Not attached. The reproduction screenshots (before/after from the web UI built from each checkout, showing the menu clipped at −547px on the base and capped with wheel/End-key scrolling here) are in the bot's review comment on this PR.
Checklist