From 7e73aaeb8b7e891459cc39f38e3f3f82fc82f99e Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 20:57:05 -0500 Subject: [PATCH 01/11] feat: add draw_context_menu_popup() for GTK DrawingArea (#135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a Cairo-based context menu renderer in gtk/draw.rs that draws from render::ContextMenuPanel — the same data TUI and Win-GUI use. Wired into the draw loop but currently no-op because GTK still closes the engine context menu before drawing (PopoverMenu is still active). The full migration (removing PopoverMenu, keeping engine context menu alive, adding click handling) is a follow-up. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/draw.rs | 117 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/src/gtk/draw.rs b/src/gtk/draw.rs index bca99017..8d033815 100644 --- a/src/gtk/draw.rs +++ b/src/gtk/draw.rs @@ -617,6 +617,17 @@ pub(super) fn draw_editor( line_height, ); *dialog_btn_rects_out.borrow_mut() = btn_rects; + + draw_context_menu_popup( + cr, + &layout, + &screen, + &theme, + width as f64, + height as f64, + char_width, + line_height, + ); } /// Draw thin Cairo horizontal scrollbars that overlay the bottom of each editor @@ -3430,6 +3441,112 @@ pub(super) fn draw_dialog_popup( rects } +/// Draw an engine-driven context menu popup on the DrawingArea. +/// Uses the same data as TUI/Win-GUI for visual consistency. +#[allow(clippy::too_many_arguments)] +pub(super) fn draw_context_menu_popup( + cr: &Context, + _layout: &pango::Layout, + screen: &render::ScreenLayout, + theme: &Theme, + editor_width: f64, + editor_height: f64, + char_width: f64, + line_height: f64, +) { + let Some(cm) = &screen.context_menu else { + return; + }; + if cm.items.is_empty() { + return; + } + + let pango_ctx = pangocairo::create_context(cr); + let ui_font_desc = FontDescription::from_string(UI_FONT); + let ui_layout = pango::Layout::new(&pango_ctx); + ui_layout.set_font_description(Some(&ui_font_desc)); + + // Calculate popup dimensions. + let sep_count = cm.items.iter().filter(|i| i.separator_after).count(); + let max_label = cm.items.iter().map(|i| i.label.len()).max().unwrap_or(4); + let max_sc = cm.items.iter().map(|i| i.shortcut.len()).max().unwrap_or(0); + let content_cols = (max_label + max_sc + 6).clamp(20, 50); + let popup_w = content_cols as f64 * char_width; + let popup_h = (cm.items.len() + sep_count + 2) as f64 * line_height; + + // Position: use char-cell coordinates from engine, scaled to pixels. + let raw_x = cm.screen_col as f64 * char_width; + let raw_y = cm.screen_row as f64 * line_height; + let px = raw_x.min(editor_width - popup_w); + let py = raw_y.min(editor_height - popup_h); + + // Background. + let (r, g, b) = theme.fuzzy_bg.to_cairo(); + cr.set_source_rgb(r, g, b); + cr.rectangle(px, py, popup_w, popup_h); + cr.fill().ok(); + + // Border. + let (r, g, b) = theme.fuzzy_border.to_cairo(); + cr.set_source_rgb(r, g, b); + cr.set_line_width(1.0); + cr.rectangle(px, py, popup_w, popup_h); + cr.stroke().ok(); + + // Items. + let mut visual_row: usize = 0; + let item_x = px + char_width; + for (i, item) in cm.items.iter().enumerate() { + let item_y = py + (visual_row + 1) as f64 * line_height; + + // Selection highlight. + if i == cm.selected_idx && item.enabled { + let (r, g, b) = theme.fuzzy_selected_bg.to_cairo(); + cr.set_source_rgb(r, g, b); + cr.rectangle(px + 1.0, item_y, popup_w - 2.0, line_height); + cr.fill().ok(); + } + + // Label. + let fg = if item.enabled { + theme.fuzzy_fg + } else { + theme.line_number_fg + }; + let (r, g, b) = fg.to_cairo(); + cr.set_source_rgb(r, g, b); + ui_layout.set_text(&item.label); + ui_layout.set_attributes(None); + cr.move_to(item_x, item_y); + pangocairo::show_layout(cr, &ui_layout); + + // Shortcut (right-aligned). + if !item.shortcut.is_empty() { + ui_layout.set_text(&item.shortcut); + let (sw, _) = ui_layout.pixel_size(); + let sc_x = px + popup_w - sw as f64 - char_width; + let (r, g, b) = theme.line_number_fg.to_cairo(); + cr.set_source_rgb(r, g, b); + cr.move_to(sc_x, item_y); + pangocairo::show_layout(cr, &ui_layout); + } + + visual_row += 1; + + // Separator line. + if item.separator_after { + let sep_y = py + (visual_row + 1) as f64 * line_height + line_height / 2.0; + let (r, g, b) = theme.fuzzy_border.to_cairo(); + cr.set_source_rgb(r, g, b); + cr.set_line_width(0.5); + cr.move_to(px + 4.0, sep_y); + cr.line_to(px + popup_w - 4.0, sep_y); + cr.stroke().ok(); + visual_row += 1; + } + } +} + /// Draw the tab bar for the bottom panel (Terminal / Debug Output). /// One row high at `(x, y)`, full width `w`. #[allow(clippy::too_many_arguments)] From fdf1542180ab3189a16f5852be0550018ac0fced Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 21:04:28 -0500 Subject: [PATCH 02/11] feat: GTK tab/editor context menus use engine-drawn rendering (#135) Migrates GTK tab-bar and editor right-click context menus from native PopoverMenu widgets to engine-driven rendering via draw_context_menu_popup(). Tab and editor right-clicks now call engine.open_*_context_menu() with char-cell coordinates, keeping the engine context menu alive for rendering. Changes: - Tab right-click: engine.open_tab_context_menu() instead of PopoverMenu - Editor right-click: engine.open_editor_context_menu() instead of PopoverMenu - Left-click handling: resolve_context_menu_click() dismisses or selects - Keyboard handling: Escape dismisses, j/k navigate, Enter confirms - draw_context_menu_popup() renders from ContextMenuPanel (same as TUI/Win-GUI) Explorer right-click still uses PopoverMenu (tied to GTK TreeView widget). Old handler methods kept with #[allow(dead_code)] for reference. Partial #135 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/mod.rs | 127 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 2 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index e3622cd4..5ce7f714 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -4063,14 +4063,26 @@ impl SimpleComponent for App { x, y, } => { - self.handle_tab_right_click(group_id, tab_idx, x, y, &sender); + let cw = self.cached_char_width.max(1.0); + let lh = self.cached_line_height.max(1.0); + let cx = (x / cw) as u16; + let cy = (y / lh) as u16; + self.engine + .borrow_mut() + .open_tab_context_menu(group_id, tab_idx, cx, cy); + self.draw_needed.set(true); } Msg::TabSwitcherRelease => { // Handled directly by the root EventControllerKey release handler. // Kept as a no-op for exhaustive match. } Msg::EditorRightClick { x, y } => { - self.handle_editor_right_click(x, y); + let cw = self.cached_char_width.max(1.0); + let lh = self.cached_line_height.max(1.0); + let cx = (x / cw) as u16; + let cy = (y / lh) as u16; + self.engine.borrow_mut().open_editor_context_menu(cx, cy); + self.draw_needed.set(true); } Msg::Resize => { // Propagate window resize to open terminal panes. @@ -5085,6 +5097,64 @@ impl App { return; } + // Dismiss context menu on any key press (Escape, or j/k for nav, Enter to confirm). + if self.engine.borrow().context_menu.is_some() { + let mut engine = self.engine.borrow_mut(); + match key_name.as_str() { + "Escape" => { + engine.close_context_menu(); + drop(engine); + self.draw_needed.set(true); + return; + } + "Return" => { + let _act = engine.context_menu_confirm(); + let needs_refresh = engine.explorer_needs_refresh; + if needs_refresh { + engine.explorer_needs_refresh = false; + } + drop(engine); + if needs_refresh { + sender.input(Msg::RefreshFileTree); + } + self.draw_needed.set(true); + return; + } + "j" | "Down" => { + if let Some(ref mut cm) = engine.context_menu { + let len = cm.items.len(); + if len > 0 { + cm.selected = (cm.selected + 1) % len; + } + } + drop(engine); + self.draw_needed.set(true); + return; + } + "k" | "Up" => { + if let Some(ref mut cm) = engine.context_menu { + let len = cm.items.len(); + if len > 0 { + cm.selected = if cm.selected > 0 { + cm.selected - 1 + } else { + len - 1 + }; + } + } + drop(engine); + self.draw_needed.set(true); + return; + } + _ => { + engine.close_context_menu(); + drop(engine); + self.draw_needed.set(true); + // Fall through to normal key handling + } + } + } + // Dismiss any panel hover popup on key press. self.engine.borrow_mut().dismiss_panel_hover_now(); if let Some(ref da) = *self.panel_hover_da.borrow() { @@ -5750,6 +5820,57 @@ impl App { alt: bool, sender: &ComponentSender, ) { + // ── Context menu click handling (engine-drawn) ── + if self.engine.borrow().context_menu.is_some() { + let cw = self.cached_char_width.max(1.0); + let lh = self.cached_line_height.max(1.0); + let click_col = (x / cw) as u16; + let click_row = (y / lh) as u16; + let term_w = (width / cw) as u16; + let term_h = (height / lh) as u16; + + let result = { + let engine = self.engine.borrow(); + let cm = engine.context_menu.as_ref().unwrap(); + crate::core::engine::resolve_context_menu_click( + &cm.items, + cm.screen_x, + cm.screen_y, + term_w, + term_h, + click_col, + click_row, + ) + }; + + use crate::core::engine::ContextMenuClickResult; + match result { + ContextMenuClickResult::Item(idx) => { + let mut engine = self.engine.borrow_mut(); + engine.context_menu.as_mut().unwrap().selected = idx; + // context_menu_confirm() handles the action internally and + // consumes the menu. + let _act = engine.context_menu_confirm(); + let needs_tree_refresh = engine.explorer_needs_refresh; + if needs_tree_refresh { + engine.explorer_needs_refresh = false; + } + drop(engine); + if needs_tree_refresh { + sender.input(Msg::RefreshFileTree); + } + } + ContextMenuClickResult::InsidePopup => { + // Click inside but not on an item — ignore + } + ContextMenuClickResult::Outside => { + self.engine.borrow_mut().close_context_menu(); + } + } + self.draw_needed.set(true); + return; + } + // ── Find/replace overlay click handling (using shared hit regions) ── if self.engine.borrow().find_replace_open { let cw = self.cached_char_width.max(1.0); @@ -6898,6 +7019,7 @@ impl App { } } + #[allow(dead_code)] fn handle_tab_right_click( &mut self, group_id: core::window::GroupId, @@ -7122,6 +7244,7 @@ impl App { } } + #[allow(dead_code)] fn handle_editor_right_click(&mut self, x: f64, y: f64) { let da = match self.drawing_area.borrow().as_ref() { Some(da) => da.clone(), From 8371acfd6f1c2b177d85257c7d527de00e7bbfba Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 21:12:41 -0500 Subject: [PATCH 03/11] fix: add mouse hover tracking for GTK engine-drawn context menus Adds EventControllerMotion on the drawing area that updates context_menu.selected as the mouse moves over items, triggering a redraw for highlight feedback. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/mod.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 5ce7f714..614f9992 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3964,6 +3964,51 @@ impl SimpleComponent for App { widgets.drawing_area.add_controller(rc_gesture); } + // Context menu hover: update selected item as mouse moves over the popup. + { + let engine_hover = engine.clone(); + let lh_cell = line_height_cell.clone(); + let cw_cell = char_width_cell.clone(); + let da_hover = widgets.drawing_area.clone(); + let motion = gtk4::EventControllerMotion::new(); + motion.connect_motion(move |_, x, y| { + let mut engine = engine_hover.borrow_mut(); + if engine.context_menu.is_none() { + return; + } + let lh = lh_cell.get(); + let cw = cw_cell.get(); + if lh < 1.0 || cw < 1.0 { + return; + } + let click_col = (x / cw) as u16; + let click_row = (y / lh) as u16; + let da_w = da_hover.width() as f64; + let da_h = da_hover.height() as f64; + let term_w = (da_w / cw) as u16; + let term_h = (da_h / lh) as u16; + + let cm = engine.context_menu.as_ref().unwrap(); + let result = crate::core::engine::resolve_context_menu_click( + &cm.items, + cm.screen_x, + cm.screen_y, + term_w, + term_h, + click_col, + click_row, + ); + if let crate::core::engine::ContextMenuClickResult::Item(idx) = result { + if engine.context_menu.as_ref().unwrap().selected != idx { + engine.context_menu.as_mut().unwrap().selected = idx; + drop(engine); + da_hover.queue_draw(); + } + } + }); + widgets.drawing_area.add_controller(motion); + } + // Tab switcher auto-confirm: poll modifier state every 50ms while open. // When neither Ctrl nor Alt is held, confirm immediately. { From 0e0797a7e694266f09169549cb5b961aa2f605f2 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 21:27:58 -0500 Subject: [PATCH 04/11] fix: use try_borrow_mut in context menu hover to avoid RefCell panic The motion callback fires during GTK event loop iterations where the engine RefCell may already be borrowed by the draw function. Using try_borrow_mut() skips the hover update gracefully instead of panicking. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 614f9992..9185cdde 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3972,7 +3972,9 @@ impl SimpleComponent for App { let da_hover = widgets.drawing_area.clone(); let motion = gtk4::EventControllerMotion::new(); motion.connect_motion(move |_, x, y| { - let mut engine = engine_hover.borrow_mut(); + let Ok(mut engine) = engine_hover.try_borrow_mut() else { + return; // Engine already borrowed (e.g. by draw function) + }; if engine.context_menu.is_none() { return; } From f61bcaf78a9ffdabeea460302977ad0377d816d0 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 21:43:03 -0500 Subject: [PATCH 05/11] fix: safe RefCell handling in context menu hover + push workflow rule - Motion callback uses try_borrow()/try_borrow_mut() with separate scopes to avoid RefCell panics in GTK's extern "C" callback - No unwrap() calls that could panic across FFI boundary - Updated CLAUDE.md: don't push until user approves smoke tests Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 5 ++-- src/gtk/mod.rs | 70 +++++++++++++++++++++++++++----------------------- 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e8807b7c..3582a77f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,8 +10,9 @@ All non-trivial work should be tracked via GitHub Issues. Issues are the source **Starting work on an issue:** 1. Create a feature branch from `develop`: `git checkout -b issue-{number}-{short-description} develop` 2. Do the work on that branch, committing as you go -3. When done, create a PR to `develop` using `gh pr create` — reference the issue with "Closes #{number}" in the PR body -4. The user reviews and merges the PR. When the user confirms the merge, immediately close the issue with `gh issue close -c "Implemented in PR #N"` — do not rely on GitHub auto-close +3. **Do NOT push or create a PR until the user has run smoke tests and confirmed the changes work.** Commit locally, offer smoke tests, wait for approval before pushing. +4. When the user approves, push and create a PR to `develop` using `gh pr create` — reference the issue with "Closes #{number}" in the PR body +5. The user reviews and merges the PR. When the user confirms the merge, immediately close the issue with `gh issue close -c "Implemented in PR #N"` — do not rely on GitHub auto-close **Creating issues:** - At session end, create issues for any planned but unstarted work discussed during the session diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 9185cdde..224ce5df 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3972,39 +3972,45 @@ impl SimpleComponent for App { let da_hover = widgets.drawing_area.clone(); let motion = gtk4::EventControllerMotion::new(); motion.connect_motion(move |_, x, y| { - let Ok(mut engine) = engine_hover.try_borrow_mut() else { - return; // Engine already borrowed (e.g. by draw function) + // Resolve hover target from immutable borrow, then update if changed. + let hover_result = { + let Ok(engine) = engine_hover.try_borrow() else { + return; + }; + let Some(ref cm) = engine.context_menu else { + return; + }; + let lh = lh_cell.get(); + let cw = cw_cell.get(); + if lh < 1.0 || cw < 1.0 { + return; + } + let col = (x / cw) as u16; + let row = (y / lh) as u16; + let tw = (da_hover.width() as f64 / cw) as u16; + let th = (da_hover.height() as f64 / lh) as u16; + let result = crate::core::engine::resolve_context_menu_click( + &cm.items, + cm.screen_x, + cm.screen_y, + tw, + th, + col, + row, + ); + let old = cm.selected; + // engine (Ref) dropped here at end of block + (result, old) }; - if engine.context_menu.is_none() { - return; - } - let lh = lh_cell.get(); - let cw = cw_cell.get(); - if lh < 1.0 || cw < 1.0 { - return; - } - let click_col = (x / cw) as u16; - let click_row = (y / lh) as u16; - let da_w = da_hover.width() as f64; - let da_h = da_hover.height() as f64; - let term_w = (da_w / cw) as u16; - let term_h = (da_h / lh) as u16; - - let cm = engine.context_menu.as_ref().unwrap(); - let result = crate::core::engine::resolve_context_menu_click( - &cm.items, - cm.screen_x, - cm.screen_y, - term_w, - term_h, - click_col, - click_row, - ); - if let crate::core::engine::ContextMenuClickResult::Item(idx) = result { - if engine.context_menu.as_ref().unwrap().selected != idx { - engine.context_menu.as_mut().unwrap().selected = idx; - drop(engine); - da_hover.queue_draw(); + if let crate::core::engine::ContextMenuClickResult::Item(idx) = hover_result.0 { + if hover_result.1 != idx { + if let Ok(mut eng) = engine_hover.try_borrow_mut() { + if let Some(ref mut cm) = eng.context_menu { + cm.selected = idx; + } + drop(eng); + da_hover.queue_draw(); + } } } }); From 46266b99043a60097844b78804f0e55c2f6137cd Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 21:48:26 -0500 Subject: [PATCH 06/11] fix: move context menu hover into existing motion handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the separate EventControllerMotion that was causing RefCell panics across GTK's extern "C" boundary. Instead, add hover logic to the existing mouse_pos_cell motion handler — uses try_borrow_mut and computes the new selection index from an immutable borrow before mutating, avoiding any unwrap() calls in the FFI callback. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/mod.rs | 95 ++++++++++++++++++++++---------------------------- 1 file changed, 42 insertions(+), 53 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 224ce5df..0befb20b 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3896,9 +3896,51 @@ impl SimpleComponent for App { { let pos_cell = mouse_pos_cell.clone(); let pos_cell_leave = mouse_pos_cell.clone(); + let engine_motion = engine.clone(); + let lh_motion = line_height_cell.clone(); + let cw_motion = char_width_cell.clone(); + let da_motion = widgets.drawing_area.clone(); let mc = gtk4::EventControllerMotion::new(); mc.connect_motion(move |_, x, y| { pos_cell.set((x, y)); + // Update context menu hover selection. + if let Ok(mut eng) = engine_motion.try_borrow_mut() { + let needs_redraw = if let Some(ref cm) = eng.context_menu { + let lh = lh_motion.get(); + let cw = cw_motion.get(); + if lh >= 1.0 && cw >= 1.0 { + let col = (x / cw) as u16; + let row = (y / lh) as u16; + let tw = (da_motion.width() as f64 / cw) as u16; + let th = (da_motion.height() as f64 / lh) as u16; + match crate::core::engine::resolve_context_menu_click( + &cm.items, + cm.screen_x, + cm.screen_y, + tw, + th, + col, + row, + ) { + crate::core::engine::ContextMenuClickResult::Item(idx) + if cm.selected != idx => + { + Some(idx) + } + _ => None, + } + } else { + None + } + } else { + None + }; + if let Some(idx) = needs_redraw { + eng.context_menu.as_mut().unwrap().selected = idx; + drop(eng); + da_motion.queue_draw(); + } + } }); mc.connect_leave(move |_| { pos_cell_leave.set((-1.0, -1.0)); @@ -3964,59 +4006,6 @@ impl SimpleComponent for App { widgets.drawing_area.add_controller(rc_gesture); } - // Context menu hover: update selected item as mouse moves over the popup. - { - let engine_hover = engine.clone(); - let lh_cell = line_height_cell.clone(); - let cw_cell = char_width_cell.clone(); - let da_hover = widgets.drawing_area.clone(); - let motion = gtk4::EventControllerMotion::new(); - motion.connect_motion(move |_, x, y| { - // Resolve hover target from immutable borrow, then update if changed. - let hover_result = { - let Ok(engine) = engine_hover.try_borrow() else { - return; - }; - let Some(ref cm) = engine.context_menu else { - return; - }; - let lh = lh_cell.get(); - let cw = cw_cell.get(); - if lh < 1.0 || cw < 1.0 { - return; - } - let col = (x / cw) as u16; - let row = (y / lh) as u16; - let tw = (da_hover.width() as f64 / cw) as u16; - let th = (da_hover.height() as f64 / lh) as u16; - let result = crate::core::engine::resolve_context_menu_click( - &cm.items, - cm.screen_x, - cm.screen_y, - tw, - th, - col, - row, - ); - let old = cm.selected; - // engine (Ref) dropped here at end of block - (result, old) - }; - if let crate::core::engine::ContextMenuClickResult::Item(idx) = hover_result.0 { - if hover_result.1 != idx { - if let Ok(mut eng) = engine_hover.try_borrow_mut() { - if let Some(ref mut cm) = eng.context_menu { - cm.selected = idx; - } - drop(eng); - da_hover.queue_draw(); - } - } - } - }); - widgets.drawing_area.add_controller(motion); - } - // Tab switcher auto-confirm: poll modifier state every 50ms while open. // When neither Ctrl nor Alt is held, confirm immediately. { From fa1bea4c8bacc57e9b848b535bfeecc9495f5d1f Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 21:51:08 -0500 Subject: [PATCH 07/11] =?UTF-8?q?fix:=20remove=20context=20menu=20hover=20?= =?UTF-8?q?=E2=80=94=20GTK=20RefCell=20conflicts=20in=20motion=20callbacks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mouse hover highlighting in engine-drawn context menus causes RefCell panics across GTK's extern "C" FFI boundary. Removed all hover logic. Context menus still work via click and keyboard (j/k/Enter/Escape). Hover can be revisited when the drawing architecture avoids holding engine borrows during GTK signal dispatch. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/mod.rs | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 0befb20b..5ce7f714 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3896,51 +3896,9 @@ impl SimpleComponent for App { { let pos_cell = mouse_pos_cell.clone(); let pos_cell_leave = mouse_pos_cell.clone(); - let engine_motion = engine.clone(); - let lh_motion = line_height_cell.clone(); - let cw_motion = char_width_cell.clone(); - let da_motion = widgets.drawing_area.clone(); let mc = gtk4::EventControllerMotion::new(); mc.connect_motion(move |_, x, y| { pos_cell.set((x, y)); - // Update context menu hover selection. - if let Ok(mut eng) = engine_motion.try_borrow_mut() { - let needs_redraw = if let Some(ref cm) = eng.context_menu { - let lh = lh_motion.get(); - let cw = cw_motion.get(); - if lh >= 1.0 && cw >= 1.0 { - let col = (x / cw) as u16; - let row = (y / lh) as u16; - let tw = (da_motion.width() as f64 / cw) as u16; - let th = (da_motion.height() as f64 / lh) as u16; - match crate::core::engine::resolve_context_menu_click( - &cm.items, - cm.screen_x, - cm.screen_y, - tw, - th, - col, - row, - ) { - crate::core::engine::ContextMenuClickResult::Item(idx) - if cm.selected != idx => - { - Some(idx) - } - _ => None, - } - } else { - None - } - } else { - None - }; - if let Some(idx) = needs_redraw { - eng.context_menu.as_mut().unwrap().selected = idx; - drop(eng); - da_motion.queue_draw(); - } - } }); mc.connect_leave(move |_| { pos_cell_leave.set((-1.0, -1.0)); From 1b7a91f7ac3f2d8ed4b4e147a65287f307fd32bb Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 22:01:09 -0500 Subject: [PATCH 08/11] fix: context menu hover via draw-time mouse position lookup Instead of mutating engine state in a motion callback (which causes RefCell panics across GTK's extern C boundary), compute the hovered item inside draw_context_menu_popup() from the existing mouse_pos_cell. Motion callback just triggers queue_draw() when a context menu is open (using try_borrow to safely check). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/draw.rs | 36 +++++++++++++++++++++++++++++++++++- src/gtk/mod.rs | 10 ++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/gtk/draw.rs b/src/gtk/draw.rs index 8d033815..80175219 100644 --- a/src/gtk/draw.rs +++ b/src/gtk/draw.rs @@ -627,6 +627,7 @@ pub(super) fn draw_editor( height as f64, char_width, line_height, + mouse_pos, ); } @@ -3453,6 +3454,7 @@ pub(super) fn draw_context_menu_popup( editor_height: f64, char_width: f64, line_height: f64, + mouse_pos: (f64, f64), ) { let Some(cm) = &screen.context_menu else { return; @@ -3493,6 +3495,38 @@ pub(super) fn draw_context_menu_popup( cr.rectangle(px, py, popup_w, popup_h); cr.stroke().ok(); + // Compute hovered item from mouse position (avoids engine borrow in motion callback). + let hover_idx: Option = if mouse_pos.0 >= 0.0 { + let mcol = (mouse_pos.0 / char_width) as u16; + let mrow = (mouse_pos.1 / line_height) as u16; + let tw = (editor_width / char_width) as u16; + let th = (editor_height / line_height) as u16; + match crate::core::engine::resolve_context_menu_click( + &cm.items + .iter() + .map(|i| crate::core::engine::ContextMenuItem { + label: i.label.clone(), + action: String::new(), + shortcut: i.shortcut.clone(), + separator_after: i.separator_after, + enabled: i.enabled, + }) + .collect::>(), + cm.screen_col, + cm.screen_row, + tw, + th, + mcol, + mrow, + ) { + crate::core::engine::ContextMenuClickResult::Item(idx) => Some(idx), + _ => None, + } + } else { + None + }; + let selected = hover_idx.unwrap_or(cm.selected_idx); + // Items. let mut visual_row: usize = 0; let item_x = px + char_width; @@ -3500,7 +3534,7 @@ pub(super) fn draw_context_menu_popup( let item_y = py + (visual_row + 1) as f64 * line_height; // Selection highlight. - if i == cm.selected_idx && item.enabled { + if i == selected && item.enabled { let (r, g, b) = theme.fuzzy_selected_bg.to_cairo(); cr.set_source_rgb(r, g, b); cr.rectangle(px + 1.0, item_y, popup_w - 2.0, line_height); diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 5ce7f714..dcc85fed 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3896,9 +3896,19 @@ impl SimpleComponent for App { { let pos_cell = mouse_pos_cell.clone(); let pos_cell_leave = mouse_pos_cell.clone(); + let engine_motion = engine.clone(); + let da_motion = widgets.drawing_area.clone(); let mc = gtk4::EventControllerMotion::new(); mc.connect_motion(move |_, x, y| { pos_cell.set((x, y)); + // Trigger redraw for context menu hover highlight. + // Uses try_borrow to avoid panic if engine is borrowed by draw. + if let Ok(eng) = engine_motion.try_borrow() { + if eng.context_menu.is_some() { + drop(eng); + da_motion.queue_draw(); + } + } }); mc.connect_leave(move |_| { pos_cell_leave.set((-1.0, -1.0)); From 02d24c2dd810529510b49c9e45e26fc8a36eceac Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 22:05:22 -0500 Subject: [PATCH 09/11] fix: context menu overflow crash + hover snapping to first item - resolve_context_menu_click: use saturating_sub to prevent overflow when click_row == py (clicking on the top border) - draw_context_menu_popup: only highlight when mouse is directly over an item; when mouse is on separator/border inside popup, no highlight; when mouse is outside popup, fall back to keyboard selection Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/engine/mod.rs | 2 +- src/gtk/draw.rs | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/core/engine/mod.rs b/src/core/engine/mod.rs index bdd795ba..30254096 100644 --- a/src/core/engine/mod.rs +++ b/src/core/engine/mod.rs @@ -1227,7 +1227,7 @@ pub fn resolve_context_menu_click( } // Inside popup — find which item - let inner_row = click_row - py - 1; // -1 for top border + let inner_row = click_row.saturating_sub(py).saturating_sub(1); // -1 for top border let mut visual_row: u16 = 0; for (i, item) in items.iter().enumerate() { if visual_row == inner_row && item.enabled { diff --git a/src/gtk/draw.rs b/src/gtk/draw.rs index 80175219..6d665ef6 100644 --- a/src/gtk/draw.rs +++ b/src/gtk/draw.rs @@ -3525,7 +3525,25 @@ pub(super) fn draw_context_menu_popup( } else { None }; - let selected = hover_idx.unwrap_or(cm.selected_idx); + // Use hover index if mouse is over an item; otherwise use keyboard selection + // only if the mouse is outside the popup (so keyboard nav still works). + let mouse_inside_popup = if mouse_pos.0 >= 0.0 { + let mcol = (mouse_pos.0 / char_width) as u16; + let mrow = (mouse_pos.1 / line_height) as u16; + mcol >= (px / char_width) as u16 + && mcol < ((px + popup_w) / char_width) as u16 + && mrow >= (py / line_height) as u16 + && mrow < ((py + popup_h) / line_height) as u16 + } else { + false + }; + let selected = if let Some(idx) = hover_idx { + idx + } else if mouse_inside_popup { + usize::MAX // no highlight (mouse on separator/border) + } else { + cm.selected_idx // keyboard selection when mouse is outside + }; // Items. let mut visual_row: usize = 0; From 8bdb2175681cf6ac557458d4ea73fed26c09f588 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 22:11:35 -0500 Subject: [PATCH 10/11] fix: persist hover selection + update on mouse move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Motion callback updates engine.context_menu.selected via try_borrow_mut so the selection persists when mouse leaves the popup - Draw function uses hover_idx (from mouse_pos) with fallback to cm.selected_idx — keeps last-hovered item highlighted when mouse exits - Disabled items already use theme.line_number_fg (dim grey) vs theme.fuzzy_fg (normal text) for visual distinction Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/draw.rs | 22 +++------------------- src/gtk/mod.rs | 32 +++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/gtk/draw.rs b/src/gtk/draw.rs index 6d665ef6..d39b5e96 100644 --- a/src/gtk/draw.rs +++ b/src/gtk/draw.rs @@ -3525,25 +3525,9 @@ pub(super) fn draw_context_menu_popup( } else { None }; - // Use hover index if mouse is over an item; otherwise use keyboard selection - // only if the mouse is outside the popup (so keyboard nav still works). - let mouse_inside_popup = if mouse_pos.0 >= 0.0 { - let mcol = (mouse_pos.0 / char_width) as u16; - let mrow = (mouse_pos.1 / line_height) as u16; - mcol >= (px / char_width) as u16 - && mcol < ((px + popup_w) / char_width) as u16 - && mrow >= (py / line_height) as u16 - && mrow < ((py + popup_h) / line_height) as u16 - } else { - false - }; - let selected = if let Some(idx) = hover_idx { - idx - } else if mouse_inside_popup { - usize::MAX // no highlight (mouse on separator/border) - } else { - cm.selected_idx // keyboard selection when mouse is outside - }; + // Use hover index if mouse is over an item; otherwise keep engine selection + // (preserves last-hovered or keyboard-navigated item when mouse leaves). + let selected = hover_idx.unwrap_or(cm.selected_idx); // Items. let mut visual_row: usize = 0; diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index dcc85fed..e7280c65 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3897,14 +3897,40 @@ impl SimpleComponent for App { let pos_cell = mouse_pos_cell.clone(); let pos_cell_leave = mouse_pos_cell.clone(); let engine_motion = engine.clone(); + let lh_motion = line_height_cell.clone(); + let cw_motion = char_width_cell.clone(); let da_motion = widgets.drawing_area.clone(); let mc = gtk4::EventControllerMotion::new(); mc.connect_motion(move |_, x, y| { pos_cell.set((x, y)); - // Trigger redraw for context menu hover highlight. - // Uses try_borrow to avoid panic if engine is borrowed by draw. - if let Ok(eng) = engine_motion.try_borrow() { + // Update context menu hover: persist selected index so it + // sticks when the mouse leaves. try_borrow_mut fails during + // draw (engine immutably borrowed) — that's fine, the draw + // function computes hover from mouse_pos directly. + if let Ok(mut eng) = engine_motion.try_borrow_mut() { if eng.context_menu.is_some() { + let lh = lh_motion.get(); + let cw = cw_motion.get(); + if lh >= 1.0 && cw >= 1.0 { + let col = (x / cw) as u16; + let row = (y / lh) as u16; + let tw = (da_motion.width() as f64 / cw) as u16; + let th = (da_motion.height() as f64 / lh) as u16; + let cm = eng.context_menu.as_ref().unwrap(); + if let crate::core::engine::ContextMenuClickResult::Item(idx) = + crate::core::engine::resolve_context_menu_click( + &cm.items, + cm.screen_x, + cm.screen_y, + tw, + th, + col, + row, + ) + { + eng.context_menu.as_mut().unwrap().selected = idx; + } + } drop(eng); da_motion.queue_draw(); } From b331d18111c6407a2353bff9f7f896fd2770ec53 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Fri, 17 Apr 2026 22:13:58 -0500 Subject: [PATCH 11/11] fix: disabled context menu items visually distinct with darken(0.5) Use fuzzy_fg.darken(0.5) for disabled items instead of line_number_fg which was too similar to the normal text color on some themes. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gtk/draw.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gtk/draw.rs b/src/gtk/draw.rs index d39b5e96..d93101da 100644 --- a/src/gtk/draw.rs +++ b/src/gtk/draw.rs @@ -3543,11 +3543,11 @@ pub(super) fn draw_context_menu_popup( cr.fill().ok(); } - // Label. + // Label — disabled items heavily darkened for obvious visual distinction. let fg = if item.enabled { theme.fuzzy_fg } else { - theme.line_number_fg + theme.fuzzy_fg.darken(0.5) }; let (r, g, b) = fg.to_cairo(); cr.set_source_rgb(r, g, b);