From bfdfcb9ab4dca8ba71eb67567b87f4a3f5aefe0b Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Tue, 1 Sep 2026 00:56:05 -0500 Subject: [PATCH 1/2] fix(#723): paint the minimap's scroll thumb; suppress GTK's native scrollbar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/gtk/mod.rs | 60 ++++++++-- src/render.rs | 120 +++++++++++++++++++- src/tui_main/shell_app.rs | 105 +++++++++++++++++ src/tui_main/snapshots/minimap_braille.snap | 24 ++-- 4 files changed, 283 insertions(+), 26 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index c2107aa3..930732c1 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -1427,7 +1427,7 @@ fn sync_scrollbar_positions( da_width: f64, da_height: f64, line_height: f64, - _char_width: f64, + char_width: f64, engine: &core::Engine, scrollbars: &HashMap, ) { @@ -1444,21 +1444,40 @@ fn sync_scrollbar_positions( let (window_rects, _dividers) = engine.calculate_group_window_rects(editor_bounds, tab_bar_height); + // #723: a window with an active minimap strip uses that strip as its + // scroll affordance instead (`draw_minimap_strip` paints a thumb over + // it, VS Code's "minimap is the scrollbar track" model) — the native + // widget stays pinned to the pane's right edge regardless, which is + // exactly where the strip now lives, so leaving both up doubles the + // affordance in the same column. `minimap_reserved_width` is the same + // per-window call `build_screen_layout_with_breadcrumb_row` uses to + // reserve/paint the strip, so "has a strip" can't drift from what + // actually painted this frame. + let has_minimap: std::collections::HashSet = window_rects + .iter() + .filter(|(_, r)| render::minimap_reserved_width(engine, r.width, char_width) > 0.0) + .map(|(wid, _)| *wid) + .collect(); + // Hide scrollbars for windows not in the current visible set - // (e.g. windows in non-active tabs), or when a modal popup is - // open. Native gtk4::Scrollbar widgets render above the - // DrawingArea, so they would otherwise poke through the - // palette / picker / tab-switcher overlays. + // (e.g. windows in non-active tabs), for windows whose minimap now + // owns the scroll affordance (#723), or when a modal popup is open. + // Native gtk4::Scrollbar widgets render above the DrawingArea, so + // they would otherwise poke through the palette / picker / + // tab-switcher overlays. let visible_ids: std::collections::HashSet = window_rects.iter().map(|(wid, _)| *wid).collect(); let modal_open = engine.is_blocking_modal_open(); for (wid, ws) in scrollbars.iter() { - let show = visible_ids.contains(wid) && !modal_open; + let show = visible_ids.contains(wid) && !modal_open && !has_minimap.contains(wid); ws.vertical.set_visible(show); ws.cursor_indicator.set_visible(show); } for (window_id, rect) in &window_rects { + if has_minimap.contains(window_id) { + continue; // scroll affordance is the minimap thumb, not this widget (#723) + } let ws = match scrollbars.get(window_id) { Some(ws) => ws, None => continue, @@ -2681,11 +2700,27 @@ impl App { } } + // #723: a window with an active minimap strip uses that strip as its + // scroll affordance instead (`draw_minimap_strip` paints a thumb over + // it, VS Code's "minimap is the scrollbar track" model) — leaving the + // native widget up too would double the affordance in the same + // column it now occupies. `minimap_reserved_width` is the same + // per-window call `build_screen_layout_with_breadcrumb_row` uses to + // reserve/paint the strip, so "has a strip" can't drift from what + // actually painted this frame. + let char_width = self.cached_char_width; + let has_minimap: std::collections::HashSet = window_rects + .iter() + .filter(|(_, r)| render::minimap_reserved_width(&engine, r.width, char_width) > 0.0) + .map(|(wid, _)| *wid) + .collect(); + // Hide scrollbars for windows that exist but aren't visible - // (e.g. windows in non-active tabs), or when a modal popup is - // open. Native gtk4::Scrollbar widgets render above the - // DrawingArea, so they would otherwise poke through the - // palette / picker / tab-switcher overlays. + // (e.g. windows in non-active tabs), whose minimap now owns the + // scroll affordance (#723), or when a modal popup is open. Native + // gtk4::Scrollbar widgets render above the DrawingArea, so they + // would otherwise poke through the palette / picker / + // tab-switcher overlays. let visible_ids: std::collections::HashSet = window_rects.iter().map(|(wid, _)| *wid).collect(); // Native gtk4::Scrollbar widgets render above the DrawingArea @@ -2695,13 +2730,16 @@ impl App { // in `Engine::is_blocking_modal_open()`. let modal_open = engine.is_blocking_modal_open(); for (wid, ws) in scrollbars.iter() { - let show = visible_ids.contains(wid) && !modal_open; + let show = visible_ids.contains(wid) && !modal_open && !has_minimap.contains(wid); ws.vertical.set_visible(show); ws.cursor_indicator.set_visible(show); } // Create/update scrollbars for each window for (window_id, rect) in &window_rects { + if has_minimap.contains(window_id) { + continue; // scroll affordance is the minimap thumb, not this widget (#723) + } let window = match engine.windows.get(window_id) { Some(w) => w, None => continue, diff --git a/src/render.rs b/src/render.rs index 921e874d..db12a295 100644 --- a/src/render.rs +++ b/src/render.rs @@ -4604,6 +4604,13 @@ pub fn build_minimap_data( /// `Backend::draw_minimap`, so each backend's wiring is a single call to this /// function. Nothing about sampling, scaling, dot packing or colour /// aggregation exists on either side of it in vimcode. +/// +/// #723: also paints the pane's scroll thumb over the strip — VS Code's +/// "minimap is the scrollbar track" model — whenever `MinimapLayout.scrollbar` +/// says one exists (`None` when the whole file already fits). Painted after +/// `draw_minimap` so the thumb sits on top of the strip's own content, via +/// `Backend::draw_scrollbar`, the same trait method TUI's own editor +/// scrollbar already goes through — no backend-specific painting code. pub fn draw_minimap_strip( backend: &mut dyn quadraui::Backend, screen: &ScreenLayout, @@ -4612,13 +4619,59 @@ pub fn draw_minimap_strip( .minimap .iter() .map(|mm| { - backend - .draw_minimap(minimap_strip_rect(mm), &mm.minimap) - .layout + let result = backend.draw_minimap(minimap_strip_rect(mm), &mm.minimap); + if result.layout.scrollbar.is_some() { + let sb = minimap_scrollbar(mm); + backend.draw_scrollbar(minimap_strip_rect(mm), &sb); + } + result.layout }) .collect() } +/// Real on-screen geometry for `mm`'s scroll thumb (#723). +/// +/// `Minimap::layout` already decides *whether* a thumb exists — +/// `MinimapLayout.scrollbar` is `None` when the whole file fits — but the +/// `Scrollbar` it carries in the `Some` case has a zero-sized `track` +/// (`Minimap::scroll_thumb` is private and builds it before real pixel +/// bounds exist), so its `thumb_start`/`thumb_len` collapse to `(0.0, 0.0)` +/// via `fit_thumb`'s own `track_len <= 0.0` bail-out — unusable for +/// painting. `scroll_thumb`'s own doc says as much: "callers that want +/// on-screen thumb pixels recompute `track` from their own bounds". This +/// does exactly that recompute — the same start/end buffer-line derivation +/// `scroll_thumb` uses internally (`MinimapLine::line_idx` at +/// `visible_row_start`/`visible_row_count`), just handed the strip's real +/// bounds instead of a zero rect — nothing here re-derives *whether* a +/// thumb should show; only `draw_minimap_strip`'s `scrollbar.is_some()` +/// gate (quadraui's decision) does that. +fn minimap_scrollbar(mm: &RenderedMinimap) -> quadraui::Scrollbar { + let m = &mm.minimap; + let start_buffer_line = m + .lines + .get(m.visible_row_start) + .map(|l| l.line_idx) + .unwrap_or(0); + let end_idx = (m.visible_row_start + m.visible_row_count).min(m.lines.len()); + let end_buffer_line = if end_idx == 0 { + start_buffer_line + } else { + m.lines + .get(end_idx - 1) + .map(|l| l.line_idx + 1) + .unwrap_or(m.total_buffer_lines) + }; + let visible = end_buffer_line.saturating_sub(start_buffer_line).max(1) as f32; + quadraui::Scrollbar::vertical( + format!("{}-scrollbar", m.id.0), + minimap_strip_rect(mm), + start_buffer_line as f32, + m.total_buffer_lines as f32, + visible, + 1.0, + ) +} + /// The strip a `RenderedMinimap` occupies, in quadraui coordinates. /// Shared by the paint path and the click path so the two cannot drift. pub fn minimap_strip_rect(mm: &RenderedMinimap) -> quadraui::Rect { @@ -16870,6 +16923,67 @@ mod tests { ); } + /// #723 part A: `minimap_scrollbar`'s track must be the strip's own + /// real on-screen bounds — not the zero-sized placeholder + /// `Minimap::scroll_thumb` builds internally before real pixel bounds + /// exist (`MinimapLayout.scrollbar.track` stays `Rect::new(0,0,0,0)`, + /// which collapses `thumb_start`/`thumb_len` to `(0.0, 0.0)` via + /// `fit_thumb`'s own `track_len <= 0.0` bail-out — unusable for + /// painting, see that function's own doc). This is the geometry + /// `draw_minimap_strip` actually hands `Backend::draw_scrollbar`. + #[test] + fn minimap_scrollbar_thumb_tracks_the_real_strip_bounds() { + let e = minimap_engine(); + let screen = render_engine(&e, 120.0, 30.0); + let mm = screen.minimap.first().expect("minimap present"); + let layout = mm + .minimap + .layout(minimap_strip_rect(mm), MINIMAP_LINES_PER_ROW); + assert!( + layout.scrollbar.is_some(), + "a 201-line file over a 30-row viewport must have a scroll thumb" + ); + + let sb = minimap_scrollbar(mm); + assert_eq!( + sb.track, + minimap_strip_rect(mm), + "the thumb's track must be the strip's real on-screen bounds" + ); + assert!(sb.thumb_len > 0.0, "the thumb must have non-zero length"); + assert!( + sb.thumb_len < sb.track.height, + "the thumb must be shorter than the full track — some of the \ + file is scrolled out of view" + ); + assert_eq!( + sb.thumb_start, 0.0, + "scrolled to the top of the file, the thumb must start at the \ + track's own top" + ); + } + + /// #723 part A, the other half: a file that fits entirely within the + /// viewport has nothing to scroll — `MinimapLayout.scrollbar` is + /// `None`, matching `Minimap::scroll_thumb`'s documented contract, and + /// `draw_minimap_strip` never calls `minimap_scrollbar` at all in that + /// case (see its `scrollbar.is_some()` gate). + #[test] + fn minimap_layout_has_no_scrollbar_when_the_file_fits() { + let mut e = test_engine(""); + e.buffer_mut().insert(0, &"short\n".repeat(5)); + let screen = render_engine(&e, 120.0, 30.0); + let mm = screen.minimap.first().expect("minimap present"); + let layout = mm + .minimap + .layout(minimap_strip_rect(mm), MINIMAP_LINES_PER_ROW); + assert!( + layout.scrollbar.is_none(), + "a 5-line file fitting entirely within a 30-row viewport must \ + have no scroll thumb" + ); + } + /// #722 acceptance: in a `:vsplit`, both panes show their own minimap /// over their own buffer — not a single strip pinned to the active /// pane. Each buffer gets distinct content so a transposed or diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index a8f67e89..80ed22d8 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -8296,4 +8296,109 @@ mod tests { 240-line file, landed on line {top} ({frac:.3}); screen:\n{after}" ); } + + /// `app_with_shaped_buffer` with the sidebar forced off (mirrors + /// `app_with_split_shaped_buffer`'s own doc comment on why: sidebar + /// visibility is otherwise ambient, read off the developer's real + /// `~/.config/vimcode`). The two tests below key off the *column* a + /// scrollbar glyph paints at, and the explorer tree view paints its + /// own `'█'`/`'░'` scrollbar too — indistinguishable by character from + /// the editor's/minimap's — so it must be off, not just "usually + /// absent", for those tests to reliably find the right column. + fn app_with_shaped_buffer_no_sidebar() -> TuiShellApp { + let mut app = app_with_shaped_buffer(); + app.engine.settings.autohide_panels = false; + app.engine.app_shell.hide_sidebar(); + app.engine.session.explorer_visible = false; + app + } + + /// Buffer short enough to fit entirely within the viewport — the other + /// half of #723's acceptance: `Minimap::scroll_thumb` returns `None` + /// when the whole file already fits, so no thumb (and, for the same + /// reason, no editor-internal scrollbar either) should paint anywhere. + /// Sidebar forced off for the same reason as + /// `app_with_shaped_buffer_no_sidebar`. + fn app_with_short_buffer() -> TuiShellApp { + let mut app = TuiShellApp::new(None); + app.engine.settings.autohide_panels = false; + app.engine.app_shell.hide_sidebar(); + app.engine.session.explorer_visible = false; + let text: String = (0..5).map(|i| format!("line {i}\n")).collect(); + app.engine.buffer_mut().insert(0, &text); + app + } + + /// #723 part A acceptance: with the minimap on and a file longer than + /// the viewport, `Minimap::layout`'s `MinimapLayout.scrollbar` resolves + /// `Some` — but `draw_minimap_strip` used to compute it via + /// `Backend::draw_minimap` and throw the result straight away (the + /// `MinimapLayout.scrollbar` field was never read anywhere in + /// `render.rs`/`src/tui_main/`/`src/gtk/`). This drives the real shell + /// paint path and confirms quadraui's own TUI scrollbar glyphs + /// (`'█'` thumb / `'░'` track, `tui::draw_scrollbar`) now paint in the + /// minimap strip's own leftmost column — immediately to the right of + /// the editor's own pre-existing vertical scrollbar column (same + /// glyphs, painted inside `draw_editor`), i.e. two adjacent columns, + /// never the same one. + /// + /// RED against `draw_minimap_strip` without its new `draw_scrollbar` + /// call: the column right of the editor's own scrollbar still shows + /// ordinary minimap braille, not a scrollbar glyph — confirmed by hand + /// by commenting out that call before restoring this fix. + #[test] + fn minimap_paints_a_scroll_thumb_when_the_file_overflows_via_shell_app() { + let mut driver = driver_with_shell(app_with_shaped_buffer_no_sidebar(), config(), 100, 24); + // Warm-up dispatch: the runner's own `AppShell` only picks up the + // engine's pinned sidebar/autohide state at the tail of a + // `handle()` call, never on the construction-time first frame + // (see `app_with_shaped_buffer_no_sidebar`'s doc comment). + driver.press_named(quadraui::NamedKey::Escape); + let screen = driver.screen(); + + fn is_scrollbar_glyph(c: char) -> bool { + c == '█' || c == '░' + } + + let row = 15usize; + let line = screen + .lines() + .nth(row) + .unwrap_or_else(|| panic!("row {row} must exist; screen:\n{screen}")); + let editor_sb_col = line + .chars() + .position(is_scrollbar_glyph) + .unwrap_or_else(|| { + panic!( + "the editor's own vertical scrollbar (240 lines overflowing \ + the viewport) must paint a '█'/'░' glyph on row {row}; \ + screen:\n{screen}" + ) + }); + + let next = line.chars().nth(editor_sb_col + 1); + assert!( + next.is_some_and(is_scrollbar_glyph), + "the minimap's own scroll thumb must paint a '█'/'░' glyph \ + immediately right of the editor's own scrollbar column \ + ({editor_sb_col}), got {next:?}; screen:\n{screen}" + ); + } + + /// #723 part A acceptance, the other half: a file that fits entirely + /// within the viewport must paint no scroll affordance anywhere — + /// neither the minimap's thumb nor the editor's own scrollbar, both + /// gated on the same "does it overflow" condition. + #[test] + fn minimap_paints_no_scroll_thumb_when_the_file_fits_via_shell_app() { + let mut driver = driver_with_shell(app_with_short_buffer(), config(), 100, 24); + // Warm-up dispatch — see `minimap_paints_a_scroll_thumb_when_the_file_overflows_via_shell_app`. + driver.press_named(quadraui::NamedKey::Escape); + let screen = driver.screen(); + assert!( + !screen.contains('█') && !screen.contains('░'), + "a file that fits entirely within the viewport must paint no \ + scroll thumb/track glyphs anywhere; screen:\n{screen}" + ); + } } diff --git a/src/tui_main/snapshots/minimap_braille.snap b/src/tui_main/snapshots/minimap_braille.snap index b17c0451..6bc05716 100644 --- a/src/tui_main/snapshots/minimap_braille.snap +++ b/src/tui_main/snapshots/minimap_braille.snap @@ -4,17 +4,17 @@ expression: "lines.join(\"\\n\")" --- 󰍜  [No Name] × 󰤲  ⋯ ▎ -  line 0 █ ⣿⣿⣿⣿⣿⣿⣿⣿⣿⡇⠀⢸⣿⣿ -  line 1 █ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ -  line 2 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ -  line 3 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ -  line 4 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 5 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 6 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 7 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 8 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ - line 9 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ - line 10 ░ ⣿⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ - line 11 ░ ⣿⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ +  line 0 █ █⣿⣿⣿⣿⣿⣿⣿⣿⡇⠀⢸⣿⣿ +  line 1 █ █⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ +  line 2 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ +  line 3 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ +  line 4 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 5 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 6 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 7 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 8 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ + line 9 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ + line 10 ░ ░⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ + line 11 ░ ░⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ NORMAL [No Name] 󰆍 󰘖 utf-8 LF Spaces: 4 Ln 1, Col 1  From f1d78c1367ef8b996189f624253ed0580e774603 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Tue, 1 Sep 2026 01:16:50 -0500 Subject: [PATCH 2/2] fix(#723): inset GTK's native scrollbar past the strip, not hide it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/gtk/mod.rs | 170 ++++++++++++++------ src/render.rs | 132 ++------------- src/tui_main/shell_app.rs | 95 ++++++----- src/tui_main/snapshots/minimap_braille.snap | 24 +-- 4 files changed, 199 insertions(+), 222 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 930732c1..58fc1af4 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -1412,6 +1412,39 @@ enum Msg { }, } +/// Left edge (drawing-area px, as a GTK `margin-start`) of a window's native +/// vertical `gtk4::Scrollbar`. +/// +/// #723: `minimap_reserved_width` carves the minimap strip out of the pane's +/// **right** edge, but native `gtk4::Scrollbar` widgets live in the `Overlay` +/// *above* the `DrawingArea` — so a scrollbar pinned to `rect`'s outer edge +/// lands on top of the strip, where it is both indistinguishable from the +/// minimap and (being an opaque widget) hides the strip content underneath. +/// That is the "no scrollbar when the minimap is on" the issue reports. +/// +/// Insetting by `minimap_width` puts the scrollbar immediately to the *left* +/// of the strip — exactly where TUI's editor-internal scrollbar column +/// already sits (quadraui's `tui::draw_editor` paints it at the editor +/// viewport's right edge, and that viewport is already narrowed by the same +/// reserved width). Both backends therefore land on one scroll affordance in +/// the same relative place, which is why `draw_minimap_strip` deliberately +/// does not paint a second one over the strip. +/// +/// The trailing 2px keeps the bar off the group divider / adjacent group, +/// preserved from the pre-#723 positioning. `minimap_width` is `0.0` when the +/// strip is off (`:set nominimap`, or a pane too narrow to afford one), which +/// reduces this to exactly that original expression. Clamped to `rect_x` so a +/// pathologically narrow pane can never push the widget left of its own pane. +fn native_scrollbar_margin_start( + rect_x: f64, + rect_width: f64, + scrollbar_width: f64, + minimap_width: f64, +) -> i32 { + let x = rect_x + rect_width - minimap_width - scrollbar_width - 2.0; + x.max(rect_x).round() as i32 +} + /// Reposition existing scrollbar widgets for the given drawing-area size. /// /// This is a free function so it can be called both from `sync_scrollbar` (via @@ -1444,40 +1477,21 @@ fn sync_scrollbar_positions( let (window_rects, _dividers) = engine.calculate_group_window_rects(editor_bounds, tab_bar_height); - // #723: a window with an active minimap strip uses that strip as its - // scroll affordance instead (`draw_minimap_strip` paints a thumb over - // it, VS Code's "minimap is the scrollbar track" model) — the native - // widget stays pinned to the pane's right edge regardless, which is - // exactly where the strip now lives, so leaving both up doubles the - // affordance in the same column. `minimap_reserved_width` is the same - // per-window call `build_screen_layout_with_breadcrumb_row` uses to - // reserve/paint the strip, so "has a strip" can't drift from what - // actually painted this frame. - let has_minimap: std::collections::HashSet = window_rects - .iter() - .filter(|(_, r)| render::minimap_reserved_width(engine, r.width, char_width) > 0.0) - .map(|(wid, _)| *wid) - .collect(); - // Hide scrollbars for windows not in the current visible set - // (e.g. windows in non-active tabs), for windows whose minimap now - // owns the scroll affordance (#723), or when a modal popup is open. - // Native gtk4::Scrollbar widgets render above the DrawingArea, so - // they would otherwise poke through the palette / picker / - // tab-switcher overlays. + // (e.g. windows in non-active tabs), or when a modal popup is + // open. Native gtk4::Scrollbar widgets render above the + // DrawingArea, so they would otherwise poke through the + // palette / picker / tab-switcher overlays. let visible_ids: std::collections::HashSet = window_rects.iter().map(|(wid, _)| *wid).collect(); let modal_open = engine.is_blocking_modal_open(); for (wid, ws) in scrollbars.iter() { - let show = visible_ids.contains(wid) && !modal_open && !has_minimap.contains(wid); + let show = visible_ids.contains(wid) && !modal_open; ws.vertical.set_visible(show); ws.cursor_indicator.set_visible(show); } for (window_id, rect) in &window_rects { - if has_minimap.contains(window_id) { - continue; // scroll affordance is the minimap thumb, not this widget (#723) - } let ws = match scrollbars.get(window_id) { Some(ws) => ws, None => continue, @@ -1493,13 +1507,16 @@ fn sync_scrollbar_positions( // — Vertical scrollbar — // Query the actual allocated width so we position correctly even if // GTK's theme enforces a minimum wider than our CSS min-width. - // Inset 2px from the right edge so the scrollbar doesn't visually - // overlap the group divider or the adjacent group's space. let sb_actual_w = ws.vertical.width().max(4) as f64; + let minimap_w = render::minimap_reserved_width(engine, rect.width, char_width); ws.vertical.set_halign(gtk4::Align::Start); ws.vertical.set_valign(gtk4::Align::Start); - ws.vertical - .set_margin_start(rect.x as i32 + (rect.width - sb_actual_w) as i32 - 2); + ws.vertical.set_margin_start(native_scrollbar_margin_start( + rect.x, + rect.width, + sb_actual_w, + minimap_w, + )); ws.vertical.set_margin_top(rect.y as i32); ws.vertical .set_height_request((rect.height as i32 - 4).max(0)); @@ -2700,27 +2717,18 @@ impl App { } } - // #723: a window with an active minimap strip uses that strip as its - // scroll affordance instead (`draw_minimap_strip` paints a thumb over - // it, VS Code's "minimap is the scrollbar track" model) — leaving the - // native widget up too would double the affordance in the same - // column it now occupies. `minimap_reserved_width` is the same - // per-window call `build_screen_layout_with_breadcrumb_row` uses to - // reserve/paint the strip, so "has a strip" can't drift from what - // actually painted this frame. + // #723: the minimap strip is reserved out of each pane's right edge, + // so the native scrollbar has to be inset past it — see + // `native_scrollbar_margin_start`. Same per-window call + // `build_screen_layout_with_breadcrumb_row` reserves/paints the strip + // with, so the inset can't drift from what actually painted. let char_width = self.cached_char_width; - let has_minimap: std::collections::HashSet = window_rects - .iter() - .filter(|(_, r)| render::minimap_reserved_width(&engine, r.width, char_width) > 0.0) - .map(|(wid, _)| *wid) - .collect(); // Hide scrollbars for windows that exist but aren't visible - // (e.g. windows in non-active tabs), whose minimap now owns the - // scroll affordance (#723), or when a modal popup is open. Native - // gtk4::Scrollbar widgets render above the DrawingArea, so they - // would otherwise poke through the palette / picker / - // tab-switcher overlays. + // (e.g. windows in non-active tabs), or when a modal popup is + // open. Native gtk4::Scrollbar widgets render above the + // DrawingArea, so they would otherwise poke through the + // palette / picker / tab-switcher overlays. let visible_ids: std::collections::HashSet = window_rects.iter().map(|(wid, _)| *wid).collect(); // Native gtk4::Scrollbar widgets render above the DrawingArea @@ -2730,16 +2738,13 @@ impl App { // in `Engine::is_blocking_modal_open()`. let modal_open = engine.is_blocking_modal_open(); for (wid, ws) in scrollbars.iter() { - let show = visible_ids.contains(wid) && !modal_open && !has_minimap.contains(wid); + let show = visible_ids.contains(wid) && !modal_open; ws.vertical.set_visible(show); ws.cursor_indicator.set_visible(show); } // Create/update scrollbars for each window for (window_id, rect) in &window_rects { - if has_minimap.contains(window_id) { - continue; // scroll affordance is the minimap thumb, not this widget (#723) - } let window = match engine.windows.get(window_id) { Some(w) => w, None => continue, @@ -2767,7 +2772,10 @@ impl App { ws.vertical.set_halign(gtk4::Align::Start); ws.vertical.set_valign(gtk4::Align::Start); - let scrollbar_x = rect.x as i32 + (rect.width - 10.0) as i32; + // Inset past the minimap strip (#723) — see + // `native_scrollbar_margin_start`. + let minimap_w = render::minimap_reserved_width(&engine, rect.width, char_width); + let scrollbar_x = native_scrollbar_margin_start(rect.x, rect.width, 10.0, minimap_w); ws.vertical.set_margin_start(scrollbar_x); ws.vertical.set_margin_top(rect.y as i32); ws.vertical @@ -2795,7 +2803,10 @@ impl App { let indicator_y = rect.y + (ratio * scrollbar_height); let sb_w = ws.vertical.width().max(4) as f64; - let indicator_x = rect.x as i32 + (rect.width - sb_w) as i32; + // Track the scrollbar's own inset so the cursor tick stays + // on the bar rather than under the minimap strip (#723). + let indicator_x = + native_scrollbar_margin_start(rect.x, rect.width, sb_w, minimap_w); ws.cursor_indicator.set_margin_start(indicator_x); ws.cursor_indicator.set_margin_top(indicator_y as i32); @@ -10576,6 +10587,61 @@ fn build_shell_config(app: &App) -> quadraui::ShellConfig { .with_activity_bar_width_px(48.0) } +#[cfg(test)] +mod native_scrollbar_placement_tests { + //! #723: GTK's 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 minimap strip that + //! `render::minimap_reserved_width` reserves out of that same edge — + //! the "no scrollbar when the minimap is on" symptom. + //! + //! Widget visibility/geometry itself is out of reach headlessly + //! (`App::new_headless` never constructs a real `gtk4::Scrollbar`, and + //! `GtkDriver` only sees Cairo paint, not overlay widgets), so the + //! geometry decision is factored into the pure + //! `native_scrollbar_margin_start` and pinned here. The on-screen + //! result is a SMOKE_TESTS item. + use super::native_scrollbar_margin_start; + + /// Strip off (`:set nominimap`, or a pane too narrow to afford one): + /// unchanged from the pre-#723 expression — right edge, less the + /// scrollbar's own width, less the 2px divider gap. + #[test] + fn no_minimap_keeps_the_scrollbar_on_the_panes_right_edge() { + assert_eq!(native_scrollbar_margin_start(0.0, 800.0, 12.0, 0.0), 786); + assert_eq!(native_scrollbar_margin_start(400.0, 400.0, 12.0, 0.0), 786); + } + + /// RED against `develop`: the pre-fix expression ignored the strip and + /// returned 786 here too — 48px *inside* the strip, i.e. the scrollbar + /// painted over the minimap instead of beside it. + #[test] + fn minimap_pushes_the_scrollbar_left_of_the_strip() { + let x = native_scrollbar_margin_start(0.0, 800.0, 12.0, 48.0); + assert_eq!(x, 738, "must be inset by the strip's full 48px width"); + assert!( + x + 12 <= 800 - 48, + "the scrollbar's right edge ({}) must not reach into the strip, \ + which starts at 752", + x + 12 + ); + } + + /// Second pane of a `:vsplit` — the inset is relative to that pane's own + /// origin, not the drawing area's. + #[test] + fn split_pane_inset_is_relative_to_the_panes_own_origin() { + assert_eq!(native_scrollbar_margin_start(400.0, 400.0, 12.0, 48.0), 738); + } + + /// A pane narrower than strip + scrollbar can't push the widget out of + /// its own pane. + #[test] + fn degenerate_pane_clamps_to_the_pane_origin() { + assert_eq!(native_scrollbar_margin_start(100.0, 30.0, 12.0, 48.0), 100); + } +} + #[cfg(test)] mod shell_config_identity_tests { //! #719: quadraui#656/#657 landed `ShellConfig::with_app_id()` / diff --git a/src/render.rs b/src/render.rs index db12a295..3a15ba89 100644 --- a/src/render.rs +++ b/src/render.rs @@ -4605,12 +4605,19 @@ pub fn build_minimap_data( /// function. Nothing about sampling, scaling, dot packing or colour /// aggregation exists on either side of it in vimcode. /// -/// #723: also paints the pane's scroll thumb over the strip — VS Code's -/// "minimap is the scrollbar track" model — whenever `MinimapLayout.scrollbar` -/// says one exists (`None` when the whole file already fits). Painted after -/// `draw_minimap` so the thumb sits on top of the strip's own content, via -/// `Backend::draw_scrollbar`, the same trait method TUI's own editor -/// scrollbar already goes through — no backend-specific painting code. +/// #723: the strip carries its own scroll affordance — `Minimap::layout` +/// resolves a `viewport_highlight` band that *both* quadraui rasterisers +/// already paint (a background accent across the visible rows on TUI, a +/// translucent slider on GTK). `MinimapLayout.scrollbar` is deliberately +/// **not** painted on top of it: TUI's `draw_editor` already paints a solid +/// one-column vertical scrollbar in the column immediately left of the +/// strip, so an extra solid bar in the strip's own first column reads as +/// two bars jammed together — the exact regression +/// `test_tui_two_groups_single_boundary_scrollbar_481` guards. The pane's +/// scrollbar instead stays *beside* the strip on both backends; GTK's +/// native widget is inset past the strip by `native_scrollbar_margin_start` +/// in `src/gtk/mod.rs`, which reads the strip width from +/// [`minimap_reserved_width`] — the same call that reserved it here. pub fn draw_minimap_strip( backend: &mut dyn quadraui::Backend, screen: &ScreenLayout, @@ -4619,59 +4626,13 @@ pub fn draw_minimap_strip( .minimap .iter() .map(|mm| { - let result = backend.draw_minimap(minimap_strip_rect(mm), &mm.minimap); - if result.layout.scrollbar.is_some() { - let sb = minimap_scrollbar(mm); - backend.draw_scrollbar(minimap_strip_rect(mm), &sb); - } - result.layout + backend + .draw_minimap(minimap_strip_rect(mm), &mm.minimap) + .layout }) .collect() } -/// Real on-screen geometry for `mm`'s scroll thumb (#723). -/// -/// `Minimap::layout` already decides *whether* a thumb exists — -/// `MinimapLayout.scrollbar` is `None` when the whole file fits — but the -/// `Scrollbar` it carries in the `Some` case has a zero-sized `track` -/// (`Minimap::scroll_thumb` is private and builds it before real pixel -/// bounds exist), so its `thumb_start`/`thumb_len` collapse to `(0.0, 0.0)` -/// via `fit_thumb`'s own `track_len <= 0.0` bail-out — unusable for -/// painting. `scroll_thumb`'s own doc says as much: "callers that want -/// on-screen thumb pixels recompute `track` from their own bounds". This -/// does exactly that recompute — the same start/end buffer-line derivation -/// `scroll_thumb` uses internally (`MinimapLine::line_idx` at -/// `visible_row_start`/`visible_row_count`), just handed the strip's real -/// bounds instead of a zero rect — nothing here re-derives *whether* a -/// thumb should show; only `draw_minimap_strip`'s `scrollbar.is_some()` -/// gate (quadraui's decision) does that. -fn minimap_scrollbar(mm: &RenderedMinimap) -> quadraui::Scrollbar { - let m = &mm.minimap; - let start_buffer_line = m - .lines - .get(m.visible_row_start) - .map(|l| l.line_idx) - .unwrap_or(0); - let end_idx = (m.visible_row_start + m.visible_row_count).min(m.lines.len()); - let end_buffer_line = if end_idx == 0 { - start_buffer_line - } else { - m.lines - .get(end_idx - 1) - .map(|l| l.line_idx + 1) - .unwrap_or(m.total_buffer_lines) - }; - let visible = end_buffer_line.saturating_sub(start_buffer_line).max(1) as f32; - quadraui::Scrollbar::vertical( - format!("{}-scrollbar", m.id.0), - minimap_strip_rect(mm), - start_buffer_line as f32, - m.total_buffer_lines as f32, - visible, - 1.0, - ) -} - /// The strip a `RenderedMinimap` occupies, in quadraui coordinates. /// Shared by the paint path and the click path so the two cannot drift. pub fn minimap_strip_rect(mm: &RenderedMinimap) -> quadraui::Rect { @@ -16923,67 +16884,6 @@ mod tests { ); } - /// #723 part A: `minimap_scrollbar`'s track must be the strip's own - /// real on-screen bounds — not the zero-sized placeholder - /// `Minimap::scroll_thumb` builds internally before real pixel bounds - /// exist (`MinimapLayout.scrollbar.track` stays `Rect::new(0,0,0,0)`, - /// which collapses `thumb_start`/`thumb_len` to `(0.0, 0.0)` via - /// `fit_thumb`'s own `track_len <= 0.0` bail-out — unusable for - /// painting, see that function's own doc). This is the geometry - /// `draw_minimap_strip` actually hands `Backend::draw_scrollbar`. - #[test] - fn minimap_scrollbar_thumb_tracks_the_real_strip_bounds() { - let e = minimap_engine(); - let screen = render_engine(&e, 120.0, 30.0); - let mm = screen.minimap.first().expect("minimap present"); - let layout = mm - .minimap - .layout(minimap_strip_rect(mm), MINIMAP_LINES_PER_ROW); - assert!( - layout.scrollbar.is_some(), - "a 201-line file over a 30-row viewport must have a scroll thumb" - ); - - let sb = minimap_scrollbar(mm); - assert_eq!( - sb.track, - minimap_strip_rect(mm), - "the thumb's track must be the strip's real on-screen bounds" - ); - assert!(sb.thumb_len > 0.0, "the thumb must have non-zero length"); - assert!( - sb.thumb_len < sb.track.height, - "the thumb must be shorter than the full track — some of the \ - file is scrolled out of view" - ); - assert_eq!( - sb.thumb_start, 0.0, - "scrolled to the top of the file, the thumb must start at the \ - track's own top" - ); - } - - /// #723 part A, the other half: a file that fits entirely within the - /// viewport has nothing to scroll — `MinimapLayout.scrollbar` is - /// `None`, matching `Minimap::scroll_thumb`'s documented contract, and - /// `draw_minimap_strip` never calls `minimap_scrollbar` at all in that - /// case (see its `scrollbar.is_some()` gate). - #[test] - fn minimap_layout_has_no_scrollbar_when_the_file_fits() { - let mut e = test_engine(""); - e.buffer_mut().insert(0, &"short\n".repeat(5)); - let screen = render_engine(&e, 120.0, 30.0); - let mm = screen.minimap.first().expect("minimap present"); - let layout = mm - .minimap - .layout(minimap_strip_rect(mm), MINIMAP_LINES_PER_ROW); - assert!( - layout.scrollbar.is_none(), - "a 5-line file fitting entirely within a 30-row viewport must \ - have no scroll thumb" - ); - } - /// #722 acceptance: in a `:vsplit`, both panes show their own minimap /// over their own buffer — not a single strip pinned to the active /// pane. Each buffer gets distinct content so a transposed or diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index 80ed22d8..6a5e9abc 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -8314,11 +8314,9 @@ mod tests { } /// Buffer short enough to fit entirely within the viewport — the other - /// half of #723's acceptance: `Minimap::scroll_thumb` returns `None` - /// when the whole file already fits, so no thumb (and, for the same - /// reason, no editor-internal scrollbar either) should paint anywhere. - /// Sidebar forced off for the same reason as - /// `app_with_shaped_buffer_no_sidebar`. + /// half of #723's acceptance: nothing overflows, so no scrollbar (and no + /// minimap thumb) should paint anywhere. Sidebar forced off for the same + /// reason as `app_with_shaped_buffer_no_sidebar`. fn app_with_short_buffer() -> TuiShellApp { let mut app = TuiShellApp::new(None); app.engine.settings.autohide_panels = false; @@ -8329,25 +8327,29 @@ mod tests { app } - /// #723 part A acceptance: with the minimap on and a file longer than - /// the viewport, `Minimap::layout`'s `MinimapLayout.scrollbar` resolves - /// `Some` — but `draw_minimap_strip` used to compute it via - /// `Backend::draw_minimap` and throw the result straight away (the - /// `MinimapLayout.scrollbar` field was never read anywhere in - /// `render.rs`/`src/tui_main/`/`src/gtk/`). This drives the real shell - /// paint path and confirms quadraui's own TUI scrollbar glyphs - /// (`'█'` thumb / `'░'` track, `tui::draw_scrollbar`) now paint in the - /// minimap strip's own leftmost column — immediately to the right of - /// the editor's own pre-existing vertical scrollbar column (same - /// glyphs, painted inside `draw_editor`), i.e. two adjacent columns, - /// never the same one. + /// #723 acceptance (TUI half): with the minimap on and a file longer + /// than the viewport, the pane shows **exactly one** vertical scroll + /// affordance, and it sits *beside* the strip rather than on top of it — + /// one column of `'█'`/`'░'` (quadraui's `tui::draw_editor` scrollbar), + /// with the minimap's braille starting in the very next column. /// - /// RED against `draw_minimap_strip` without its new `draw_scrollbar` - /// call: the column right of the editor's own scrollbar still shows - /// ordinary minimap braille, not a scrollbar glyph — confirmed by hand - /// by commenting out that call before restoring this fix. + /// This is the invariant the first attempt at #723 broke: painting + /// `MinimapLayout.scrollbar` over the strip via `Backend::draw_scrollbar` + /// put a second solid bar in the strip's leftmost column, directly + /// against the editor's own — two bars jammed together, which is exactly + /// the operator-visible defect + /// `render_impl::tests::test_tui_two_groups_single_boundary_scrollbar_481` + /// exists to prevent (it went from 2 scrollbar columns to 4). The strip's + /// own scroll feedback is quadraui's `viewport_highlight` band — a + /// *background* accent across the visible rows, painted by both + /// rasterisers — not a second foreground bar. + /// + /// RED against the reverted state: with `draw_minimap_strip` calling + /// `draw_scrollbar`, the column right of the editor's scrollbar is a + /// second `'░'`/`'█'` instead of braille, and the "exactly one" count is + /// 2. Verified by hand by restoring that call. #[test] - fn minimap_paints_a_scroll_thumb_when_the_file_overflows_via_shell_app() { + fn minimap_strip_does_not_double_the_scrollbar_via_shell_app() { let mut driver = driver_with_shell(app_with_shaped_buffer_no_sidebar(), config(), 100, 24); // Warm-up dispatch: the runner's own `AppShell` only picks up the // engine's pinned sidebar/autohide state at the tail of a @@ -8359,36 +8361,45 @@ mod tests { fn is_scrollbar_glyph(c: char) -> bool { c == '█' || c == '░' } + // Braille block: what `tui::draw_minimap` packs its dot cells from. + fn is_braille(c: char) -> bool { + ('\u{2800}'..='\u{28FF}').contains(&c) + } let row = 15usize; - let line = screen + let line: Vec = screen .lines() .nth(row) - .unwrap_or_else(|| panic!("row {row} must exist; screen:\n{screen}")); - let editor_sb_col = line + .unwrap_or_else(|| panic!("row {row} must exist; screen:\n{screen}")) .chars() - .position(is_scrollbar_glyph) - .unwrap_or_else(|| { - panic!( - "the editor's own vertical scrollbar (240 lines overflowing \ - the viewport) must paint a '█'/'░' glyph on row {row}; \ - screen:\n{screen}" - ) - }); + .collect(); + + let sb_cols: Vec = line + .iter() + .enumerate() + .filter(|(_, c)| is_scrollbar_glyph(**c)) + .map(|(i, _)| i) + .collect(); + assert_eq!( + sb_cols.len(), + 1, + "a pane with the minimap on must show exactly one vertical \ + scrollbar column on row {row}, got {sb_cols:?}; screen:\n{screen}" + ); - let next = line.chars().nth(editor_sb_col + 1); + let next = line.get(sb_cols[0] + 1).copied(); assert!( - next.is_some_and(is_scrollbar_glyph), - "the minimap's own scroll thumb must paint a '█'/'░' glyph \ - immediately right of the editor's own scrollbar column \ - ({editor_sb_col}), got {next:?}; screen:\n{screen}" + next.is_some_and(is_braille), + "the minimap strip must begin in the column immediately right of \ + the scrollbar ({}), painting braille rather than a second bar; \ + got {next:?}; screen:\n{screen}", + sb_cols[0] ); } - /// #723 part A acceptance, the other half: a file that fits entirely - /// within the viewport must paint no scroll affordance anywhere — - /// neither the minimap's thumb nor the editor's own scrollbar, both - /// gated on the same "does it overflow" condition. + /// #723 acceptance, the other half: a file that fits entirely within the + /// viewport must paint no scroll affordance anywhere — neither the + /// editor's own scrollbar nor a thumb over the minimap strip. #[test] fn minimap_paints_no_scroll_thumb_when_the_file_fits_via_shell_app() { let mut driver = driver_with_shell(app_with_short_buffer(), config(), 100, 24); diff --git a/src/tui_main/snapshots/minimap_braille.snap b/src/tui_main/snapshots/minimap_braille.snap index 6bc05716..b17c0451 100644 --- a/src/tui_main/snapshots/minimap_braille.snap +++ b/src/tui_main/snapshots/minimap_braille.snap @@ -4,17 +4,17 @@ expression: "lines.join(\"\\n\")" --- 󰍜  [No Name] × 󰤲  ⋯ ▎ -  line 0 █ █⣿⣿⣿⣿⣿⣿⣿⣿⡇⠀⢸⣿⣿ -  line 1 █ █⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ -  line 2 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ -  line 3 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ -  line 4 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 5 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 6 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 7 ░ ░⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ - line 8 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ - line 9 ░ ░⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ - line 10 ░ ░⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ - line 11 ░ ░⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ +  line 0 █ ⣿⣿⣿⣿⣿⣿⣿⣿⣿⡇⠀⢸⣿⣿ +  line 1 █ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ +  line 2 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ +  line 3 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ +  line 4 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 5 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 6 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 7 ░ ⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⢸⣿ + line 8 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ + line 9 ░ ⣿⣿⣿⣿⣿⣿⣿⣿⠀⠀⣿⣿⣿⣿ + line 10 ░ ⣿⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ + line 11 ░ ⣿⣿⣿⣿⣿⣿⣿⠀⢸⣿⣿⣿⣿⣿ NORMAL [No Name] 󰆍 󰘖 utf-8 LF Spaces: 4 Ln 1, Col 1 