Prompt UI: horizontal input scrolling, overflow scrollbar on suggestion dropdowns - #2954
Conversation
de681db to
95022c2
Compare
CI regression triage — approach changed: the dropdown height growth is droppedRebased onto current master ( Why the height change was wrong to begin with
And it isn't harmless. Reproducing Keeping the 10-row cap and adding only the scrollbar satisfies both issues (14 locales > 10 rows and ~200 commands > 10 rows both overflow, so the indicator shows in exactly the reported scenarios) and is the strictly smaller change. Dropped it. Per-failure determination1. The test opens the palette on a 100x24 terminal, presses Down 15 times and asserts the first command scrolled out of view. With the 10-row cap that holds. With the height-sized dropdown the viewport became 17 rows, so selection 15 was still on screen and nothing scrolled — 2. This is the #1660 regression guard: 9 Downs must not move the list (selection stays inside the viewport), the 10th must. Under the taller dropdown the first half still passed and the second half failed, because the selection no longer left the viewport at row 10. I could have "updated it to the new geometry" by changing 9/10 to 17/18, but that would have been rewriting a regression guard to accommodate a change nobody asked for — and it would have baked a terminal-height-dependent number into a test. Reverting the geometry fixes it with no test edit. 3. After the geometry revert, the remaining diff is exactly one thing, and it is the intended #1593 fix: on the 10 suggestion rows the right-border column changes from Everything else — the split, the separator, the horizontal-scrolled long line, the row count, the suggestion text — is byte-identical. Regenerated with Also done
Verification on the rebased tree
Generated by Claude Code Generated by Claude Code |
With input longer than the prompt line (search prompt, open-file prompt, palette input), the renderer clipped the paragraph at the right edge and skipped set_cursor_position entirely once the cursor's logical column passed the width: the cursor vanished, the tail of the input was unviewable, and Left presses gave no visual feedback even though the internal cursor moved (issue #2876). The label now stays anchored at the left edge while the input renders in the remaining columns with a horizontal scroll that keeps the cursor inside the viewport (pinned to the last column while past the right edge), and the terminal cursor is always placed. Both prompt renderers (generic and file-open) share the new tail, which also replaces the file-open renderer's approximate cursor-column arithmetic with the actual display width of the label spans. Fixes #2876 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
Prompt suggestion dropdowns (Select Locale, command palette, and every other bottom-anchored suggestions popup) do scroll, but nothing said so: the popup's right border stayed a plain frame line, so a 14-locale list looked like a 10-locale list — issue #623, "at first glance, I thought only these". The command palette showed no scrollbar at all, the remaining item of issue #1593. Both lists render through the same SuggestionsRenderer, so one fix covers both. SuggestionsRenderer now draws the shared scrollbar widget over the popup's right border whenever the list overflows, and the scrollbar rect is registered in the chrome layout so the existing prompt-scrollbar mouse handlers (click-to-jump, thumb drag) work here exactly like in the Live Grep overlay. Scrollbar interaction latches the prompt's manual-scroll flag so the renderer's keep-selection-visible pass doesn't immediately undo it (mirroring the wheel-scroll behaviour of issue #2119), and moving the selection clears the latch again. The stale-rect case is also closed: the chrome's suggestions_scrollbar_rect now resets every frame instead of surviving after the overlay prompt closes. The dropdown deliberately keeps its 10-row height. Sizing it to the terminal instead was tried and dropped: neither issue asks for more rows, and a height-sized palette is worse to use — on an 80x24 terminal it covers all but one line of the buffer, and in a split layout it hides the other pane and the split separator entirely. MAX_VISIBLE_SUGGESTIONS now documents that reasoning; the unused Prompt::ensure_selected_visible wrapper is gone, since every caller passes the height it actually rendered. The Comprehensive UI B visual snapshot is regenerated for the one cell that changed: the palette's right border column now carries scrollbar cells instead of the plain frame glyph. Fixes #623 Fixes #1593 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
Clicking the command palette's new scrollbar put the thumb one row above the row under the cursor, and the bottom row of the track could never reach the end of the list — reported by the maintainer while testing #2954. The prompt scrollbar handlers mapped a click with `ScrollbarState::click_to_offset`, which divides the click row by the whole track height. The thumb's top can only travel `track_height - thumb_size` rows, so dividing by the track over-estimates the denominator and the result drifts up by a factor of `thumb_size / track_height` — one row for a palette-sized dropdown, and never quite `max_scroll` at the bottom. `offset_for_thumb_top` is the function that inverts the geometry the renderer actually draws, so both the press and the drag follow-up use it now, through one shared helper so the two can't drift apart again. `offset_for_thumb_top` itself was only accurate to within a row: `thumb_geometry` floors offset -> thumb row, so inverting it with a *rounding* division still landed a row high whenever the quotient wasn't integral. It now takes the smallest offset that reaches the requested row, and — because a list barely longer than its viewport has fewer scroll positions than track rows, so some rows are unreachable — falls back to whichever neighbouring offset renders closest. Its round-trip test asserts equality instead of a one-row tolerance, and fails on the old implementation. Refs #623 Refs #1593 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
…selection Editor-wide rule (and what VS Code does): the wheel scrolls the view, it never moves the selection. Prompt suggestion dropdowns broke it — the command palette, Select Locale, Set Language, quick open and every other bottom-anchored list walked `selected_suggestion` on each wheel tick and rewrote the prompt input to match, so the wheel silently retargeted what Enter would commit. Once #2954 let the scrollbar pin the viewport, that also produced the jump the maintainer reported: scroll the list with the scrollbar, then wheel, and the list leapt somewhere else. The wheel cleared the manual-scroll latch (on the grounds that it had "moved the selection"), so the renderer's keep-selection-visible pass yanked the offset back to a selection that had not visibly moved. `handle_prompt_scroll` now defers to `Prompt::scroll_results`, the same view-only scroll the Live Grep overlay has used since #2119, sized by the row count the renderer reported rather than a hard-coded 10. The selection stays where it is and may scroll out of sight; keyboard navigation still clears the latch and brings it back into view, so the latch now means one thing only — "the viewport is user-positioned, stop following the selection" — rather than papering over the coupling. `Popup::scroll_by` had the same coupling for `List` popups, dragging the selection along to keep it inside the viewport on every wheel tick and scrollbar click; it scrolls the view only now too. The keyboard paths (`select_next`, `page_down`, `select_last`, …) are unchanged: they move the selection and scroll the view to follow it. Refs #623 Refs #1593 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
95022c2 to
7081481
Compare
|
Thanks for testing this — both bugs you found are fixed, in two commits on top of the branch (force-pushed, rebased on current Bug 1 — the scrollbar thumb landed one row above the click
Not a border/geometry offset: the registered rect and the drawn track line up exactly (both are Both prompt-scrollbar handlers converted a click with
This is a shared primitive, so the fix also tightens press-to-position for the keybinding editor and the floating widget panels, which use it through Bug 2 — the wheel moved the selection
You're right that this is what produced the jump. It now defers to I applied the rule to every list surface in this code path, not just the palette:
Audit of the other wheel handlers — all already comply, no changes needed: the file browser ( TestsUnit, on the arithmetic itself (
E2E, driving real mouse/keyboard events and asserting only on rendered cells (
All 7 fail on this branch without the two fixes (verified from a clean build of the unpatched tree; the 4 pre-existing tests in the file pass both ways), and all 11 pass with them. Follow-up worth filing separately, both pre-existing and outside this PR: (a) Generated by Claude Code |
Clicking the command palette's new scrollbar put the thumb one row above the row under the cursor, and the bottom row of the track could never reach the end of the list — reported by the maintainer while testing #2954. The prompt scrollbar handlers mapped a click with `ScrollbarState::click_to_offset`, which divides the click row by the whole track height. The thumb's top can only travel `track_height - thumb_size` rows, so dividing by the track over-estimates the denominator and the result drifts up by a factor of `thumb_size / track_height` — one row for a palette-sized dropdown, and never quite `max_scroll` at the bottom. `offset_for_thumb_top` is the function that inverts the geometry the renderer actually draws, so both the press and the drag follow-up use it now, through one shared helper so the two can't drift apart again. `offset_for_thumb_top` itself was only accurate to within a row: `thumb_geometry` floors offset -> thumb row, so inverting it with a *rounding* division still landed a row high whenever the quotient wasn't integral. It now takes the smallest offset that reaches the requested row, and — because a list barely longer than its viewport has fewer scroll positions than track rows, so some rows are unreachable — falls back to whichever neighbouring offset renders closest. Its round-trip test asserts equality instead of a one-row tolerance, and fails on the old implementation. Refs #623 Refs #1593 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
Fixes #2876
Fixes #623
Fixes #1593
Motivation
Three related affordance gaps in the prompt/dropdown UI:
render_promptrendered a clipped paragraph and skippedset_cursor_positionentirely once the cursor's display column passed the line width, so the cursor vanished and the tail of a long input was unviewable even though the internal cursor kept moving.SuggestionsRendereras can the select locale scroll, or add a progress bar at the right. #623's locale list (not throughview/popup.rs, which already draws scrollbars for its own list popups), so one fix covers both issues.The fix
Commit 1 (#2876),
view/ui/status_bar.rs: the prompt label now stays anchored at the left edge while the input renders in the remaining columns with a horizontal scroll (Paragraph::scroll) computed by a newinput_hscroll(cursor_cells, width)helper — zero while the cursor fits, otherwise just enough that the cursor rides the last column — and the terminal cursor is always placed. Both prompt renderers (generic and file-open) share the new tail; this also replaces the file-open renderer's approximate cursor-column arithmetic with the actual display width of the label spans. Applies to all bottom prompt-line inputs (search, palette, open file / switch project / save as, plugin prompts).Commit 2 (#623 + #1593),
app/render.rs+view/ui/suggestions.rs:SuggestionsRendererdraws the shared scrollbar widget (view/ui/scrollbar.rs) over the popup's right border — same thumb geometry and theme colors as every other scrollbar. That is the missing indicator both issues asked for.suggestions_scrollbar_rect), so the existing prompt-scrollbar mouse handlers (click-to-jump, thumb drag) work for the bottom popup exactly like in the Live Grep overlay. Scrollbar interaction latches the prompt'smanual_scrollflag so the renderer's keep-selection-visible pass doesn't immediately undo the user's scroll; the latch is released when the selection moves.suggestions_scrollbar_rectnow resets every frame (previously it could go stale after the overlay prompt closed), and the now-unusedPrompt::ensure_selected_visiblewrapper is removed — every caller passes the actual rendered height toensure_selected_visible_within.The dropdown keeps its 10-row height. An earlier revision of this PR also grew the dropdown to fill the rows above the prompt (cap 20). That was dropped — see the "approach changed" comment below. Neither issue asks for more rows, and the taller popup made the palette measurably worse to use: on an 80x24 terminal it covers all but one line of the buffer, and in a split layout it hides the other pane and the split separator entirely.
MAX_VISIBLE_SUGGESTIONSnow documents that reasoning.Follow-up: two bugs found in maintainer testing (commits 3 and 4)
Commit 3 —
fix: land the dropdown scrollbar thumb on the row that was clicked. Clicking the palette's scrollbar put the thumb one row above the clicked row, and the bottom of the track could never reach the end of the list. The registered rect and the drawn track line up exactly; the bug was the mapping. Both prompt-scrollbar handlers usedScrollbarState::click_to_offset, which divides the click row by the whole track height, whereas the thumb's top only travelstrack_height - thumb_sizerows — so the result drifts up bythumb_size / track_height(one row for a palette-sized dropdown) and never quite reachesmax_scroll. Press and drag now share one helper built onoffset_for_thumb_top, the real inverse of the geometry the renderer draws.offset_for_thumb_topwas itself only accurate to within a row (hence its<= 1round-trip tolerance):thumb_geometryfloors offset → thumb row, so a rounding inverse still landed high. It now takes the smallest offset that reaches the requested row and falls back to the closest-rendering neighbour when a row is unreachable (a list barely longer than its viewport has fewer scroll positions than track rows). Because this is a shared primitive, press-to-position also gets more precise for the keybinding editor and the floating widget panels.Commit 4 —
fix: make the mouse wheel scroll prompt dropdowns without moving the selection. Editor-wide rule (and what VS Code does): the wheel scrolls the view, never the selection.handle_prompt_scrollwalkedselected_suggestionon each tick and rewrote the prompt input to match, so the wheel silently retargeted what Enter would commit — and cleared themanual_scrolllatch on the grounds that it had "moved the selection", which made the renderer yank the offset back and produced the reported jump after a scrollbar click. It now defers toPrompt::scroll_results, the view-only scroll the Live Grep overlay has used since #2119, sized by the row count the renderer reported instead of a hard-coded 10. The selection stays put and may scroll out of sight; keyboard navigation still clears the latch and brings it back, so the latch now means one thing only — "the viewport is user-positioned, stop following the selection" — rather than papering over the coupling.Popup::scroll_byhad the same coupling forview/popup.rsListpopups (dragging the selection to keep it inside the viewport on every wheel tick and scrollbar click) and is view-only now too; the keyboard paths (select_next,page_down,select_last, …) are unchanged.Audited and already compliant, so untouched: the file browser (
handle_file_open_scroll), the Live Grep overlay results and preview, the widget-panelList/Tree/Textwheels (Git Log, diagnostics, package manager, orchestrator dock), and the keybinding editor. Settings is a separate change.Tests
New tests (each verified to fail without the corresponding fix and pass with it):
status_bar.rsunit tests:input_hscrollcursor-visibility invariant sweep (scroll <= cursor,cursor - scroll < width, no scroll while it fits) and aTestBackendrender test asserting the scrolled tail, the anchored label, and the cursor pinned to the last column.suggestions.rsunit tests: scrollbar cells replace the right-border glyphs when the list overflows; the plain│border is kept when it fits.scrollbar.rsunit tests:test_offset_for_thumb_top_round_tripnow asserts exact equality for every reachable thumb row (including the two prompt-dropdown shapes, 30-in-10 and 14-in-10) and fails on the pre-fix rounding inverse;test_offset_for_thumb_top_picks_nearest_when_rows_unreachableguards the 11-in-10 case where exactness is impossible.issue_2876_prompt_input_hscroll.rs: 80-col search prompt, 100 typed chars — tail visible, hardware cursor at the right edge, and still visible after 15 Left presses while the window shifts.issue_623_prompt_dropdown_scrollbar.rs(11 tests, driving real mouse/keyboard events and asserting only on rendered cells): the overflowing command palette carries a scrollbar cell on every suggestion row; the Select Locale dropdown (14 locales, 10 rows) does too — issue can the select locale scroll, or add a progress bar at the right. #623 verbatim; a filtered short list keeps its plain border (negative control); clicking low on the track scrolls the list and keeps it scrolled; clicking every reachable track row lands the rendered thumb top on exactly that row, including both extremes; a continuous press-and-drag sweep down and back up keeps the thumb on the cursor row; and four wheel tests — over the palette, over Select Locale, over Set Language — assert the rendered rows change while the highlighted entry stays the same, that a selection scrolled out of view comes back unchanged, and that an arrow key releases the latch and brings it into view.Test runs observed on the current tree: the 11
issue_623_prompt_dropdown_scrollbare2e tests (the 7 new ones all fail on the unpatched tree, from a clean rebuild, while the 4 pre-existing ones pass both ways) and thescrollbar.rsoffset_for_thumb_topunit tests;cargo fmt;cargo clippy -p fresh-editorwith no findings in the touched files. No visual snapshot is changed by commits 3–4 — both are interaction-only and touch no rendering path. The broader suite runs quoted for commits 1–2 (fullfresh-editorlib unit suite, e2ecommand_palette/visual_regression/prompt/prompt_editing/locale/unicode_prompt_bugs/quick_open) were observed on the tree as of commit 2; CI covers the breadth for the current head.Updated: the
Comprehensive UI Bvisual snapshot (commit 2). The only cell that changed is the palette's right border column, which now carries scrollbar cells instead of the plain│frame glyph — the layout, the split, and the suggestion text are byte-identical.Notes for reviewers:
app/render.rs) still truncates long input at the right edge — it has its own layout (status/count cluster on the same row) and wasn't reported in input box can't use left / right when too long #2876; it would need the sameinput_hscrolltreatment as a follow-up.handle_click_popup_scrollbar(genericview/popup.rsscrollbars) uses a third, differently-imprecise mapping; and a double-click on any scrollbar goes tohandle_mouse_double_click, which never reaches the scrollbar handlers, so the second press of a fast double gesture is dropped.🤖 Generated with Claude Code
https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y