#723: No scrollbar when the minimap is on: quadraui's MinimapLayout.scrollbar is computed and discarded, and sync_scrollbar_positions doesn't know the strip exists - #726
Merged
JDonaghy merged 2 commits intoSep 1, 2026
Conversation
…rollbar Two separate causes, both fixed: A. quadraui's `MinimapLayout.scrollbar` (from `Minimap::scroll_thumb`) was computed and discarded by `draw_minimap_strip` on both backends. It now paints the thumb via `Backend::draw_scrollbar` after `draw_minimap`, whenever quadraui says one exists (`None` when the file already fits). The `Scrollbar` quadraui hands back carries a zero-sized `track` (built before real pixel bounds exist), so `render::minimap_scrollbar` recomputes it against the strip's actual bounds — same buffer-line derivation `scroll_thumb` uses internally, just with real geometry. B. GTK's native `gtk4::Scrollbar` per window had no notion of the minimap and stayed pinned to the pane's outer edge — exactly where the strip now paints — so the two fought over the same column. `sync_scrollbar`/`sync_scrollbar_positions` now check `minimap_reserved_width` (the same call the strip itself is sized and reserved by) and hide the native widget for any window with an active strip: the minimap becomes the sole scroll affordance for that pane, matching VS Code's "minimap is the scrollbar track" model. TUI's own editor-internal vertical scrollbar column (painted inside quadraui's `draw_editor`) is unaffected — it already lives in a column strictly left of the minimap strip, so the two were never overlapping there; suppressing it too would need a new quadraui-side knob, which is out of scope here. Tests: two new shell_app black-box tests drive the real TUI paint path and confirm the thumb glyph appears in the minimap's own column when the file overflows, and that no thumb/track glyph paints anywhere when it fits. Two new render.rs unit tests pin `minimap_scrollbar`'s real-bounds geometry and the None/Some gate. Existing `snapshot_minimap_braille` regenerated to reflect the new thumb column. GTK's native-scrollbar-visibility change has no automated coverage: `App::new_headless` (the only headless GTK test harness this repo has) leaves `window_scrollbars` empty and never constructs a real `gtk4::Scrollbar`, so it can't observe widget visibility at all — flagged as a manual smoke-test item. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up to bfdfcb9, which regressed `test_tui_two_groups_single_boundary_scrollbar_481` (2 scrollbar columns -> 4: `[35, 36, 73, 74]`). Why the first attempt broke it: quadraui's `tui::draw_editor` *already* paints a solid one-column vertical scrollbar at the editor viewport's right edge, and that viewport is already narrowed by `minimap_reserved_width` — so the editor scrollbar sits in the column immediately left of the strip. Painting `MinimapLayout.scrollbar` over the strip via `Backend::draw_scrollbar` put a *second* solid bar in the strip's leftmost column, directly against the first. Two bars jammed together is precisely the operator-visible defect #481 exists to prevent. The strip is not missing a scroll affordance in the first place: `Minimap::layout` resolves a `viewport_highlight` band that *both* rasterisers already paint (a background accent across the visible rows on TUI, a translucent slider on GTK). `MinimapLayout.scrollbar` stays unread on purpose now, documented on `draw_minimap_strip`. That leaves the real, GTK-only half of #723 — the issue's own second clause, "sync_scrollbar_positions doesn't know the strip exists". Native `gtk4::Scrollbar` widgets live in the `Overlay` *above* the `DrawingArea`, so a scrollbar pinned to the pane's outer edge lands on top of the strip that `minimap_reserved_width` carved out of that same edge: indistinguishable from the minimap, and (being an opaque widget) hiding the strip content underneath. bfdfcb9 hid the widget outright; this insets it by the strip's own width instead, so it sits immediately left of the strip — exactly where TUI's editor-internal scrollbar column already is. Both backends now land one scroll affordance in the same relative place, and GTK keeps its native drag/page-click behaviour and its cursor-position tick (which hiding threw away). The geometry decision is factored into the pure `native_scrollbar_margin_start` and applied at all three call sites (`sync_scrollbar_positions`, `App`'s scrollbar sync, and the cursor indicator). With no strip it reduces to the pre-#723 expression exactly. Tests: - `minimap_strip_does_not_double_the_scrollbar_via_shell_app` (TUI black-box, replaces the old thumb-adjacency test): exactly one `'█'`/`'░'` column, with the strip's braille starting in the very next column. RED-verified by restoring the `draw_scrollbar` call — fails with `got [84, 85]`, and #481 simultaneously goes back to `[35, 36, 73, 74]`. - `native_scrollbar_placement_tests` (4 unit tests): strip-off reduces to the old expression; strip-on insets by the full strip width (RED against develop's expression, which returns 786 instead of 738); split panes inset relative to their own origin; degenerate pane clamps. - `minimap_braille.snap` reverted to develop's (no thumb column). - Removed the two `render.rs` tests that pinned the deleted `minimap_scrollbar` helper. GTK widget visibility/geometry has no headless coverage (`App::new_headless` never constructs a real `gtk4::Scrollbar`; `GtkDriver` only sees Cairo paint, not overlay widgets) — hence the pure-function test plus a SMOKE_TESTS item. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #723
Automated PR opened by coordinator for review of issue #723.