From 256bb19b23f26412b421402c273d428681ecd9b0 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Wed, 2 Sep 2026 14:14:17 -0500 Subject: [PATCH 1/3] =?UTF-8?q?#754:=20converge=20the=20panels=20rung=20?= =?UTF-8?q?=E2=80=94=20bottom=20panel,=20activity=20bar,=20sidebar=20owner?= =?UTF-8?q?=20+=20hover?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 4 of the #733 mouse ladder. WIP: production convergence, tests follow. New shared API (`src/render.rs`, "Panels rung (#754)"): - `quickfix_panel_rows` — one statement of the quickfix band height, the painter's. - `BottomPanelMetrics` / `BottomPanelRoute` / `route_bottom_panel_click` / `apply_bottom_panel_route` / `terminal_scrollback_drag_target` — the tab-strip / toolbar / split / pane ladder, once. - `SidebarOwner` / `sidebar_owner` — who owns the sidebar body, once. - `apply_activity_panel_switch` — the activity-bar icon activation, once. - `SidebarBodyGeometry` / `route_sidebar_hover` — the SC + ext-panel hover rung, which existed only on TUI. Both backends' bespoke arms are deleted in this commit. Co-Authored-By: Claude Opus 5 --- src/gtk/mod.rs | 230 ++++++++++--------- src/render.rs | 499 ++++++++++++++++++++++++++++++++++++++++++ src/tui_main/mouse.rs | 349 +++++++++-------------------- 3 files changed, 719 insertions(+), 359 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index f6e1c466..f6ffe190 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -47,10 +47,6 @@ fn is_ext_panel_id(id: &str) -> bool { id.starts_with("ext:") } -fn ext_panel_name(id: &str) -> Option<&str> { - id.strip_prefix("ext:") -} - type TabSlotMap = HashMap>; /// Pango font family for UI panels (menu bar, sidebars, dropdown, @@ -2405,6 +2401,19 @@ impl App { self.cached_editor_bounds.get() } + /// Left edge of the bottom panel as last painted — the same `x` + /// `render_content` hands `draw_tab_bar` / the terminal pane, i.e. the + /// editor's left edge, right of the activity bar and sidebar. + /// + /// [`render::BottomPanelMetrics::panel_left`] (#754). Falls back to `0.0` + /// before the first frame, when there is no panel on screen to click. + fn painted_bottom_panel_left(&self) -> f64 { + self.cached_editor_bounds + .get() + .map(|(r, _)| r.x) + .unwrap_or(0.0) + } + /// Both divider lists for the frame just painted, plus whether `(x, y)` /// lands on a group's tab bar — everything /// [`render::route_divider_grab`] needs from this backend. @@ -2982,68 +2991,40 @@ impl App { // version of this clear was incomplete; tracked all fields via // `clear_sidebar_focus()` instead. self.engine.borrow_mut().clear_sidebar_focus(); - // Check if click lands in the terminal panel before general handling. - // Layout (bottom to top): status | toolbar | terminal | quickfix | DAP | editor - // Geometry is cached at paint time on engine.bottom_panel_geometry (#418). - let zone = self.engine.borrow().resolve_bottom_panel_zone(y); - if let Some(zone) = zone { - use crate::core::engine::BottomPanelZone; - if matches!(zone, BottomPanelZone::TabBar) { - self.engine.borrow_mut().handle_bottom_tab_bar_click(x); + // ── Bottom panel (tab strip / toolbar / terminal content) — #754 ── + // Zone, split hit-test and pane-cell translation are all + // `render::route_bottom_panel_click`, shared verbatim with TUI's + // `handle_mouse`. What this replaced computed the pane column as a + // bare `x / cached_char_width` against a *window-absolute* `x`, + // while `render_content` paints the panel at the editor's left + // edge — so with the sidebar open every terminal click landed + // roughly `(activity_bar + sidebar) / char_width` columns right of + // the glyph aimed at. `panel_left` is now a required input. + let route = render::route_bottom_panel_click( + &self.engine.borrow(), + x, + y, + render::BottomPanelMetrics { + panel_left: self.painted_bottom_panel_left(), + col_width: self.cached_char_width.max(1.0), + }, + ); + if let Some(route) = route { + if !matches!(route, render::BottomPanelRoute::TabBar) { + self.terminal_resize_dragging = false; + } + let ctx = crate::core::engine::UiEventContext { + terminal_cols: self.terminal_cols(), + terminal_max_rows: self.terminal_target_maximize_rows(), + }; + let effect = + render::apply_bottom_panel_route(&mut self.engine.borrow_mut(), route, x, ctx); + self.terminal_split_dragging |= effect.split_drag; + self.terminal_resize_dragging |= effect.resize_drag; + if effect.relayout { self.handle_resize(); return; } - self.engine.borrow_mut().terminal_has_focus = true; - if let BottomPanelZone::Content { .. } = zone { - let split_layout = *self.engine.borrow().terminal_split_layout.borrow(); - if let Some(ref sl) = split_layout { - let hit = sl.hit_test(x as f32, y as f32); - // #533: pass button/mods so the engine can - // forward_mouse(Press) to the child when it has mouse - // reporting enabled. - if self.engine.borrow_mut().handle_terminal_split_click( - hit, - quadraui::MouseButton::Left, - quadraui::Modifiers::default(), - ) { - self.terminal_split_dragging = true; - } - } else { - self.terminal_resize_dragging = false; - let col = (x / self.cached_char_width.max(1.0)) as u16; - let row_offset = match zone { - BottomPanelZone::Content { row_offset } => row_offset, - _ => 0, - }; - // #533: shared press handler — tries forward_mouse(Press) - // when the child has mouse reporting, falls back to - // terminal_scroll_reset + local selection start. - self.engine.borrow_mut().handle_terminal_pane_press( - col, - row_offset, - quadraui::MouseButton::Left, - quadraui::Modifiers::default(), - ); - } - } else { - // Header row — dispatch through cached toolbar hit regions. - let action = self.engine.borrow().resolve_terminal_toolbar_click(x); - let ctx = crate::core::engine::UiEventContext { - terminal_cols: self.terminal_cols(), - terminal_max_rows: self.terminal_target_maximize_rows(), - }; - if !self - .engine - .borrow_mut() - .execute_terminal_toolbar_action(action, ctx) - && matches!( - action, - crate::core::engine::TerminalToolbarAction::StartResize - ) - { - self.terminal_resize_dragging = true; - } - } self.draw_needed.set(true); } else { { @@ -3984,40 +3965,19 @@ impl App { } /// Switch the sidebar to a different panel. + /// + /// #754: the ext-panel-vs-built-in bookkeeping this used to spell out is + /// `render::apply_activity_panel_switch`, shared with TUI's activity-bar + /// arm. The only thing left here is this backend's own widget re-sync, + /// which differs by branch (a plugin panel does not move + /// `app_shell.active_panel_id()`, so `sync_sidebar_from_engine` has nothing + /// to sync for it). fn switch_panel(&mut self, panel_id: String) { - if let Some(name) = ext_panel_name(&panel_id) { - // Extension panels bypass AppShell (no dynamic registration). - let mut engine = self.engine.borrow_mut(); - let same = engine.ext_panel_active.as_deref() == Some(name) - && engine.app_shell.sidebar_visible(); - if same { - engine.app_shell.hide_sidebar(); - engine.ext_panel_has_focus = false; - engine.ext_panel_active = None; - } else { - if !engine.app_shell.sidebar_visible() { - engine.app_shell.toggle_sidebar(); - } - let already = engine.ext_panel_active.as_deref() == Some(name); - engine.ext_panel_has_focus = true; - engine.ext_panel_active = Some(name.to_string()); - if !already { - engine.ext_panel_selected = 0; - engine.plugin_event("panel_focus", name); - } - } - engine.session.explorer_visible = engine.app_shell.sidebar_visible(); - let _ = engine.session.save(); - drop(engine); - let _ = panel_id; // engine.ext_panel_active drives `current_active_panel_id()` + let is_ext = panel_id.starts_with("ext:"); + render::apply_activity_panel_switch(&mut self.engine.borrow_mut(), &panel_id); + if is_ext { self.sync_sidebar_widgets(); } else { - { - let mut engine = self.engine.borrow_mut(); - engine.ext_panel_has_focus = false; - engine.ext_panel_active = None; - engine.toggle_sidebar_panel(&panel_id); - } self.sync_sidebar_from_engine(); } } @@ -4320,28 +4280,21 @@ impl App { return true; } - // Which panel owns the sidebar body? Derived exactly as - // `render_content` derives it, so the click router and the painter can - // never disagree about who is on screen. - let active_id: String = { - let engine = self.engine.borrow(); - if let Some(ref name) = engine.ext_panel_active { - format!("ext:{name}") - } else { - engine - .app_shell - .active_panel_id() - .map(|id| id.as_str().to_string()) - .unwrap_or_else(|| PANEL_EXPLORER.to_string()) - } - }; + // Which panel owns the sidebar body? `render::sidebar_owner` states + // that precedence once (#754) — `ext_panel_active` first, then + // `app_shell.active_panel_id()`, Explorer as the fallback — so the + // click router, the hover router and the painter can never disagree + // about who is on screen. This used to be an inline `format!("ext:{}")` + // here and an `if …is_some() / else if active_panel_is(…)` chain on + // TUI. + let owner = render::sidebar_owner(&self.engine.borrow()); - let consumed = match active_id.as_str() { - PANEL_EXPLORER => { + let consumed = match &owner { + render::SidebarOwner::Explorer => { self.explorer_ui_event(event.clone()); true } - PANEL_SEARCH => { + render::SidebarOwner::Search => { let mut engine = self.engine.borrow_mut(); if is_press { engine.search_set_focus(true); @@ -4349,9 +4302,9 @@ impl App { engine.handle_search_sidebar_ui_event(event.clone()); true } - PANEL_DEBUG => self.route_debug_sidebar_event(event), - PANEL_GIT => self.route_sc_sidebar_event(event), - PANEL_EXTENSIONS => { + render::SidebarOwner::Debug => self.route_debug_sidebar_event(event), + render::SidebarOwner::Git => self.route_sc_sidebar_event(event), + render::SidebarOwner::Extensions => { let mut engine = self.engine.borrow_mut(); if is_press { engine.ext_sidebar_has_focus = true; @@ -4362,7 +4315,7 @@ impl App { } true } - PANEL_SETTINGS => { + render::SidebarOwner::Settings => { let mut engine = self.engine.borrow_mut(); if is_press { engine.settings_has_focus = true; @@ -4379,7 +4332,7 @@ impl App { render::handle_settings_form_ui_event(&mut engine, event, sb); true } - id if is_ext_panel_id(id) => { + render::SidebarOwner::ExtPanel(_) => { // Plugin-provided panel: `render_content` paints it through the // same `ext_sidebar_system` at the same rect, so it routes the // same way. @@ -4390,11 +4343,11 @@ impl App { engine.handle_ext_sidebar_ui_event(event.clone()); true } - PANEL_AI => self.route_ai_sidebar_event(event), + render::SidebarOwner::Ai => self.route_ai_sidebar_event(event), // Unknown panel id: nothing was painted, so there is nothing // for a click to hit — let it fall through rather than // swallow it. - _ => false, + render::SidebarOwner::Unknown => false, }; if consumed { @@ -6900,6 +6853,45 @@ impl quadraui::ShellApp for App { backend.set_cursor(shape); } + // ── Sidebar hover — #754 rung ───────────────────────────────────── + // This backend already *painted* `screen.panel_hover` (the + // `RichTextPopup` block in `render_content`) and already tracked the + // popup's own rect, but nothing on this side ever set + // `engine.panel_hover` or `engine.sc_button_hovered`: the router was + // ~78 lines of TUI-only code. That is the #499/#484 mechanism — paint + // without input on one backend, input without a second painter on the + // other. `render::route_sidebar_hover` is now the single router and + // both backends call it. + if let UiEvent::MouseMoved { position, .. } = &event { + if let Some(sb) = ctx.layout.sidebar_content_bounds { + let lh = backend.line_height(); + let on_popup = self + .panel_hover_popup_rect + .get() + .is_some_and(|(px, py, pw, ph)| { + let (mx, my) = (position.x as f64, position.y as f64); + mx >= px && mx < px + pw && my >= py && my < py + ph + }); + let owner = render::sidebar_owner(&self.engine.borrow()); + let changed = render::route_sidebar_hover( + &mut self.engine.borrow_mut(), + &owner, + position.x, + position.y, + render::SidebarBodyGeometry { + bounds: sb, + row_h: lh.max(1.0), + header_rows: 1.0, + }, + true, + on_popup, + ); + if changed { + self.draw_needed.set(true); + } + } + } + // ── CSD titlebar background: drag-to-move / double-click-maximize ── // (quadraui#400) + outer window border: edge-resize (quadraui#406). // Runs after the menu-item intercept and the window-control-button diff --git a/src/render.rs b/src/render.rs index 93cd2db7..272b0262 100644 --- a/src/render.rs +++ b/src/render.rs @@ -3532,6 +3532,505 @@ impl TabDragState { } } +// ═══ Panels rung (#754, mouse-ladder slice 4) ════════════════════════════════ +// +// The rung beneath the divider rung above: once no modal, no chrome band and no +// resize handle has claimed the point, the *panels* get a look — the bottom +// panel (terminal / debug output and the tab strip above them), the activity +// bar's sidebar band, and the sidebar's own hover feedback. +// +// What was wrong with the two transcriptions this replaces: +// +// 1. **The quickfix band height had three different rules in one binary.** +// The painter reserves rows only for a quickfix that has something in it +// (`compute_editor_layout`: `quickfix_open && !quickfix_items.is_empty()`), +// but TUI's mouse handler asked `if engine.quickfix_open { 6 }` in **four** +// separate places. `:copen` on an empty list therefore moved every band +// *below* the editor — the terminal strip, the separated status line, the +// terminal-resize clamp — six rows away from where they were painted, so +// clicks in the bottom sixth of the screen hit the wrong surface entirely. +// [`quickfix_panel_rows`] is now the one rule, and it is the painter's. +// +// 2. **GTK measured the terminal pane's columns from the wrong origin.** +// `render_content` paints the bottom panel at the *editor's* left edge +// (right of the activity bar and sidebar), and TUI's handler duly did +// `col.saturating_sub(editor_left)` before calling +// `Engine::handle_terminal_pane_press`. GTK's arm did a bare +// `x / cached_char_width` against a window-absolute `x`, so with the +// sidebar open every click in the terminal landed ~`(activity_bar + +// sidebar) / char_width` columns to the right of the glyph the user aimed +// at — and the further right you clicked, the further the terminal's own +// selection anchor drifted. [`BottomPanelMetrics::panel_left`] makes the +// origin an input the caller must state, so it cannot be forgotten again. +// +// 3. **The `terminal_open` gate disagreed.** TUI refused to route Toolbar / +// Content presses unless `engine.terminal_open`, GTK routed them whenever +// geometry existed. `Engine::resolve_bottom_panel_zone` already returns +// `None` when the panel is not painted, which is the question both gates +// were badly approximating, so the converged router asks only that. +// +// 4. **The sidebar hover rung existed on TUI and was blank on GTK** — the +// mechanism behind #499/#484. `sc_panel_layout` is populated by the +// *shared* painter (`draw_sc_sidebar_panel`), so the geometry a hover needs +// was already cross-backend; only the ~78 lines that read it were +// TUI-only. [`route_sidebar_hover`] is that code, once, in the caller's own +// units. +// +// **Deliberately still per backend**: TUI's terminal scrollback scrollbar. +// `TerminalSplitHit::Scrollbar` is resolved by the shared split layout, but +// only the TUI paints a scrollback track, so only the TUI arms a drag for it. +// The *geometry* of that drag is stated here anyway +// ([`terminal_scrollback_drag_target`]) so the day GTK grows a track it reads +// the same numbers, rather than re-deriving them from `bottom_panel_geometry` +// by hand the way both backends historically did with everything else. + +/// Rows the quickfix panel occupies, as the **painter** reserves them. +/// +/// The single source of truth for "how tall is the quickfix band" on the +/// mouse-routing side, matching `compute_editor_layout`'s `quickfix_rows` +/// exactly — including the `!quickfix_items.is_empty()` term that TUI's four +/// hand-rolled `if engine.quickfix_open { 6 }` copies all omitted (see this +/// section's banner, point 1). +pub fn quickfix_panel_rows(engine: &Engine) -> u16 { + if engine.quickfix_open && !engine.quickfix_items.is_empty() { + 6 + } else { + 0 + } +} + +/// The caller's own bottom-panel geometry, in the caller's own units. +/// +/// Everything else the router needs is already cached on the engine at paint +/// time (`bottom_panel_geometry`, `terminal_split_layout`) by whichever backend +/// painted the frame, so these two numbers are the whole of the per-backend +/// input. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct BottomPanelMetrics { + /// Left edge of the painted panel — the editor's left edge, i.e. right of + /// the activity bar and sidebar. Cell column on TUI, pixels on GTK. + /// + /// Pane presses are reported *panel-relative* because that is what + /// `Engine::handle_terminal_pane_press` documents ("0-based cells within + /// the pane"); getting this wrong is bug 2 in the section banner. + pub panel_left: f64, + /// Width of one content column: `1.0` on TUI, the char advance on GTK. + pub col_width: f64, +} + +/// Where a press inside the bottom panel landed. +/// +/// Resolved from geometry alone — no engine mutation — so a caller can look +/// before it leaps (GTK's right-click path needs "is this the terminal?" +/// without actually driving the terminal). +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum BottomPanelRoute { + /// The shared "TERMINAL / DEBUG CONSOLE" tab strip. + TabBar, + /// The per-panel toolbar row (terminal tab strip or find bar). + Toolbar, + /// A hit inside a split terminal's own layout — divider, pane gutter or + /// scrollback track. + Split(quadraui::TerminalSplitHit), + /// A plain (unsplit) terminal pane press, already translated into + /// pane-local cells. + Pane { col: u16, row_offset: u16 }, +} + +/// Resolve a press at `(x, y)` against the bottom panel as last painted. +/// +/// Returns `None` when the point is outside the panel — which is also the +/// answer when no panel is painted at all, because +/// `Engine::resolve_bottom_panel_zone` reads the geometry the painter cached +/// and clears (see the section banner, point 3: this replaced two different +/// `terminal_open` gates that were both trying to ask this). +pub fn route_bottom_panel_click( + engine: &Engine, + x: f64, + y: f64, + metrics: BottomPanelMetrics, +) -> Option { + use crate::core::engine::BottomPanelZone; + if x < metrics.panel_left { + return None; + } + let zone = engine.resolve_bottom_panel_zone(y)?; + let geom = (*engine.bottom_panel_geometry.borrow())?; + Some(match zone { + BottomPanelZone::TabBar => BottomPanelRoute::TabBar, + BottomPanelZone::Toolbar => BottomPanelRoute::Toolbar, + BottomPanelZone::Content { row_offset } => { + let split = *engine.terminal_split_layout.borrow(); + if let Some(sl) = split { + // The split layout hit-tests in the *absolute* space it was + // built in. TUI reconstructs that y from the cached geometry + // (its `row_offset` has already lost the panel origin); + // recomputing it here rather than at each call site is what + // stops the two backends drifting on which `y` they pass. + let abs_y = geom.top_y + geom.content_y + row_offset as f64 * geom.content_row_h; + BottomPanelRoute::Split(sl.hit_test(x as f32, abs_y as f32)) + } else { + let col = ((x - metrics.panel_left) / metrics.col_width.max(f64::EPSILON)) as u16; + BottomPanelRoute::Pane { col, row_offset } + } + } + }) +} + +/// The scrollback-scrollbar drag a [`quadraui::TerminalSplitHit::Scrollbar`] +/// arms, in the caller's own units. +/// +/// Only TUI paints a scrollback track today, so only TUI calls this — but the +/// numbers are derived from the same cached `bottom_panel_geometry` both +/// backends write, so a future GTK track needs no new arithmetic. +pub fn terminal_scrollback_drag_target(engine: &Engine) -> Option { + let geom = (*engine.bottom_panel_geometry.borrow())?; + let track_start = (geom.top_y + geom.content_y) as f32; + let track_length = (geom.height - geom.content_y).max(0.0) as f32; + let total = engine + .active_terminal() + .map(|t| t.history_len()) + .unwrap_or(0); + Some(quadraui::DragTarget::ScrollbarY { + widget: quadraui::WidgetId::new("terminal_scrollback"), + track_start, + track_length, + thumb_length: (track_length / total.max(1) as f32).max(1.0), + max_scroll: total, + grab_offset: 0.0, + inverted: true, + }) +} + +/// What the caller still has to do after [`apply_bottom_panel_route`]. +/// +/// Both backends kept these as bare `bool` locals set from inside the arm +/// (`*dragging_terminal_resize`, `self.terminal_resize_dragging`, …); returning +/// them makes the arm's contract explicit and stops one backend quietly +/// growing a follow-up the other lacks. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct BottomPanelEffect { + /// A terminal-split divider grab started — track it until mouse-up. + pub split_drag: bool, + /// The panel's own resize handle was grabbed. + pub resize_drag: bool, + /// The panel's tab strip changed which panel is showing, so the caller + /// must re-run its layout (GTK's `handle_resize`). + pub relayout: bool, +} + +/// Apply a resolved [`BottomPanelRoute`]. +/// +/// `toolbar_ctx` is the caller's terminal sizing context, used only by the +/// `Toolbar` arm. Focus bookkeeping (`terminal_has_focus`) is done here so both +/// backends agree on it — TUI used to set it only in the `Toolbar` arm and GTK +/// for every zone but `TabBar`. +pub fn apply_bottom_panel_route( + engine: &mut Engine, + route: BottomPanelRoute, + x: f64, + toolbar_ctx: crate::core::engine::UiEventContext, +) -> BottomPanelEffect { + use crate::core::engine::TerminalToolbarAction; + let mut effect = BottomPanelEffect::default(); + match route { + BottomPanelRoute::TabBar => { + engine.handle_bottom_tab_bar_click(x); + effect.relayout = true; + } + BottomPanelRoute::Toolbar => { + engine.terminal_has_focus = true; + let action = engine.resolve_terminal_toolbar_click(x); + if !engine.execute_terminal_toolbar_action(action, toolbar_ctx) + && matches!(action, TerminalToolbarAction::StartResize) + { + effect.resize_drag = true; + } + } + BottomPanelRoute::Split(hit) => { + engine.terminal_has_focus = true; + // #533: the button/mods are passed so a split-pane click can + // `forward_mouse(Press)` to a child that has mouse reporting on. + effect.split_drag = engine.handle_terminal_split_click( + hit, + quadraui::MouseButton::Left, + quadraui::Modifiers::default(), + ); + } + BottomPanelRoute::Pane { col, row_offset } => { + engine.terminal_has_focus = true; + engine.handle_terminal_pane_press( + col, + row_offset, + quadraui::MouseButton::Left, + quadraui::Modifiers::default(), + ); + } + } + effect +} + +/// Which panel owns the sidebar body this frame. +/// +/// Derived exactly once, from the same two engine fields `render_content` / +/// `render_sidebar` dispatch on, so the click router, the hover router and the +/// painter can never disagree about who is on screen. The precedence — +/// `ext_panel_active` **first**, `app_shell.active_panel_id()` second, +/// Explorer as the fallback — is the rule both backends had written out +/// longhand (TUI as an `if ext_panel_name.is_some() … else if +/// active_panel_is(…)` chain, GTK as a `format!("ext:{name}")` string). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SidebarOwner { + Explorer, + Search, + Debug, + Git, + Extensions, + Settings, + Ai, + /// A plugin-provided panel, by bare name (no `ext:` prefix). + ExtPanel(String), + /// A panel id nothing paints — a click on it belongs to whatever is + /// underneath, not to the sidebar. + Unknown, +} + +impl SidebarOwner { + /// The `app_shell` panel id this owner corresponds to, for the arms that + /// still need to talk to the engine in strings. + pub fn panel_id(&self) -> Option<&'static str> { + use crate::core::engine::sidebar::*; + Some(match self { + SidebarOwner::Explorer => PANEL_EXPLORER, + SidebarOwner::Search => PANEL_SEARCH, + SidebarOwner::Debug => PANEL_DEBUG, + SidebarOwner::Git => PANEL_GIT, + SidebarOwner::Extensions => PANEL_EXTENSIONS, + SidebarOwner::Settings => PANEL_SETTINGS, + SidebarOwner::Ai => PANEL_AI, + SidebarOwner::ExtPanel(_) | SidebarOwner::Unknown => return None, + }) + } +} + +/// Resolve [`SidebarOwner`] for the current frame. +pub fn sidebar_owner(engine: &Engine) -> SidebarOwner { + use crate::core::engine::sidebar::*; + if let Some(name) = engine.ext_panel_active.as_ref() { + return SidebarOwner::ExtPanel(name.clone()); + } + let id = engine + .app_shell + .active_panel_id() + .map(|id| id.as_str().to_string()) + .unwrap_or_else(|| PANEL_EXPLORER.to_string()); + match id.as_str() { + PANEL_EXPLORER => SidebarOwner::Explorer, + PANEL_SEARCH => SidebarOwner::Search, + PANEL_DEBUG => SidebarOwner::Debug, + PANEL_GIT => SidebarOwner::Git, + PANEL_EXTENSIONS => SidebarOwner::Extensions, + PANEL_SETTINGS => SidebarOwner::Settings, + PANEL_AI => SidebarOwner::Ai, + other => match other.strip_prefix("ext:") { + Some(name) => SidebarOwner::ExtPanel(name.to_string()), + None => SidebarOwner::Unknown, + }, + } +} + +/// What an activity-bar panel switch left behind, for the caller's own +/// sidebar bookkeeping. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ActivityPanelSwitch { + /// Whether the sidebar is showing after the switch. + pub sidebar_visible: bool, + /// The plugin panel now owning the sidebar body, if any. TUI mirrors this + /// into `TuiSidebar::ext_panel_name`. + pub ext_panel: Option, +} + +/// Activate the activity-bar item for `panel_id` — the click-on-an-icon +/// behaviour, stated once. +/// +/// `panel_id` is either a built-in `PANEL_*` id or an `ext:{name}` plugin +/// panel; the two need different bookkeeping because plugin panels bypass +/// `AppShell` entirely (there is no dynamic `PanelDefinition` to `show_panel`), +/// and *that* asymmetry is the whole reason both backends had grown their own +/// copy — TUI in the `ActivityBarTarget` match inside `handle_mouse`, GTK in +/// `App::switch_panel`. The two copies had drifted on both halves: +/// +/// * **#637's focus clear was TUI-only.** A plugin panel taking over the +/// sidebar body must drop whatever panel's focus flag was left set, because +/// `app_shell`'s active-panel id is deliberately *not* moved for a plugin +/// panel and so nothing else clears it. Without it a stale +/// `ext_sidebar_has_focus` from an earlier visit to the Extensions +/// marketplace keeps `active_panel_is(PANEL_EXTENSIONS)`'s `SidebarSystem` +/// intercept looking focused while a completely different panel is on +/// screen — GTK had that bug for as long as it had `switch_panel`. +/// * **the re-entry guard was GTK-only.** TUI reset `ext_panel_selected` to 0 +/// and re-fired `plugin_event("panel_focus", …)` on *every* activation, +/// including one that merely re-showed the panel already active, so +/// clicking a plugin icon twice scrolled its list back to the top and made +/// the plugin see a spurious second focus event. +/// +/// Both are fixed here, in the one copy. +pub fn apply_activity_panel_switch(engine: &mut Engine, panel_id: &str) -> ActivityPanelSwitch { + match panel_id.strip_prefix("ext:") { + Some(name) => { + let already_showing = engine.ext_panel_active.as_deref() == Some(name); + if already_showing && engine.app_shell.sidebar_visible() { + // Second click on the active plugin panel's icon — VS Code + // hides the sidebar rather than re-showing it. + engine.app_shell.hide_sidebar(); + engine.ext_panel_has_focus = false; + engine.ext_panel_active = None; + } else { + engine.clear_sidebar_focus(); + if !engine.app_shell.sidebar_visible() { + engine.toggle_sidebar(); + } + engine.ext_panel_active = Some(name.to_string()); + engine.ext_panel_has_focus = true; + if !already_showing { + engine.ext_panel_selected = 0; + engine.plugin_event("panel_focus", name); + } + } + } + None => { + engine.ext_panel_has_focus = false; + engine.ext_panel_active = None; + engine.toggle_sidebar_panel(panel_id); + } + } + engine.session.explorer_visible = engine.app_shell.sidebar_visible(); + let _ = engine.session.save(); + ActivityPanelSwitch { + sidebar_visible: engine.app_shell.sidebar_visible(), + ext_panel: engine.ext_panel_active.clone(), + } +} + +/// The painted sidebar body, in the caller's own units, plus the row pitch a +/// list panel inside it uses. +/// +/// TUI passes cells (`row_h == 1.0`); GTK passes the pixel +/// `ShellContext::layout.sidebar_content_bounds` and its line height. Both +/// numbers come from the frame that was actually painted — never re-derived — +/// which is the rule that keeps hover highlight and hover *content* on the same +/// row (CLAUDE.md rule 1's failure mode, one frame earlier). +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct SidebarBodyGeometry { + pub bounds: quadraui::Rect, + /// Height of one list row. + pub row_h: f32, + /// Rows of panel chrome above the first content row. An ext panel paints a + /// one-row header; a backend that paints more says so here rather than + /// baking the offset into its own index arithmetic. + pub header_rows: f32, +} + +impl SidebarBodyGeometry { + /// Content-row index under `y`, or `None` when `y` is in the chrome above + /// the first content row (or outside the body entirely). + fn content_row(&self, y: f32) -> Option { + if self.row_h <= 0.0 || y < self.bounds.y || y >= self.bounds.y + self.bounds.height { + return None; + } + let rel = ((y - self.bounds.y) / self.row_h).floor() - self.header_rows; + (rel >= 0.0).then_some(rel as usize) + } + + fn contains_x(&self, x: f32) -> bool { + x >= self.bounds.x && x < self.bounds.x + self.bounds.width + } +} + +/// Hover feedback for the sidebar — the Source Control toolbar buttons and +/// section rows, and plugin ext-panel rows. +/// +/// This is the rung the issue calls out as "blank on GTK": the code below ran +/// only on TUI, so #499/#484's ext-panel hover cards and the SC button +/// highlight simply did not exist in the GUI, and any fix to one side was +/// invisible on the other. +/// +/// `mouse_on_popup` is the caller's own answer to "is the pointer over the +/// hover card itself" — dismissing while the pointer is on the card is what +/// makes a hover card impossible to read. +/// +/// Returns `true` when the pointer was inside the sidebar body (so the caller +/// can skip its editor-hover rungs). +pub fn route_sidebar_hover( + engine: &mut Engine, + owner: &SidebarOwner, + x: f32, + y: f32, + geometry: SidebarBodyGeometry, + sidebar_visible: bool, + mouse_on_popup: bool, +) -> bool { + let inside = sidebar_visible && geometry.contains_x(x); + match owner { + SidebarOwner::Git if inside => { + // Route via the cached `SidebarPanelLayout` (#509) — no per-frame + // arithmetic, and it hit-tests in the same absolute space the + // shared painter built it in, which is why this works unchanged on + // both backends. + let hit = { + let layout = engine.sc_panel_layout.borrow(); + layout.as_ref().map(|l| l.hit_test(x, y)) + }; + match hit { + Some(quadraui::SidebarPanelHit::ToolbarButton(_)) + | Some(quadraui::SidebarPanelHit::ToolbarEmpty) => { + engine.sc_button_hovered = engine.sc_button_hit(x, y); + if !mouse_on_popup { + engine.dismiss_panel_hover(); + } + } + Some(quadraui::SidebarPanelHit::Content { y: content_y, .. }) => { + engine.sc_button_hovered = None; + if let Some((flat_idx, _is_header)) = + engine.sc_content_row_to_flat(content_y as usize, true) + { + engine.panel_hover_mouse_move("source_control", "", flat_idx); + } else if !mouse_on_popup { + engine.dismiss_panel_hover(); + } + } + _ => { + engine.sc_button_hovered = None; + if !mouse_on_popup { + engine.dismiss_panel_hover(); + } + } + } + } + SidebarOwner::ExtPanel(name) if inside => { + let name = name.clone(); + match geometry.content_row(y) { + Some(row) => { + let flat_idx = engine.ext_panel_scroll_top + row; + engine.panel_hover_mouse_move(&name, "", flat_idx); + } + // Header row: nothing to hover. + None if !mouse_on_popup => engine.dismiss_panel_hover(), + None => {} + } + } + // Pointer left the panel that owns the hover card (or the sidebar + // entirely) — drop it, unless the pointer is *on* the card. + _ => { + engine.sc_button_hovered = None; + if engine.panel_hover.is_some() && !mouse_on_popup { + engine.dismiss_panel_hover(); + } + } + } + inside +} + // ─── Overlay-band composition (#735 slice 1) ────────────────────────────────── // // The paint twin of `route_modal_overlay_click` / `route_modal_key` above, and diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index fc9a2573..314ecd55 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -857,7 +857,7 @@ pub(super) fn handle_mouse( } // Terminal panel resize drag if *dragging_terminal_resize { - let qf_h: u16 = if engine.quickfix_open { 6 } else { 0 }; + let qf_h: u16 = render::quickfix_panel_rows(engine); let available = term_height.saturating_sub(row + bottom_chrome + qf_h); // Leave at least 4 editor lines visible (+ menu/tab bar chrome) let min_editor_chrome = 4 + menu_rows + 1; // 4 lines + menu + tab bar @@ -983,7 +983,7 @@ pub(super) fn handle_mouse( // Only activate if the drag originated in the terminal (selection exists) // and the mouse is within the terminal panel bounds. { - let qf_rows: u16 = if engine.quickfix_open { 6 } else { 0 }; + let qf_rows: u16 = render::quickfix_panel_rows(engine); let strip_rows: u16 = if engine.terminal_open { super::effective_terminal_panel_rows_tui(engine, term_height) + 1 } else { @@ -1342,7 +1342,12 @@ pub(super) fn handle_mouse( // pre-#451 behavior and issue #575's "or no menu, if one isn't // implemented yet for that panel" expectation. if sb_visible && col >= ab_width && col < ab_width + sidebar_width { - if engine.active_panel_is(PANEL_EXPLORER) { + // #754: the same `render::sidebar_owner` the left-click arm and the + // hover arm ask. Strictly better than the `active_panel_is` this + // replaced for #575's own purpose: a plugin ext panel painted over + // the sidebar body leaves `active_panel_id` pointing at Explorer, + // so the old gate handed *its* right-clicks to the file tree too. + if render::sidebar_owner(engine) == render::SidebarOwner::Explorer { let sidebar_row = row.saturating_sub(menu_rows); let tree_row = sidebar_row as usize + engine.explorer_tree.borrow().scroll_offset(); if tree_row < engine.explorer_rows.len() { @@ -1413,7 +1418,7 @@ pub(super) fn handle_mouse( // Right-click on terminal panel → suppress (don't show editor context menu). { - let qf_rows: u16 = if engine.quickfix_open { 6 } else { 0 }; + let qf_rows: u16 = render::quickfix_panel_rows(engine); let strip_rows: u16 = if engine.terminal_open { super::effective_terminal_panel_rows_tui(engine, term_height) + 1 } else { @@ -1456,78 +1461,29 @@ pub(super) fn handle_mouse( engine.cancel_editor_hover_dismiss(); } - // ── SC button hover (mouse moved) ─────────────────────────────────────── + // ── Sidebar hover (mouse moved) — #754 rung ───────────────────────────── + // The SC toolbar/section hover and the plugin ext-panel row hover were ~78 + // lines here and *nothing at all* on GTK — the asymmetry behind #499/#484. + // Both are now `render::route_sidebar_hover`, which GTK calls too. if matches!(ev.kind, MouseEventKind::Moved) { - if sb_visible - && engine.active_panel_is(PANEL_GIT) - && col >= ab_width - && col < ab_width + sidebar_width - { - // Route via cached SidebarPanelLayout (#509) — no per-frame - // arithmetic; hit_test uses absolute terminal coordinates. - let hit = { - let layout = engine.sc_panel_layout.borrow(); - layout.as_ref().map(|l| l.hit_test(col as f32, row as f32)) - }; - match hit { - Some(quadraui::SidebarPanelHit::ToolbarButton(_)) - | Some(quadraui::SidebarPanelHit::ToolbarEmpty) => { - engine.sc_button_hovered = engine.sc_button_hit(col as f32, row as f32); - if !mouse_on_hover_popup { - engine.dismiss_panel_hover(); - } - } - Some(quadraui::SidebarPanelHit::Content { y: content_y, .. }) => { - engine.sc_button_hovered = None; - // content_y is content-local (row 0 = first section row). - let content_row = content_y as usize; - if let Some((flat_idx, _is_header)) = - engine.sc_content_row_to_flat(content_row, true) - { - engine.panel_hover_mouse_move("source_control", "", flat_idx); - } else if !mouse_on_hover_popup { - engine.dismiss_panel_hover(); - } - } - _ => { - engine.sc_button_hovered = None; - if !mouse_on_hover_popup { - engine.dismiss_panel_hover(); - } - } - } - } else { - engine.sc_button_hovered = None; - // If we were showing an SC hover and mouse left Git panel, dismiss - // — unless the mouse is over the popup itself. - if engine.panel_hover.is_some() && !mouse_on_hover_popup { - engine.dismiss_panel_hover(); - } - } - } - - // ── Ext panel hover (mouse moved) ─────────────────────────────────────── - if matches!(ev.kind, MouseEventKind::Moved) { - if sb_visible - && sidebar.ext_panel_name.is_some() - && col >= ab_width - && col < ab_width + sidebar_width - { - if let Some(ref panel_name) = sidebar.ext_panel_name.clone() { - let sidebar_row = row.saturating_sub(menu_rows); - // Row 0 is the header; content items start at row 1. - if sidebar_row >= 1 { - let flat_idx = - engine.ext_panel_scroll_top + (sidebar_row as usize).saturating_sub(1); - engine.panel_hover_mouse_move(panel_name, "", flat_idx); - } else if !mouse_on_hover_popup { - engine.dismiss_panel_hover(); - } - } - } else if sidebar.ext_panel_name.is_some() && !mouse_on_hover_popup { - // Mouse moved outside the ext panel area — dismiss hover. - engine.dismiss_panel_hover(); - } + render::route_sidebar_hover( + engine, + &render::sidebar_owner(engine), + col as f32, + row as f32, + render::SidebarBodyGeometry { + bounds: quadraui::Rect::new( + ab_width as f32, + menu_rows as f32, + sidebar_width as f32, + term_height.saturating_sub(menu_rows) as f32, + ), + row_h: 1.0, + header_rows: 1.0, + }, + sb_visible, + mouse_on_hover_popup, + ); } // ── Tab hover tooltip (mouse moved over tab bar) ──────────────────────── @@ -1815,17 +1771,9 @@ pub(super) fn handle_mouse( return sidebar_width; } - // ── Bottom panel tab bar click (shared row above Terminal / Debug Output) ── - // Geometry is cached at paint time on engine.bottom_panel_geometry (#418). - if col >= editor_left - && matches!( - engine.resolve_bottom_panel_zone(row as f64), - Some(crate::core::engine::BottomPanelZone::TabBar) - ) - { - engine.handle_bottom_tab_bar_click(col as f64); - return sidebar_width; - } + // #754: the bottom-panel tab-strip arm that used to sit here — and the + // ~95-line terminal-panel arm ~90 lines further down — are now one call to + // `render::route_bottom_panel_click`, below, shared with GTK. // ── Scroll-surface click dispatch (scrollbar thumb-drag + track-page). ── { @@ -1900,105 +1848,53 @@ pub(super) fn handle_mouse( // first status band `render::route_chrome_click` walks (above), and the // follow-up is `render::apply_status_action`. - // ── Terminal panel click ─────────────────────────────────────────────────── - // Zone resolved from cached geometry written at paint time (#418). Toolbar - // and content rows live inside the bottom panel area; their absolute y - // (e.g. for the scrollbar track) is recovered from the cached top_y. - if engine.terminal_open && col >= editor_left { - let zone = engine.resolve_bottom_panel_zone(row as f64); - let geom = *engine.bottom_panel_geometry.borrow(); - if let (Some(zone), Some(geom)) = (zone, geom) { - use crate::core::engine::BottomPanelZone; - // Tab bar was already dispatched above; only Toolbar / Content land here. - if matches!(zone, BottomPanelZone::Toolbar) { - // Header row — dispatch through cached toolbar hit regions. - engine.terminal_has_focus = true; - let action = engine.resolve_terminal_toolbar_click(col as f64); - let screen_h = terminal_size.map(|s| s.height).unwrap_or(24); - let panel_cols = terminal_size - .map(|s| s.width) - .unwrap_or(80) - .saturating_sub(editor_left); - let ctx = crate::core::engine::UiEventContext { - terminal_cols: panel_cols, - terminal_max_rows: super::terminal_target_maximize_rows_tui(engine, screen_h), - }; - if !engine.execute_terminal_toolbar_action(action, ctx) - && matches!( - action, - crate::core::engine::TerminalToolbarAction::StartResize - ) - { - *dragging_terminal_resize = true; - } - } else if let BottomPanelZone::Content { row_offset } = zone { - // Use cached TerminalSplitLayout for divider/pane/scrollbar - // detection (#430). Non-split fallback uses row_offset directly. - let split_layout = engine.terminal_split_layout.borrow(); - if let Some(ref sl) = *split_layout { - let abs_y = geom.top_y + geom.content_y + row_offset as f64; - let hit = sl.hit_test(col as f32, abs_y as f32); - drop(split_layout); - match hit { - quadraui::TerminalSplitHit::Scrollbar => { - let track_start = (geom.top_y + geom.content_y) as u16; - let track_len = (geom.height - geom.content_y).max(0.0) as u16; - let total = engine - .active_terminal() - .map(|t| t.history_len()) - .unwrap_or(0); - let tl = track_len as f32; - drag_state.begin(quadraui::DragTarget::ScrollbarY { - widget: quadraui::WidgetId::new("terminal_scrollback"), - track_start: track_start as f32, - track_length: tl, - thumb_length: (tl / total.max(1) as f32).max(1.0), - max_scroll: total, - grab_offset: 0.0, - inverted: true, - }); - apply_scrollbar_drag( - drag_state, - quadraui::Point { - x: col as f32, - y: row as f32, - }, - engine, - sidebar, - ); - } - _ => { - // #533: pass button/mods so split click can - // forward_mouse(Press) to the child when it - // has mouse reporting enabled. - if engine.handle_terminal_split_click( - hit, - quadraui::MouseButton::Left, - quadraui::Modifiers::default(), - ) { - *dragging_terminal_split = true; - } - } - } - } else { - drop(split_layout); - // #429/#533: focus + scroll reset + selection / mouse - // forwarding are owned by the engine. TUI still does - // the col conversion (panel is offset by the - // sidebar/activity-bar on the left). - let term_col = col.saturating_sub(editor_left); - engine.handle_terminal_pane_press( - term_col, - row_offset, - quadraui::MouseButton::Left, - quadraui::Modifiers::default(), - ); - } + // ── Bottom panel (tab strip / toolbar / terminal content) — #754 rung ───── + // Zone, split hit-test and pane-cell translation are all + // `render::route_bottom_panel_click`, shared verbatim with GTK. Only the + // scrollback scrollbar stays here: TUI is the only backend that paints a + // track for it (see the rung's banner in `render.rs`). + if let Some(route) = render::route_bottom_panel_click( + engine, + col as f64, + row as f64, + render::BottomPanelMetrics { + panel_left: editor_left as f64, + col_width: 1.0, + }, + ) { + if route == render::BottomPanelRoute::Split(quadraui::TerminalSplitHit::Scrollbar) { + if let Some(target) = render::terminal_scrollback_drag_target(engine) { + drag_state.begin(target); + apply_scrollbar_drag( + drag_state, + quadraui::Point { + x: col as f32, + y: row as f32, + }, + engine, + sidebar, + ); } return sidebar_width; } + let screen_h = terminal_size.map(|s| s.height).unwrap_or(24); + let effect = render::apply_bottom_panel_route( + engine, + route, + col as f64, + crate::core::engine::UiEventContext { + terminal_cols: terminal_size + .map(|s| s.width) + .unwrap_or(80) + .saturating_sub(editor_left), + terminal_max_rows: super::terminal_target_maximize_rows_tui(engine, screen_h), + }, + ); + *dragging_terminal_resize |= effect.resize_drag; + *dragging_terminal_split |= effect.split_drag; + return sidebar_width; } - // Click landed outside the terminal panel — return focus to the editor. + // Click landed outside the bottom panel — return focus to the editor. engine.terminal_has_focus = false; // ── Activity bar ────────────────────────────────────────────────────────── @@ -2029,62 +1925,31 @@ pub(super) fn handle_mouse( } return sidebar_width; } - Some(ActivityBarTarget::ExtensionPanel(name)) => { - if sidebar.ext_panel_name.as_deref() == Some(&name) - && engine.app_shell.sidebar_visible() - { - engine.app_shell.hide_sidebar(); - sidebar.ext_panel_name = None; - engine.ext_panel_has_focus = false; - engine.ext_panel_active = None; - } else { - // #637: a plugin panel taking over the sidebar body - // must drop whatever panel's focus flag (and, for - // Extensions, `active_panel_id`-derived state like - // `ext_sidebar_has_focus`) was left set from before — - // `app_shell`'s active-panel id is deliberately left - // untouched here (this isn't a `toggle_sidebar_panel` - // switch), so nothing else clears it. A stale - // `ext_sidebar_has_focus = true` left over from a - // previous visit to the Extensions marketplace panel - // otherwise keeps `active_panel_is(PANEL_EXTENSIONS)`'s - // SidebarSystem intercept looking "focused" even though - // this plugin panel is what's actually on screen. - engine.clear_sidebar_focus(); - sidebar.ext_panel_name = Some(name.clone()); - if !engine.app_shell.sidebar_visible() { - engine.toggle_sidebar(); - } - sidebar.has_focus = true; - engine.ext_panel_active = Some(name.clone()); - engine.ext_panel_has_focus = true; - engine.ext_panel_selected = 0; - engine.plugin_event("panel_focus", &name); - } - engine.session.explorer_visible = engine.app_shell.sidebar_visible(); - let _ = engine.session.save(); - return sidebar_width; - } _ => {} } + // #754: the ext-panel toggle and the built-in panel switch — which used + // to be ~50 lines here and a near-copy in GTK's `App::switch_panel` — + // are one call to `render::apply_activity_panel_switch`. let target_panel_id = match ab_target { - Some(ActivityBarTarget::Panel(p)) => Some(match p { - SidebarPanel::Explorer => PANEL_EXPLORER, - SidebarPanel::Search => PANEL_SEARCH, - SidebarPanel::Debug => PANEL_DEBUG, - SidebarPanel::Git => PANEL_GIT, - SidebarPanel::Extensions => PANEL_EXTENSIONS, - SidebarPanel::Ai => PANEL_AI, - }), - Some(ActivityBarTarget::Settings) => Some(PANEL_SETTINGS), + Some(ActivityBarTarget::ExtensionPanel(name)) => Some(format!("ext:{name}")), + Some(ActivityBarTarget::Panel(p)) => Some( + match p { + SidebarPanel::Explorer => PANEL_EXPLORER, + SidebarPanel::Search => PANEL_SEARCH, + SidebarPanel::Debug => PANEL_DEBUG, + SidebarPanel::Git => PANEL_GIT, + SidebarPanel::Extensions => PANEL_EXTENSIONS, + SidebarPanel::Ai => PANEL_AI, + } + .to_string(), + ), + Some(ActivityBarTarget::Settings) => Some(PANEL_SETTINGS.to_string()), _ => None, }; if let Some(panel_id) = target_panel_id { - sidebar.ext_panel_name = None; - engine.ext_panel_has_focus = false; - engine.ext_panel_active = None; - engine.toggle_sidebar_panel(panel_id); - if engine.app_shell.sidebar_visible() { + let switched = render::apply_activity_panel_switch(engine, &panel_id); + sidebar.ext_panel_name = switched.ext_panel; + if switched.sidebar_visible { sidebar.has_focus = true; } } @@ -2096,8 +1961,12 @@ pub(super) fn handle_mouse( // Account for menu bar: when visible it occupies absolute row 0, so the // sidebar's logical row 0 is at absolute terminal row `menu_rows`. let sidebar_row = row.saturating_sub(menu_rows); - // Extension panel must be checked FIRST — ext_panel_name overrides active_panel - if sidebar.ext_panel_name.is_some() { + // #754: who owns the sidebar body is `render::sidebar_owner`, the same + // question GTK's `try_route_sidebar_mouse_event` asks — including the + // "an ext panel outranks `active_panel_id`" precedence this chain used + // to state for itself via `sidebar.ext_panel_name`. + let owner = render::sidebar_owner(engine); + if matches!(owner, render::SidebarOwner::ExtPanel(_)) { sidebar.has_focus = true; engine.ext_panel_has_focus = true; @@ -2160,7 +2029,7 @@ pub(super) fn handle_mouse( } } } - } else if engine.active_panel_is(PANEL_EXPLORER) { + } else if owner == render::SidebarOwner::Explorer { sidebar.has_focus = true; engine.explorer_has_focus = true; @@ -2179,7 +2048,7 @@ pub(super) fn handle_mouse( engine.open_file_preview(&path); } } - } else if engine.active_panel_is(PANEL_DEBUG) { + } else if owner == render::SidebarOwner::Debug { sidebar.has_focus = true; engine.dap_sidebar_has_focus = true; @@ -2218,7 +2087,7 @@ pub(super) fn handle_mouse( engine.dispatch_dap_sidebar_event(sidebar_event); } return sidebar_width; - } else if engine.active_panel_is(PANEL_GIT) { + } else if owner == render::SidebarOwner::Git { sidebar.has_focus = true; engine.sc_set_focus(true); @@ -2275,7 +2144,7 @@ pub(super) fn handle_mouse( } } return sidebar_width; - } else if engine.active_panel_is(PANEL_SEARCH) { + } else if owner == render::SidebarOwner::Search { sidebar.has_focus = true; if !engine.search_has_focus { engine.search_set_focus(true); @@ -2292,7 +2161,7 @@ pub(super) fn handle_mouse( if !engine.search_has_focus { sidebar.has_focus = false; } - } else if engine.active_panel_is(PANEL_EXTENSIONS) { + } else if owner == render::SidebarOwner::Extensions { sidebar.has_focus = true; engine.ext_sidebar_has_focus = true; if sidebar_row == 0 { @@ -2301,7 +2170,7 @@ pub(super) fn handle_mouse( engine.ext_sidebar_input_active = true; } // Rows 2+ handled by SidebarSystem mouse intercept in main loop - } else if engine.active_panel_is(PANEL_SETTINGS) { + } else if owner == render::SidebarOwner::Settings { sidebar.has_focus = true; engine.settings_has_focus = true; let flat_total = engine.settings_flat_list().len(); @@ -2966,7 +2835,7 @@ fn route_and_apply_chrome_click( // first: it paints in its own full-width band *outside* every window's // rect, so a click there must not fall through to what sits under it. if let Some(status) = &layout.separated_status_line { - let qf_rows: u16 = if engine.quickfix_open { 6 } else { 0 }; + let qf_rows: u16 = render::quickfix_panel_rows(engine); let strip_rows: u16 = if engine.terminal_open { super::effective_terminal_panel_rows_tui(engine, geom.term_height) + 1 } else { From 6ae0f4c4aee88f7934aa5440770e0219b585f464 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Wed, 2 Sep 2026 14:26:51 -0500 Subject: [PATCH 2/3] #754: black-box coverage for the panels rung, both backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four tests, all asserting on rendered output (CLAUDE.md rule 1) and all verified RED against the unfixed code (rule 2). TUI (`TuiDriver` via `driver_with_shell`, in `tui_main::shell_app`): - `bottom_panel_tab_strip_click_switches_the_painted_panel_via_shell_app` — clicking the Debug Output tab must repaint the panel body with the debug output. RED with `apply_bottom_panel_route`'s `TabBar` arm stubbed. - `empty_quickfix_does_not_displace_the_terminal_band_via_shell_app` — with `:copen` on an empty list, a right-click three rows above the painted terminal must still open the editor context menu. RED with `quickfix_panel_rows` reverted to `if quickfix_open { 6 }`. GTK (`GtkDriver`, in `gtk::testing`): - `bottom_panel_tab_strip_click_switches_the_painted_panel` — the same claim on this backend, aimed at the `slot_positions` the frame painted. RED with `route_bottom_panel_click`'s `TabBar` zone mis-resolved. - `source_control_toolbar_button_highlights_on_hover` — pixel probe across the button's own painted bounds, before and after the pointer arrives. RED with the new `route_sidebar_hover` call gated off, which is exactly develop's state: GTK painted `button_hovered` and nothing ever set it. Also: replace a single-arm `match` with `matches!` for clippy. Co-Authored-By: Claude Opus 5 --- src/gtk/testing.rs | 120 ++++++++++++++++++++++++++++++++++++++ src/tui_main/mouse.rs | 25 ++++---- src/tui_main/shell_app.rs | 99 +++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 14 deletions(-) diff --git a/src/gtk/testing.rs b/src/gtk/testing.rs index a5a64f1e..a08861e9 100644 --- a/src/gtk/testing.rs +++ b/src/gtk/testing.rs @@ -2694,6 +2694,126 @@ mod sidebar_panel_clicks { ); } + // ── #754 (mouse ladder slice 4: panels) ──────────────────────────────── + + /// GTK half of `bottom_panel_tab_strip_click_switches_the_painted_panel_ + /// via_shell_app`: the shared tab strip must switch which panel is + /// **painted** here too, through `render::route_bottom_panel_click` -> + /// `render::apply_bottom_panel_route`. + /// + /// The click is aimed at the geometry the frame actually painted — the + /// `slot_positions` `draw_tab_bar` returned into + /// `engine.bottom_tab_bar_hits`, and the tab-strip row from + /// `engine.bottom_panel_geometry` — never a guessed offset, so a strip + /// that paints somewhere else fails rather than passing by luck. + #[test] + fn bottom_panel_tab_strip_click_switches_the_painted_panel() { + let mut engine = Engine::new(); + engine.settings.use_nerd_fonts = false; + engine.terminal_new_tab(80, 10); + engine + .dap_output_lines + .push("ZQXW754GTKDEBUGMARKER".to_string()); + + let mut h = harness(engine, 1400, 900); + h.driver.render(); + assert!( + !h.driver.screen_contains("ZQXW754GTKDEBUGMARKER"), + "precondition: the Terminal tab owns the panel body" + ); + + // Locate the *second* painted tab slot (Terminal, then Debug Output). + let (slot_x, strip_y) = { + let engine = h.engine.borrow(); + let hits = engine.bottom_tab_bar_hits.borrow(); + let hits = hits + .as_ref() + .expect("the bottom panel must have painted a tab strip"); + let &(sx, ex) = hits + .slot_positions + .get(1) + .expect("Terminal + Debug Output are two painted slots"); + let geom = engine + .bottom_panel_geometry + .borrow() + .expect("the bottom panel must have painted"); + ((sx + ex) / 2.0, geom.top_y + geom.toolbar_y / 2.0) + }; + h.driver.click(slot_x as f32, strip_y as f32); + h.driver.render(); + + assert!( + h.driver.screen_contains("ZQXW754GTKDEBUGMARKER"), + "clicking the Debug Output tab must repaint the panel body with the \ + debug output (#754 `BottomPanelRoute::TabBar`)" + ); + } + + /// The sidebar hover rung must exist **on this backend at all**. + /// + /// Before #754 the Source Control toolbar's hover highlight was driven by + /// ~78 lines that ran only in `tui_main/mouse.rs`: GTK painted + /// `SourceControlData::button_hovered` faithfully (`draw_sc_sidebar_panel` + /// passes it to `Backend::draw_sidebar_panel` as `hovered_id`) but nothing + /// on this side ever set it, so the highlight could not appear no matter + /// where the pointer went. That paint-without-input asymmetry is the + /// mechanism behind #499/#484. + /// + /// Asserts on **rendered pixels** (`CLAUDE.md` rule 1) — the button's own + /// painted band before and after the pointer arrives. Asserting + /// `sc_button_hovered == Some(_)` would pass against a backend that sets + /// the field and never repaints, which is precisely the failure this rung + /// is fixing in the other direction. + /// + /// Skipped when the checkout isn't a git repo: with no `SourceControl` + /// screen the panel paints no toolbar and there is no button to hover. + #[test] + fn source_control_toolbar_button_highlights_on_hover() { + let mut h = panel_harness(PANEL_GIT); + h.driver.render(); + let sb = match h.painted_sidebar_bounds.get() { + Some(sb) if h.engine.borrow().sc_panel_layout.borrow().is_some() => sb, + _ => return, + }; + + // Locate a toolbar button from the layout the frame painted — its own + // `bounds`, not a scan or a guess. The hover highlight is a rounded + // rect inset 2px inside those bounds, so probe the centre. + let button = { + let engine = h.engine.borrow(); + let layout = engine.sc_panel_layout.borrow(); + layout + .as_ref() + .and_then(|l| l.toolbar_layout.as_ref()) + .and_then(|t| t.visible_items.iter().find(|i| i.clickable).cloned()) + }; + let Some(button) = button else { + return; // no clickable toolbar buttons painted in this repo state + }; + let (bx, by) = ( + button.bounds.x + button.bounds.width / 2.0, + button.bounds.y + button.bounds.height / 2.0, + ); + + let sample = |h: &mut Harness<_>| -> Vec<(u8, u8, u8)> { + let x0 = (button.bounds.x + 3.0) as i32; + let x1 = (button.bounds.x + button.bounds.width - 3.0) as i32; + (x0..x1).map(|x| h.driver.pixel(x, by as i32)).collect() + }; + let before = sample(&mut h); + + h.driver.mouse_move(bx, by); + h.driver.render(); + let after = sample(&mut h); + + assert_ne!( + before, after, + "moving the pointer onto a Source Control toolbar button must repaint \ + it hovered — GTK painted `button_hovered` but nothing set it before \ + #754 made `render::route_sidebar_hover` shared" + ); + } + /// Debug: a press in the panel body must reach the panel at all — before /// #544 it was swallowed by the editor click path, which left /// `dap_sidebar_has_focus` false so every subsequent keystroke went to the diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index 314ecd55..1260f2f2 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -1911,21 +1911,18 @@ pub(super) fn handle_mouse( let ab_target = crate::core::engine::resolve_activity_bar_click(bar_row, bar_height, &ext_names); use crate::core::engine::{ActivityBarTarget, SidebarPanel}; - match ab_target { - Some(ActivityBarTarget::MenuToggle) => { - engine.toggle_menu_bar(); - if !engine.menu_bar_visible { - // Close the dropdown. MenuSystem::close() needs &mut Backend, - // but the mouse handler only has (drag_state, modal_stack). - // Pop the modal directly and reset the MenuSystem state by - // re-creating it with the same menu definitions. - modal_stack.pop(&quadraui::WidgetId::new("menu-system-dropdown")); - let menus = crate::render::build_menu_defs(engine.is_vscode_mode()); - *engine.menu_system.borrow_mut() = quadraui::MenuSystem::new(menus); - } - return sidebar_width; + if matches!(ab_target, Some(ActivityBarTarget::MenuToggle)) { + engine.toggle_menu_bar(); + if !engine.menu_bar_visible { + // Close the dropdown. MenuSystem::close() needs &mut Backend, + // but the mouse handler only has (drag_state, modal_stack). + // Pop the modal directly and reset the MenuSystem state by + // re-creating it with the same menu definitions. + modal_stack.pop(&quadraui::WidgetId::new("menu-system-dropdown")); + let menus = crate::render::build_menu_defs(engine.is_vscode_mode()); + *engine.menu_system.borrow_mut() = quadraui::MenuSystem::new(menus); } - _ => {} + return sidebar_width; } // #754: the ext-panel toggle and the built-in panel switch — which used // to be ~50 lines here and a near-copy in GTK's `App::switch_panel` — diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index e762d326..2abd7409 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -5646,6 +5646,105 @@ mod tests { ); } + // ── #754 (mouse ladder slice 4: panels) ──────────────────────────────── + + /// The bottom panel's shared tab strip must switch which panel is + /// **painted**, end to end through `driver_with_shell` -> `TuiShellApp:: + /// handle` -> `mouse::handle_mouse` -> `render::route_bottom_panel_click` + /// -> `render::apply_bottom_panel_route`. + /// + /// Asserts on rendered output (`CLAUDE.md` rule 1): the Debug Output + /// marker line has to actually reach the screen. Asserting + /// `bottom_panel_kind == DebugOutput` would pass against a router that + /// flips the field while the painter still draws the terminal — the + /// #587/#592 failure shape. + /// + /// Before #754 the `TabBar` zone was resolved by a bespoke arm here and a + /// different one on GTK; it is now the one `BottomPanelRoute::TabBar` both + /// call. + #[test] + fn bottom_panel_tab_strip_click_switches_the_painted_panel_via_shell_app() { + let mut app = TuiShellApp::new(None); + app.engine.terminal_new_tab(80, 10); + app.engine + .dap_output_lines + .push("ZQXW_754_DEBUG_MARKER".to_string()); + + let mut driver = driver_with_shell(app, config(), 80, 24); + // Settle the sidebar-derived layout before measuring — see + // `group_divider_drag_moves_the_painted_divider_via_shell_app`. + driver.mouse_up(1.0, 1.0); + driver.render(); + assert!( + !driver.screen_contains("ZQXW_754_DEBUG_MARKER"), + "precondition: the Terminal tab owns the panel body; screen:\n{}", + driver.screen() + ); + + let (dx, dy) = driver + .find("Debug Output") + .expect("the bottom panel tab strip must paint a Debug Output tab"); + driver.click(dx, dy); + driver.render(); + + assert!( + driver.screen_contains("ZQXW_754_DEBUG_MARKER"), + "clicking the Debug Output tab must repaint the panel body with the \ + debug output (#754 `BottomPanelRoute::TabBar`); screen:\n{}", + driver.screen() + ); + } + + /// An **open but empty** quickfix list must reserve no rows for mouse + /// routing, because it reserves none for painting. + /// + /// `compute_editor_layout` gates the quickfix band on `quickfix_open && + /// !quickfix_items.is_empty()`, but `handle_mouse` asked `if + /// engine.quickfix_open { 6 }` in four places — so `:copen` on an empty + /// list moved every band below the editor six rows up from where it was + /// painted. `render::quickfix_panel_rows` is now the single rule. + /// + /// The discriminator needs no knowledge of the panel's height: with the + /// old rule the terminal's right-click *suppression band* starts six rows + /// above the painted terminal, so a right-click three rows **above** the + /// painted tab strip — plainly in the editor — was silently swallowed and + /// no editor context menu appeared. Asserts on rendered output: the menu's + /// own painted item text. + #[test] + fn empty_quickfix_does_not_displace_the_terminal_band_via_shell_app() { + let mut app = TuiShellApp::new(None); + app.engine.terminal_new_tab(80, 8); + // `:copen` with nothing in the list — open, but paints nothing. + app.engine.quickfix_open = true; + app.engine.quickfix_items.clear(); + + let mut driver = driver_with_shell(app, config(), 80, 24); + driver.mouse_up(1.0, 1.0); + driver.render(); + + let (_, strip_y) = driver + .find("Terminal") + .expect("the bottom panel tab strip must paint a Terminal tab"); + // Three rows above the painted strip: editor text, and inside the + // six-row band the old rule wrongly attributed to the terminal. + let target_y = strip_y - 3.0; + assert!( + target_y > 1.0, + "fixture must leave editor rows above the terminal panel; screen:\n{}", + driver.screen() + ); + driver.right_click(40.0, target_y); + driver.render(); + + assert!( + driver.screen_contains("Go to Definition"), + "a right-click in the editor must open the editor context menu even \ + with an empty quickfix open — the terminal's suppression band must \ + not be displaced by rows nothing painted (#754); screen:\n{}", + driver.screen() + ); + } + // ── #605 (Stage 6 parity sweep) ──────────────────────────────────────── // // The rest of `draw_frame`'s tail, each asserted through From 2ac9a81f2ca1c7d71c07da9c56d6185e8264dae6 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Wed, 2 Sep 2026 15:15:26 -0500 Subject: [PATCH 3/3] fix(#754 review): actually converge the sidebar-panel-area rung, both backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blocking finding: try_route_sidebar_mouse_event's per-panel arms (explorer_ui_event, route_debug_sidebar_event, route_sc_sidebar_event, route_ai_sidebar_event) and mouse.rs's owner match were byte-for-byte duplicated business logic behind a shared "who owns the sidebar" check — not converged, contrary to the issue's hard exit condition and the PR's own commit message. Shared, in render.rs, now called from both backends: - route_explorer_tree_event: the TreeController populate/handle/resolve- context-menu dispatch (GTK's explorer_ui_event, TUI's mouse.rs Explorer arm and TuiShellApp's explorer intercept all call it). - dispatch_dap_sidebar_body_event + dap_sidebar_action_click_at: the Debug panel's body dispatch and chrome (title/action-row) hit test. - route_sc_sidebar_click (+ ScSidebarClickOutcome): the Git panel's header/commit-input/toolbar/content dispatch, using the same absolute coordinate frame sc_panel_layout/sc_button_hit already hit-test in on both backends (TUI's own sidebar_row-relative shortcut is gone). - route_ai_sidebar_click: the AI panel's dispatch logic (GTK-only wiring for now — TUI never cached bands for this panel; that's a real, separate gap, documented on the function). Along the way, found and fixed a second unconverged copy: TUI's real activity-bar click path (TuiShellApp::on_shell_event -> activate_ext_panel, driven by quadraui's own AppShell widget) never called apply_activity_panel_switch at all — it was a fourth hand-rolled copy that still reset ext_panel_selected/re-fired plugin_event on every re-activation even after the "shared function" claim. Converged it too. Second blocking finding: apply_activity_panel_switch's two documented behavior fixes (#637 focus-clear now on GTK, re-entry guard now on TUI) had no black-box coverage. Added: - GTK: switching_to_a_plugin_panel_clears_stale_marketplace_focus (gtk/testing.rs) — drives a real activity-bar click through GtkDriver, verified RED against the unfixed clear_sidebar_focus() call. - TUI: reactivating_the_open_plugin_panel_does_not_reset_its_scroll_position (tui_main/shell_app.rs) — drives on_shell_event twice, verified RED against the unconverged activate_ext_panel. cargo build / clippy (both feature lanes) / fmt clean. Targeted suites: tui_main:: 169 passed, gtk:: 102 passed, render:: 150 passed. Co-Authored-By: Claude Sonnet 5 --- src/gtk/mod.rs | 286 +++++++++++++------------------------- src/gtk/testing.rs | 79 +++++++++++ src/render.rs | 203 +++++++++++++++++++++++++++ src/tui_main/mouse.rs | 169 +++++++++++----------- src/tui_main/shell_app.rs | 174 ++++++++++++++++------- 5 files changed, 590 insertions(+), 321 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index f6ffe190..85d6d173 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -4039,6 +4039,15 @@ impl App { /// `UiEvent` (scroll, mouse) over the explorer panel — routed through /// `TreeController::handle` for scrollbar interaction. + /// Sidebar routing for the Explorer panel (#540/#754). + /// + /// The `TreeController` widget dispatch itself — populate, re-apply the + /// paint-time metrics, `handle()`, resolve a `ContextMenuRequested` — + /// is [`render::route_explorer_tree_event`], shared with TUI's + /// `TuiShellApp::handle_mouse_event` explorer intercept. What stays here + /// is GTK-only plumbing: which events this panel claims at all + /// (`dominated`), pulling the metrics/backend/theme it needs to make the + /// call, and its own draw-invalidation bookkeeping. fn explorer_ui_event(&mut self, ev: quadraui::UiEvent) { let dominated = matches!( ev, @@ -4053,83 +4062,53 @@ impl App { .. } ); - if dominated { - let rect = self.engine.borrow().explorer_tree_rect.get(); - if rect.width > 0.0 { - let theme = { - let eng = self.engine.borrow(); - render::Theme::from_name(&eng.settings.colorscheme) - }; - crate::render::populate_explorer_tree_controller(&self.engine.borrow(), &theme); - let tree_event = { - let mut b = self.backend.borrow_mut(); - // Re-apply the metrics the tree was drawn with so the - // hit-test row math matches the rendered rows. (#540) - let (lh, cw) = self.cached_explorer_metrics.get(); - b.set_current_line_height(lh); - b.set_current_char_width(cw); - self.engine - .borrow() - .explorer_tree - .borrow_mut() - .handle(&ev, &mut *b, rect) - }; - // #546: a right-click MouseDown lands here too (this - // arm doesn't filter by button — `try_route_sidebar_ - // mouse_event` forwards ALL MouseDown in the sidebar - // bounds), and `TreeController::handle()` already - // resolves it to `ContextMenuRequested{path, position}` - // with the correct target row. But - // `dispatch_explorer_tree_event`'s catch-all silently - // dropped that variant, so explorer right-click did - // nothing at all — independent of the render gap - // this issue otherwise fixes. Editor/tab-bar - // right-click are unaffected (dedicated - // `MouseButton::Right` branches before generic - // routing); TUI is unaffected too (it intercepts - // right-clicks at the raw crossterm layer, before - // UiEvent translation, so it never reaches - // `ContextMenuRequested`). `position` is in the same - // absolute pixel space `TreeController` just used for - // its own hit-test, so convert with the same - // `(lh, cw)` metrics used above. - if let quadraui::TreeControllerEvent::ContextMenuRequested { path, position } = - &tree_event - { - if let Some(&row_idx) = path.first() { - let idx = row_idx as usize; - let target_info = { - let eng = self.engine.borrow(); - eng.explorer_rows - .get(idx) - .map(|row| (row.path.clone(), row.is_dir)) - }; - if let Some((target, is_dir)) = target_info { - let (lh, cw) = self.cached_explorer_metrics.get(); - let cx = (position.x / (cw.max(1.0) as f32)) as u16; - let cy = (position.y / (lh.max(1.0) as f32)) as u16; - self.engine - .borrow_mut() - .open_explorer_context_menu(target, is_dir, cx, cy); - } - } - self.queue_explorer_draw(); - self.draw_needed.set(true); - return; - } - if matches!(ev, quadraui::UiEvent::DoubleClick { .. }) { - self.engine - .borrow_mut() - .dispatch_explorer_tree_event(tree_event); - } else if matches!(ev, quadraui::UiEvent::MouseDown { .. }) { - self.engine - .borrow_mut() - .handle_explorer_mouse_event(tree_event); - } - self.queue_explorer_draw(); - self.draw_needed.set(true); - } + if !dominated { + return; } + let rect = self.engine.borrow().explorer_tree_rect.get(); + if rect.width <= 0.0 { + return; + } + let theme = { + let eng = self.engine.borrow(); + render::Theme::from_name(&eng.settings.colorscheme) + }; + // Re-apply the metrics the tree was drawn with so the hit-test row + // math matches the rendered rows (#540). `set_current_line_height`/ + // `set_current_char_width` are inherent on `GtkBackend`, not trait + // methods on `dyn Backend`, so they must be set from here rather + // than inside the shared function. + let metrics = self.cached_explorer_metrics.get(); + let backend_rc = self.backend.clone(); + let mut b = backend_rc.borrow_mut(); + b.set_current_line_height(metrics.0); + b.set_current_char_width(metrics.1); + let tree_event = { + let mut engine = self.engine.borrow_mut(); + render::route_explorer_tree_event(&mut engine, &ev, rect, metrics, &theme, &mut *b) + }; + drop(b); + + // `None` means either the event was fully resolved inside + // `route_explorer_tree_event` (a `ContextMenuRequested` — #546) or + // the rect wasn't paintable; either way this panel already did + // everything it needs to. + let Some(tree_event) = tree_event else { + self.queue_explorer_draw(); + self.draw_needed.set(true); + return; + }; + if matches!(ev, quadraui::UiEvent::DoubleClick { .. }) { + self.engine + .borrow_mut() + .dispatch_explorer_tree_event(tree_event); + } else if matches!(ev, quadraui::UiEvent::MouseDown { .. }) { + self.engine + .borrow_mut() + .handle_explorer_mouse_event(tree_event); + } + self.queue_explorer_draw(); + self.draw_needed.set(true); } /// Find the runner-created top-level window once it is mapped/visible. @@ -4302,8 +4281,12 @@ impl App { engine.handle_search_sidebar_ui_event(event.clone()); true } - render::SidebarOwner::Debug => self.route_debug_sidebar_event(event), - render::SidebarOwner::Git => self.route_sc_sidebar_event(event), + render::SidebarOwner::Debug => { + self.route_debug_sidebar_event(event, pos, starts_interaction) + } + render::SidebarOwner::Git => { + self.route_sc_sidebar_event(event, pos, starts_interaction) + } render::SidebarOwner::Extensions => { let mut engine = self.engine.borrow_mut(); if is_press { @@ -4343,7 +4326,7 @@ impl App { engine.handle_ext_sidebar_ui_event(event.clone()); true } - render::SidebarOwner::Ai => self.route_ai_sidebar_event(event), + render::SidebarOwner::Ai => self.route_ai_sidebar_event(pos, starts_interaction), // Unknown panel id: nothing was painted, so there is nothing // for a click to hit — let it fall through rather than // swallow it. @@ -4360,68 +4343,54 @@ impl App { consumed } - /// Sidebar routing for the Debug panel (#544). + /// Sidebar routing for the Debug panel (#544/#754). /// /// `render_content` stacks two chrome rows above the body: a title bar and /// an action-button bar whose `StatusBarLayout` it stashes in /// `engine.dap_sidebar_action_hits`. Those hit regions are **bar-relative** /// (`StatusBar::layout` lays out from `0,0`; `quadraui::gtk::draw_status_bar` - /// returns them verbatim), so the press has to be translated into the action - /// row's own space before hit-testing. Everything below goes to the shared - /// `SidebarSystem` at the body rect it painted into. - fn route_debug_sidebar_event(&mut self, event: &quadraui::UiEvent) -> bool { + /// returns them verbatim), so the press has to be translated into the + /// action row's own space before hit-testing + /// ([`render::dap_sidebar_action_click_at`]). Everything below goes to + /// the shared `SidebarSystem` at the body rect it painted into + /// ([`render::dispatch_dap_sidebar_body_event`]) — the same two shared + /// functions TUI calls for this panel. + fn route_debug_sidebar_event( + &mut self, + event: &quadraui::UiEvent, + pos: quadraui::Point, + starts_interaction: bool, + ) -> bool { let action_rect = self.cached_dap_action_rect.get(); let body_rect = self.engine.borrow().dap_sidebar_body_rect.get(); if body_rect.width <= 0.0 { return false; } - let starts_interaction = matches!( - event, - quadraui::UiEvent::MouseDown { .. } | quadraui::UiEvent::DoubleClick { .. } - ); - let pos = match event { - quadraui::UiEvent::MouseDown { position, .. } - | quadraui::UiEvent::DoubleClick { position, .. } - | quadraui::UiEvent::MouseUp { position, .. } - | quadraui::UiEvent::MouseMoved { position, .. } - | quadraui::UiEvent::Scroll { position, .. } => *position, - _ => return false, - }; let mut engine = self.engine.borrow_mut(); if starts_interaction { engine.dap_sidebar_has_focus = true; } // Chrome band (title + action row) — above the body rect. if starts_interaction && pos.y < body_rect.y { - let matched = action_rect.is_some_and(|ar| { - let hits = engine.dap_sidebar_action_hits.borrow(); - hits.as_ref().is_some_and(|l| { - matches!( - l.hit_test(pos.x - ar.x, pos.y - ar.y), - quadraui::StatusBarHit::Segment(_) - ) - }) - }); - if matched { - engine.handle_dap_sidebar_action_click(); + if let Some(ar) = action_rect { + render::dap_sidebar_action_click_at(&mut engine, pos.x - ar.x, pos.y - ar.y); } // Claimed either way: the press landed on this panel's own chrome, // so it must not leak through to the editor beneath (#637's rule // for the TUI twin of this intercept). return true; } - render::populate_dap_sidebar_system(&engine); let backend_rc = self.backend.clone(); - let sidebar_event = engine.dap_sidebar_system.borrow_mut().handle( + render::dispatch_dap_sidebar_body_event( + &mut engine, event, - &mut *backend_rc.borrow_mut(), body_rect, + &mut *backend_rc.borrow_mut(), ); - engine.dispatch_dap_sidebar_event(sidebar_event); true } - /// Sidebar routing for the git ("source control") panel (#544). + /// Sidebar routing for the git ("source control") panel (#544/#754). /// /// The panel is three stacked bands — header, commit-message input, and the /// toolbar slab + change sections. `render_content` derives them via @@ -4429,98 +4398,41 @@ impl App { /// press against the exact geometry that was painted rather than /// re-deriving it (the pre-#544 handler assumed `DrawingArea`-local /// coordinates with the panel top at `y == 0`, which the ShellApp painter - /// never produces). - fn route_sc_sidebar_event(&mut self, event: &quadraui::UiEvent) -> bool { + /// never produces). The dispatch itself is + /// [`render::route_sc_sidebar_click`], shared with TUI. + fn route_sc_sidebar_event( + &mut self, + event: &quadraui::UiEvent, + pos: quadraui::Point, + starts_interaction: bool, + ) -> bool { let Some(bands) = self.cached_sc_bands.get() else { return false; }; - let starts_interaction = matches!( - event, - quadraui::UiEvent::MouseDown { .. } | quadraui::UiEvent::DoubleClick { .. } - ); - let pos = match event { - quadraui::UiEvent::MouseDown { position, .. } - | quadraui::UiEvent::DoubleClick { position, .. } - | quadraui::UiEvent::MouseUp { position, .. } - | quadraui::UiEvent::MouseMoved { position, .. } - | quadraui::UiEvent::Scroll { position, .. } => *position, - _ => return false, - }; let mut engine = self.engine.borrow_mut(); - if starts_interaction { - engine.sc_set_focus(true); - } - if starts_interaction { - let commit_bottom = bands.commit_input.y + bands.commit_input.height; - if pos.y < bands.header.y + bands.header.height { - engine.sc_commit_input_active = false; - return true; - } - if pos.y < commit_bottom { - engine.sc_commit_input_active = true; - engine.sc_commit_cursor = engine.sc_commit_message.len(); - return true; - } - engine.sc_commit_input_active = false; - // Toolbar buttons live in the slab above the section list; the - // cached `SidebarPanelLayout` is in absolute space because - // `render_content` painted it at an absolute `slab_rect`. - let hit = { - let layout = engine.sc_panel_layout.borrow(); - layout.as_ref().map(|l| l.hit_test(pos.x, pos.y)) - }; - if let Some(quadraui::SidebarPanelHit::ToolbarButton(_)) = hit { - if let Some(idx) = engine.sc_button_hit(pos.x, pos.y) { - engine.sc_activate_button(idx); - } - return true; - } - } - engine.handle_sc_sidebar_ui_event(event.clone()); + render::route_sc_sidebar_click(&mut engine, event, pos, &bands, starts_interaction); true } - /// Sidebar routing for the AI panel (#544/#730). + /// Sidebar routing for the AI panel (#544/#730/#754). /// /// `render_content` caches the header/messages/input bands in /// `cached_ai_bands` at paint time (`render::draw_ai_sidebar_panel`'s /// return value) — resolving a press against that means the click /// router can never derive a different layout than the one actually on - /// screen (#544/#582/#646). Neither TUI nor the pre-#730 GTK arm had any - /// body-click routing here (AI-panel focus/edit was keyboard-only on - /// both backends); this adds the same "click focuses the panel, - /// click-in-input activates editing" policy the git sidebar's commit - /// box already uses (`route_sc_sidebar_event` above), consuming the - /// press unconditionally like every other panel arm in + /// screen (#544/#582/#646). The dispatch itself is + /// [`render::route_ai_sidebar_click`] — TUI paints this same panel but + /// has never cached its bands for click routing, so it does not call + /// this yet; see that function's doc comment. Consumes the press + /// unconditionally like every other panel arm in /// `try_route_sidebar_mouse_event` — a click on empty panel padding /// still belongs to this panel, not the editor underneath it. - fn route_ai_sidebar_event(&mut self, event: &quadraui::UiEvent) -> bool { + fn route_ai_sidebar_event(&mut self, pos: quadraui::Point, starts_interaction: bool) -> bool { let Some(bands) = self.cached_ai_bands.get() else { return false; }; - let starts_interaction = matches!( - event, - quadraui::UiEvent::MouseDown { .. } | quadraui::UiEvent::DoubleClick { .. } - ); - let pos = match event { - quadraui::UiEvent::MouseDown { position, .. } - | quadraui::UiEvent::DoubleClick { position, .. } - | quadraui::UiEvent::MouseUp { position, .. } - | quadraui::UiEvent::MouseMoved { position, .. } - | quadraui::UiEvent::Scroll { position, .. } => *position, - _ => return false, - }; - if starts_interaction { - let mut engine = self.engine.borrow_mut(); - engine.ai_has_focus = true; - let in_input = pos.y >= bands.input.y && pos.y < bands.input.y + bands.input.height; - if in_input { - engine.ai_input_active = true; - engine.ai_input_cursor = engine.ai_input.chars().count(); - } else { - engine.ai_input_active = false; - } - } + let mut engine = self.engine.borrow_mut(); + render::route_ai_sidebar_click(&mut engine, pos, &bands, starts_interaction); true } diff --git a/src/gtk/testing.rs b/src/gtk/testing.rs index a08861e9..3969055e 100644 --- a/src/gtk/testing.rs +++ b/src/gtk/testing.rs @@ -2814,6 +2814,85 @@ mod sidebar_panel_clicks { ); } + /// #637/#754: switching to a plugin ("extension") panel from the + /// activity bar must clear focus flags a previously-visited panel left + /// set, on **this** backend too. + /// + /// `render::apply_activity_panel_switch`'s own doc comment: "#637's + /// focus clear was TUI-only" — TUI's `App::switch_panel` twin + /// (`mouse.rs`'s `ActivityBarTarget` match) called + /// `engine.clear_sidebar_focus()` before showing a plugin panel; GTK's + /// pre-#754 `switch_panel` never did, so a stale `ext_sidebar_has_focus` + /// left by an earlier visit to the Extensions *marketplace* panel + /// stayed stuck `true` after switching to an unrelated plugin panel. + /// TUI's regression test for the mirror-image bug is + /// `plugin_ext_panel_wins_focus_and_clicks_after_marketplace_visit` + /// (`tui_main/shell_app.rs`) — this is the GTK half. + /// + /// Drives a **real activity-bar click** through `GtkDriver` — + /// `AppShellEvent::PanelChanged` -> `App::switch_panel` -> + /// `render::apply_activity_panel_switch` — aimed at the exact row + /// `quadraui::gtk::ACTIVITY_ROW_PX` (the fixed per-icon height both the + /// painter and the runner's own hit-test use) puts the newly-registered + /// ext panel at. GTK's activity bar has no menu-toggle row (unlike + /// TUI's optional menu bar, GTK's is always the CSD title bar), so the + /// six fixed top-pinned items — explorer, search, debug, git, + /// extensions, ai (`sidebar::FIXED_ACTIVITY_PANEL_IDS`, the same shared + /// order both backends' shell config builds from) — occupy indices 0-5 + /// and the ext panel lands at index 6; found empirically with a probe + /// harness clicking each row and checking `ext_panel_active`, since + /// this backend has no cached hit-region equivalent to + /// `bottom_tab_bar_hits` to read the geometry from directly. + #[test] + fn switching_to_a_plugin_panel_clears_stale_marketplace_focus() { + let mut engine = Engine::new(); + engine.settings.use_nerd_fonts = false; + engine.ext_panels.clear(); + engine.ext_panels.insert( + "git-insights".to_string(), + crate::core::plugin::PanelRegistration { + name: "git-insights".to_string(), + title: "Git Insights".to_string(), + icon: '\u{f113}', + fallback_icon: Some('X'), + sections: Vec::new(), + }, + ); + // Visit the Extensions marketplace panel first, as a user would + // before ever opening a plugin panel this session. + engine + .app_shell + .show_panel(&quadraui::WidgetId::new(PANEL_EXTENSIONS)); + engine.ext_sidebar_has_focus = true; + + let mut h = harness(engine, 1400, 900); + h.driver.render(); + let ab_top = (h.menu_row_rect.get().y + h.menu_row_rect.get().height) as f32; + + // Click the plugin panel's activity-bar icon (index 6 — see doc + // comment above). + let y = ab_top + (6.0 + 0.5) * quadraui::gtk::ACTIVITY_ROW_PX as f32; + h.driver.click(20.0, y); + + assert_eq!( + h.engine.borrow().ext_panel_active.as_deref(), + Some("git-insights"), + "precondition: the click must have actually switched to the \ + plugin panel — if this fails, the click missed the icon" + ); + assert!( + !h.engine.borrow().ext_sidebar_has_focus, + "switching to a plugin panel must clear the Extensions \ + marketplace's stale ext_sidebar_has_focus (#637's GTK gap — \ + apply_activity_panel_switch now calls clear_sidebar_focus() \ + on both backends)" + ); + assert!( + h.engine.borrow().ext_panel_has_focus, + "the plugin panel itself must take focus" + ); + } + /// Debug: a press in the panel body must reach the panel at all — before /// #544 it was swallowed by the editor click path, which left /// `dap_sidebar_has_focus` false so every subsequent keystroke went to the diff --git a/src/render.rs b/src/render.rs index 272b0262..836e7042 100644 --- a/src/render.rs +++ b/src/render.rs @@ -4031,6 +4031,209 @@ pub fn route_sidebar_hover( inside } +// ─── Sidebar panel body dispatch (#754) ─────────────────────────────────────── +// +// The rung the issue names by line count: "who owns the sidebar body" +// (`sidebar_owner`, above) was already shared, but what each owner's press +// actually *did* — translate the event, feed the same widget/engine call the +// painter's own geometry lines up with — was re-derived independently on +// each backend. The five functions below are that dispatch, stated once. +// Callers keep their own gating (sidebar bounds, picker/context-menu +// precedence, drag-capture bookkeeping) — that's real per-backend plumbing, +// not duplicated business logic — and their own geometry *derivation* +// (GTK caches it from paint time; TUI recomputes it from cheap closed-form +// row math), because a pixel bounds and a cell bounds are answers to two +// different questions. What's shared is what happens once that geometry and +// the event are in hand. + +/// Route an explorer-sidebar event through the shared `TreeController` +/// widget and resolve the result against the engine. +/// +/// `metrics` is `(line_height, char_width)` in the caller's native unit — +/// GTK re-applies the pixel metrics the tree was painted with before +/// hit-testing (its `Backend::set_current_line_height`/`set_current_char_width` +/// are inherent, not trait methods, so the caller must set them before this +/// call); TUI's cell grid needs no such re-application and passes `(1.0, +/// 1.0)`. Both values are also used to convert a `ContextMenuRequested`'s +/// pixel/cell `position` back into the `(col, row)` `open_explorer_context_menu` +/// wants. +/// +/// A `ContextMenuRequested` result is fully resolved here (opening the same +/// `engine.open_explorer_context_menu` both backends used to call +/// independently) and reported back as `None`, since it is already consumed. +/// Any other event is returned to the caller for its own DoubleClick / +/// MouseDown dispatch and focus bookkeeping, which differ enough between a +/// GTK press and a TUI scrollbar-drag lifecycle that folding them in here +/// would just move the duplication rather than remove it. +pub fn route_explorer_tree_event( + engine: &mut Engine, + event: &quadraui::UiEvent, + rect: quadraui::Rect, + metrics: (f64, f64), + theme: &Theme, + backend: &mut dyn quadraui::Backend, +) -> Option { + if rect.width <= 0.0 { + return None; + } + populate_explorer_tree_controller(engine, theme); + let (lh, cw) = metrics; + let tree_event = engine + .explorer_tree + .borrow_mut() + .handle(event, backend, rect); + + if let quadraui::TreeControllerEvent::ContextMenuRequested { path, position } = &tree_event { + if let Some(&row_idx) = path.first() { + let idx = row_idx as usize; + let target_info = engine + .explorer_rows + .get(idx) + .map(|row| (row.path.clone(), row.is_dir)); + if let Some((target, is_dir)) = target_info { + let cx = (position.x / (cw.max(1.0) as f32)) as u16; + let cy = (position.y / (lh.max(1.0) as f32)) as u16; + engine.open_explorer_context_menu(target, is_dir, cx, cy); + } + } + return None; + } + Some(tree_event) +} + +/// Dispatch a mouse/scroll event to the debug ("run and debug") sidebar's +/// *body* — everything below its title + action-button chrome — through the +/// shared `SidebarSystem` widget. The chrome band itself is +/// [`dap_sidebar_action_click_at`], a separate function because it needs +/// only a local point, not a whole event. +pub fn dispatch_dap_sidebar_body_event( + engine: &mut Engine, + event: &quadraui::UiEvent, + rect: quadraui::Rect, + backend: &mut dyn quadraui::Backend, +) { + populate_dap_sidebar_system(engine); + let sidebar_event = engine + .dap_sidebar_system + .borrow_mut() + .handle(event, backend, rect); + engine.dispatch_dap_sidebar_event(sidebar_event); +} + +/// Resolve a press against the debug sidebar's title + action-button chrome +/// row, at a point already translated into that row's own local space (`(0, +/// 0)` at the row's own top-left — `StatusBar::layout`'s own convention). +/// GTK translates by subtracting its cached `action_rect`'s origin; TUI's +/// action row already paints at `y == 0` of its own local frame, so its +/// local point is the raw column. Returns whether a segment was actually +/// hit — the caller claims the whole chrome row regardless, matching both +/// backends' pre-#754 behaviour. +pub fn dap_sidebar_action_click_at(engine: &mut Engine, local_x: f32, local_y: f32) -> bool { + let matched = { + let hits = engine.dap_sidebar_action_hits.borrow(); + hits.as_ref().is_some_and(|l| { + matches!( + l.hit_test(local_x, local_y), + quadraui::StatusBarHit::Segment(_) + ) + }) + }; + if matched { + engine.handle_dap_sidebar_action_click(); + } + matched +} + +/// Which band of the git sidebar a [`route_sc_sidebar_click`] press landed +/// on. Both variants mean "consumed" — the split only exists because TUI's +/// double-click synthesis (crossterm has no native double-click) must fire +/// only for a genuine content-row click, matching what GTK's toolkit-native +/// `DoubleClick` event would have hit had it landed in the same place. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScSidebarClickOutcome { + /// Header, commit-input box, or a toolbar button — chrome consumed the + /// press directly. + Chrome, + /// Fell through to `handle_sc_sidebar_ui_event` — a section/content-row + /// click (or a non-`starts_interaction` follow-through, e.g. a drag). + Content, +} + +/// Route a press/scroll to the git ("source control") sidebar, given the +/// [`ScSidebarBands`] the shared painter laid the panel out into and the +/// event's position in that *same* absolute space. Mirrors the GTK-only +/// `route_sc_sidebar_event` this replaced: header row clears the commit +/// input, the commit-input band activates it, a toolbar-button hit routes +/// through `sc_button_hit`/`sc_activate_button`, and anything else in the +/// slab (or a non-`starts_interaction` follow-through, e.g. a drag) falls +/// through to `handle_sc_sidebar_ui_event`. +pub fn route_sc_sidebar_click( + engine: &mut Engine, + event: &quadraui::UiEvent, + pos: quadraui::Point, + bands: &ScSidebarBands, + starts_interaction: bool, +) -> ScSidebarClickOutcome { + if starts_interaction { + engine.sc_set_focus(true); + let commit_bottom = bands.commit_input.y + bands.commit_input.height; + if pos.y < bands.header.y + bands.header.height { + engine.sc_commit_input_active = false; + return ScSidebarClickOutcome::Chrome; + } + if pos.y < commit_bottom { + engine.sc_commit_input_active = true; + engine.sc_commit_cursor = engine.sc_commit_message.len(); + return ScSidebarClickOutcome::Chrome; + } + engine.sc_commit_input_active = false; + let hit = { + let layout = engine.sc_panel_layout.borrow(); + layout.as_ref().map(|l| l.hit_test(pos.x, pos.y)) + }; + if let Some(quadraui::SidebarPanelHit::ToolbarButton(_)) = hit { + if let Some(idx) = engine.sc_button_hit(pos.x, pos.y) { + engine.sc_activate_button(idx); + } + return ScSidebarClickOutcome::Chrome; + } + } + engine.handle_sc_sidebar_ui_event(event.clone()); + ScSidebarClickOutcome::Content +} + +/// Route a press to the AI assistant sidebar's body, given the +/// [`AiSidebarBands`] the shared painter (`draw_ai_sidebar_panel`) laid the +/// panel out into. A press in the input band activates editing there +/// (cursor at end); anywhere else in the panel just moves focus — mirrors +/// `#730`'s GTK-only `route_ai_sidebar_event`, which this replaces. +/// +/// TUI paints this same panel (`panels::render_ai_sidebar` also calls +/// `draw_ai_sidebar_panel`) but has never cached the returned bands for +/// click routing the way GTK's `cached_ai_bands` does, so it does not yet +/// call this function — the AI panel stayed keyboard-only there before +/// #754 and stays that way after it. That is a real, separate gap (wiring a +/// bands cache into `TuiShellApp`), not the duplication this function +/// fixes: there was never a second copy of this dispatch logic on TUI to +/// converge with. +pub fn route_ai_sidebar_click( + engine: &mut Engine, + pos: quadraui::Point, + bands: &AiSidebarBands, + starts_interaction: bool, +) { + if starts_interaction { + engine.ai_has_focus = true; + let in_input = pos.y >= bands.input.y && pos.y < bands.input.y + bands.input.height; + if in_input { + engine.ai_input_active = true; + engine.ai_input_cursor = engine.ai_input.chars().count(); + } else { + engine.ai_input_active = false; + } + } +} + // ─── Overlay-band composition (#735 slice 1) ────────────────────────────────── // // The paint twin of `route_modal_overlay_click` / `route_modal_key` above, and diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index 1260f2f2..6d4c1d4c 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -2030,45 +2030,58 @@ pub(super) fn handle_mouse( sidebar.has_focus = true; engine.explorer_has_focus = true; - let tree_row = sidebar_row as usize + engine.explorer_tree.borrow().scroll_offset(); - if tree_row < engine.explorer_rows.len() { - // Record potential drag source for DnD. - *explorer_drag_src = Some(tree_row); - engine - .explorer_tree - .borrow_mut() - .set_selected_path(Some(vec![tree_row as u16])); - if engine.explorer_rows[tree_row].is_dir { - engine.explorer_toggle_dir(tree_row); - } else { - let path = engine.explorer_rows[tree_row].path.clone(); - engine.open_file_preview(&path); + // #754: the `TreeController` dispatch itself — + // populate/handle/resolve — is `render::route_explorer_tree_event`, + // the same function GTK's `explorer_ui_event` and this same + // struct's `TuiShellApp::handle_mouse_event` explorer intercept + // call. Cell-native metrics (`(1.0, 1.0)`): TUI's tree paints at + // one row/column per cell, so there is no pixel scale to + // re-apply the way GTK does. + let rect = engine.explorer_tree_rect.get(); + let click_ev = quadraui::UiEvent::MouseDown { + widget: None, + button: quadraui::MouseButton::Left, + position: quadraui::Point::new(col as f32, row as f32), + modifiers: quadraui::Modifiers::default(), + }; + let theme = render::Theme::from_name(&engine.settings.colorscheme); + let mut tui_backend = super::backend::TuiBackend::default(); + if let Some(tree_event) = render::route_explorer_tree_event( + engine, + &click_ev, + rect, + (1.0, 1.0), + &theme, + &mut tui_backend, + ) { + // Record potential drag source for DnD — only a genuine row + // selection (not a chevron toggle or a scrollbar drag) arms + // one, matching what a hit *on the row* used to mean before + // the TreeController's own hit-test replaced the raw + // `sidebar_row` arithmetic here. + if let quadraui::TreeControllerEvent::RowSelected { ref path } = tree_event { + if let Some(&row_idx) = path.first() { + *explorer_drag_src = Some(row_idx as usize); + } } + engine.handle_explorer_mouse_event(tree_event); } } else if owner == render::SidebarOwner::Debug { sidebar.has_focus = true; engine.dap_sidebar_has_focus = true; if sidebar_row < 2 { - // Chrome rows (title + action button). - let guard = engine.dap_sidebar_action_hits.borrow(); - let matched = guard - .as_ref() - .map(|l| { - matches!( - l.hit_test(col as f32, 0.0), - quadraui::StatusBarHit::Segment(_) - ) - }) - .unwrap_or(false); - drop(guard); - if matched { - engine.handle_dap_sidebar_action_click(); - } + // Chrome rows (title + action button). TUI's action-hit + // layout is already populated in absolute-column space (`y` + // is always `0.0` — a single-row band), unlike GTK's, which + // is bar-local and needs its cached rect subtracted first; + // `dap_sidebar_action_click_at` takes the already-local + // point either way (#754). + render::dap_sidebar_action_click_at(engine, col as f32, 0.0); } else { - // Route body click through SidebarSystem. + // Route body click through the shared `SidebarSystem` + // dispatch (#754) — same function GTK calls. let rect = engine.dap_sidebar_body_rect.get(); - crate::render::populate_dap_sidebar_system(engine); let click_event = quadraui::UiEvent::MouseDown { widget: None, button: quadraui::MouseButton::Left, @@ -2076,68 +2089,56 @@ pub(super) fn handle_mouse( modifiers: quadraui::Modifiers::default(), }; let mut tui_backend = super::backend::TuiBackend::default(); - let sidebar_event = engine.dap_sidebar_system.borrow_mut().handle( + render::dispatch_dap_sidebar_body_event( + engine, &click_event, - &mut tui_backend, rect, + &mut tui_backend, ); - engine.dispatch_dap_sidebar_event(sidebar_event); } return sidebar_width; } else if owner == render::SidebarOwner::Git { sidebar.has_focus = true; - engine.sc_set_focus(true); - - // sidebar_row layout after #509 (option a, no padding): - // 0 = header - // 1 .. commit_end = commit input (quadraui::TextInput box, - // including its 1-row border top+bottom — #480) - // commit_end = toolbar slot (button row, SidebarPanel) - // commit_end+1 .. = sections (SidebarPanel content area) - let commit_box_h = render::sc_commit_input_box_height(&engine.sc_commit_message); - let commit_end = 1 + commit_box_h; - if sidebar_row == 0 { - engine.sc_commit_input_active = false; - } else if sidebar_row >= 1 && sidebar_row < commit_end { - engine.sc_commit_input_active = true; - engine.sc_commit_cursor = engine.sc_commit_message.len(); - } else { - // Route via cached SidebarPanelLayout (#509). - engine.sc_commit_input_active = false; - let hit = { - let layout = engine.sc_panel_layout.borrow(); - layout.as_ref().map(|l| l.hit_test(col as f32, row as f32)) - }; - match hit { - Some(quadraui::SidebarPanelHit::ToolbarButton(_)) => { - if let Some(idx) = engine.sc_button_hit(col as f32, row as f32) { - engine.sc_activate_button(idx); - } - } - Some(quadraui::SidebarPanelHit::Content { .. }) => { - let click_ev = quadraui::UiEvent::MouseDown { - widget: None, - button: quadraui::MouseButton::Left, - position: quadraui::Point::new(col as f32, row as f32), - modifiers: quadraui::Modifiers::default(), - }; - engine.handle_sc_sidebar_ui_event(click_ev); - let now = Instant::now(); - let is_double = now.duration_since(*last_click_time) - < Duration::from_millis(400) - && *last_click_pos == (col, row); - *last_click_time = now; - *last_click_pos = (col, row); - if is_double { - let double_ev = quadraui::UiEvent::DoubleClick { - widget: None, - position: quadraui::Point::new(col as f32, row as f32), - }; - engine.handle_sc_sidebar_ui_event(double_ev); - } - } - _ => {} + // #754: bands built in the *same absolute* frame `sc_panel_layout`/ + // `sc_button_hit` already hit-test in (the shared painter's own + // coordinates — `col`/`row`, not the `sidebar_row` this arm used + // pre-#754), so `render::route_sc_sidebar_click` — the same + // dispatch GTK calls — resolves identically on both backends. + let content_rect = quadraui::Rect::new( + ab_width as f32, + menu_rows as f32, + sidebar_width as f32, + term_height.saturating_sub(menu_rows) as f32, + ); + let bands = render::sc_sidebar_bands(&engine.sc_commit_message, content_rect, 1.0, 2.0); + let pos = quadraui::Point::new(col as f32, row as f32); + let click_ev = quadraui::UiEvent::MouseDown { + widget: None, + button: quadraui::MouseButton::Left, + position: pos, + modifiers: quadraui::Modifiers::default(), + }; + let outcome = render::route_sc_sidebar_click(engine, &click_ev, pos, &bands, true); + // Double-click detection stays TUI-only plumbing — crossterm has + // no double-click concept, so it must be synthesized here from + // click timing before being fed back through the same shared + // dispatch, matching how GTK's toolkit-native `DoubleClick` + // reaches `route_sc_sidebar_click` directly. Only a genuine + // content-row click gets one — a double-click on the header, + // commit box, or a toolbar button was never synthesized here. + if outcome == render::ScSidebarClickOutcome::Content { + let now = Instant::now(); + let is_double = now.duration_since(*last_click_time) < Duration::from_millis(400) + && *last_click_pos == (col, row); + *last_click_time = now; + *last_click_pos = (col, row); + if is_double { + let double_ev = quadraui::UiEvent::DoubleClick { + widget: None, + position: pos, + }; + engine.handle_sc_sidebar_ui_event(double_ev); } } return sidebar_width; diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index 2abd7409..d6e9718c 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -884,16 +884,10 @@ impl TuiShellApp { self.sidebar.has_focus = true; self.engine.dap_sidebar_has_focus = true; } - render::populate_dap_sidebar_system(&self.engine); - let sidebar_event = self - .engine - .dap_sidebar_system - .borrow_mut() - .handle(&event, backend, rect); - // #637: the event landed inside this panel's own body rect — - // it must be claimed here unconditionally, even when the - // inner `SidebarEvent` comes back `Ignored` (e.g. a click on - // empty space below the last row, or between two headers + // #637 / #754: the event landed inside this panel's own body + // rect — it must be claimed here unconditionally, even when + // the inner `SidebarEvent` comes back `Ignored` (e.g. a click + // on empty space below the last row, or between two headers // when the list is empty). Only returning `Redraw` on a // "successful" dispatch let an `Ignored` result fall through // to `mouse::handle_mouse`'s unrelated legacy dispatcher, @@ -903,7 +897,9 @@ impl TuiShellApp { // `debug_sidebar_intercept_claims_focus_on_mouse_down` / // `ext_sidebar_intercept_claims_focus_on_mouse_down`, which // hit exactly this path whenever the panel's list is empty). - self.engine.dispatch_dap_sidebar_event(sidebar_event); + // The dispatch itself is `render::dispatch_dap_sidebar_body_event` + // — the same function GTK's `route_debug_sidebar_event` calls. + render::dispatch_dap_sidebar_body_event(&mut self.engine, &event, rect, backend); return Reaction::Redraw; } } @@ -1034,28 +1030,40 @@ impl TuiShellApp { if is_explorer_event { let rect = self.engine.explorer_tree_rect.get(); let theme = self.theme(); - render::populate_explorer_tree_controller(&self.engine, &theme); - let tree_event = self - .engine - .explorer_tree - .borrow_mut() - .handle(&event, backend, rect); - let is_scrollbar = - matches!(tree_event, quadraui::TreeControllerEvent::ScrollChanged); + // #754: the `TreeController` dispatch itself is + // `render::route_explorer_tree_event`, the same function + // GTK's `explorer_ui_event` and `mouse::handle_mouse`'s own + // explorer arm call. `(1.0, 1.0)`: TUI's tree paints one + // row/column per cell, so there is no pixel metric to + // re-apply the way GTK does. + let tree_event = render::route_explorer_tree_event( + &mut self.engine, + &event, + rect, + (1.0, 1.0), + &theme, + backend, + ); match &event { UiEvent::DoubleClick { .. } => { - self.engine.explorer_has_focus = true; - self.sidebar.has_focus = true; - self.engine.dispatch_explorer_tree_event(tree_event); - } - UiEvent::MouseDown { .. } => { - if is_scrollbar { - self.explorer_sb_dragging = true; - } else { + if let Some(tree_event) = tree_event { self.engine.explorer_has_focus = true; self.sidebar.has_focus = true; + self.engine.dispatch_explorer_tree_event(tree_event); + } + } + UiEvent::MouseDown { .. } => { + if let Some(tree_event) = tree_event { + let is_scrollbar = + matches!(tree_event, quadraui::TreeControllerEvent::ScrollChanged); + if is_scrollbar { + self.explorer_sb_dragging = true; + } else { + self.engine.explorer_has_focus = true; + self.sidebar.has_focus = true; + } + self.engine.handle_explorer_mouse_event(tree_event); } - self.engine.handle_explorer_mouse_event(tree_event); } UiEvent::MouseUp { .. } => { self.explorer_sb_dragging = false; @@ -1237,32 +1245,33 @@ impl TuiShellApp { } } - /// #557: open the plugin-provided panel `name` in the sidebar. + /// #557/#754: open the plugin-provided panel `name` in the sidebar. /// - /// Verbatim mirror of `mouse::handle_mouse`'s - /// `ActivityBarTarget::ExtensionPanel` "open" branch minus its - /// toggle-vs-open decision — on the `ShellApp` path `AppShell` has already - /// made that call and reports a *second* click on the open panel's icon as - /// [`quadraui::AppShellEvent::SidebarHidden`], not `PanelChanged`. + /// The switch itself is `render::apply_activity_panel_switch` — the same + /// call `mouse::handle_mouse`'s `ActivityBarTarget::ExtensionPanel` arm + /// makes — rather than a second hand-rolled copy of it. That matters + /// here specifically: `on_shell_event`'s `PanelChanged` arm (this + /// method's only caller) can run `activate_ext_panel` again for a + /// panel that's already active — e.g. a plugin re-registering itself + /// mid-session — and the un-shared version of this method used to reset + /// `ext_panel_selected` to `0` and re-fire `plugin_event("panel_focus", + /// …)` unconditionally on *every* call, scrolling the list back to the + /// top and double-firing the plugin's own focus hook for a no-op + /// re-activation. `apply_activity_panel_switch`'s `already_showing` + /// check is exactly that guard, previously GTK-only. /// - /// The `clear_sidebar_focus()` first is the #637 fix: a plugin panel - /// taking over the sidebar body has to drop whatever built-in panel's - /// focus flag was left set, or e.g. a stale `ext_sidebar_has_focus` from - /// an earlier Extensions-marketplace visit keeps claiming clicks meant for - /// this panel. + /// A literal second left-click on the icon never reaches here at all — + /// `AppShell` reports that as [`quadraui::AppShellEvent::SidebarHidden`] + /// instead of a repeat `PanelChanged` — but `apply_activity_panel_switch` + /// still handles it correctly (hides the sidebar) on the rare path where + /// it would. fn activate_ext_panel(&mut self, name: &str) { - self.engine.clear_sidebar_focus(); - self.sidebar.ext_panel_name = Some(name.to_string()); - if !self.engine.app_shell.sidebar_visible() { - self.engine.toggle_sidebar(); + let switched = + render::apply_activity_panel_switch(&mut self.engine, &format!("ext:{name}")); + self.sidebar.ext_panel_name = switched.ext_panel; + if switched.sidebar_visible { + self.sidebar.has_focus = true; } - self.sidebar.has_focus = true; - self.engine.ext_panel_active = Some(name.to_string()); - self.engine.ext_panel_has_focus = true; - self.engine.ext_panel_selected = 0; - self.engine.plugin_event("panel_focus", name); - self.engine.session.explorer_visible = self.engine.app_shell.sidebar_visible(); - let _ = self.engine.session.save(); } } @@ -4857,6 +4866,71 @@ mod tests { ); } + /// #754: re-activating a plugin panel that's already the active one + /// (but whose sidebar was hidden through some *other* path — e.g. a + /// global sidebar-toggle key — without clearing `ext_panel_active`) + /// must not reset its scroll position or re-fire the plugin's + /// `panel_focus` hook. A second `PanelChanged` for the *visible* + /// already-active panel is the toggle-closed case, covered by + /// `on_shell_event_sidebar_hidden_clears_extension_panel_state` above + /// (`AppShell` reports that specific case as `SidebarHidden`, not a + /// repeat `PanelChanged`, but `apply_activity_panel_switch` handles it + /// the same way if one ever arrived) — this test is the other of + /// `apply_activity_panel_switch`'s two branches: `already_showing` but + /// *not* currently visible. + /// + /// `activate_ext_panel`'s pre-#754 body was TUI-only and unconditional + /// — every call, including one for a panel that's already active, reset + /// `ext_panel_selected` to `0` and called `plugin_event("panel_focus", + /// …)` again. `render::apply_activity_panel_switch`'s doc comment calls + /// this out by name: "the re-entry guard was GTK-only" — GTK's + /// `App::switch_panel` always routed through the shared function, so it + /// never had this bug; TUI's real activity-bar entry point + /// (`on_shell_event`'s `PanelChanged` arm, which every activity-bar + /// click drives — see `driver_click_on_extension_icon_opens_the_plugin_ + /// panel`, above) called this method directly and did not. Verified RED + /// against the pre-#754 `activate_ext_panel` (reinstating its old + /// unconditional body resets `ext_panel_selected` back to `0` here). + #[allow(deprecated)] + #[test] + fn reactivating_the_open_plugin_panel_does_not_reset_its_scroll_position() { + let mut app = app_with_ext_panel(); + app.on_shell_event(&quadraui::AppShellEvent::PanelChanged { + panel_id: quadraui::WidgetId::new("ext:git-insights"), + }); + assert_eq!(app.engine.ext_panel_active.as_deref(), Some("git-insights")); + + // Simulate the user having scrolled the panel's list, then hidden + // the sidebar via some path that leaves `ext_panel_active` alone + // (e.g. a global sidebar-toggle key) rather than the icon's own + // "close" click (which clears it — see + // `on_shell_event_sidebar_hidden_clears_extension_panel_state`). + app.engine.ext_panel_selected = 5; + app.engine.app_shell.hide_sidebar(); + assert!(!app.engine.app_shell.sidebar_visible()); + + app.on_shell_event(&quadraui::AppShellEvent::PanelChanged { + panel_id: quadraui::WidgetId::new("ext:git-insights"), + }); + + assert!( + app.engine.app_shell.sidebar_visible(), + "re-activating the icon must re-show the sidebar" + ); + assert_eq!( + app.engine.ext_panel_selected, 5, + "re-activating the panel that was already active must not reset \ + its selection/scroll back to the top (#754 — \ + apply_activity_panel_switch's re-entry guard, previously \ + GTK-only)" + ); + assert_eq!( + app.engine.ext_panel_active.as_deref(), + Some("git-insights"), + "and it must still be the active plugin panel" + ); + } + /// The second click on an open extension panel's icon arrives as /// `SidebarHidden` (the runner made the toggle decision itself), which /// must drop the plugin-panel state too — otherwise