add mouse support

[Panaetius]

PR Type

Enhancement

---

Description

• Add mouse support to UI components

• Enable mouse capture in terminal

• Implement mouse interactions for context menus and lists

• Enhance file tree navigation with mouse events

---

Diagram Walkthrough

flowchart LR
  A["main.rs"] -- "Enable mouse capture" --> B["TerminalBridge"]
  B -- "Mouse events" --> C["ContextMenu"]
  B -- "Mouse events" --> D["FileTree"]
  B -- "Mouse events" --> E["SystemSelectPopup"]
  B -- "Mouse events" --> F["WorkloadList"]
  C -- "Mouse actions" --> G["MenuMsg"]
  D -- "Mouse actions" --> H["FileEvent"]
  E -- "Mouse actions" --> I["SystemSelectMsg"]
  F -- "Mouse actions" --> J["JobMsg"]

Loading

File Walkthrough

Relevant files

Enhancement

context_menu.rs Add mouse support to context menu                                                coman/src/components/context_menu.rs Added mouse event support for context menu Implemented mouse position checking and selection Added mouse scroll support for navigation Integrated mouse click handling for menu actions +53/-5    file_tree.rs Add mouse support to file tree                                                      coman/src/components/file_tree.rs Added mouse event handling for file tree Implemented mouse row selection logic Added mouse scroll support for navigation Enabled right-click context menu opening +137/-19 system_select_popup.rs Add mouse support to system select popup                                  coman/src/components/system_select_popup.rs Added mouse event support for system selection popup Implemented mouse position tracking and selection Added mouse scroll support for navigation Enabled left-click selection of systems +61/-11  workload_list.rs Add mouse support to workload list                                              coman/src/components/workload_list.rs Added mouse event support for workload list Implemented mouse position tracking and selection Added mouse scroll support for navigation Enabled left/right click actions for job details/logs +60/-3    main.rs Enable mouse capture in terminal                                                  coman/src/main.rs Enabled mouse capture in terminal adapter Added mouse capture initialization for better UX Integrated mouse support across all UI components +5/-3

---

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 00d455b)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Mouse Position Calculation The mouse position calculation logic in ContextMenu may have off-by-one errors when determining the list index based on mouse coordinates, particularly around the boundaries of the component area. let mut list_index = (row - self.current_rect.y) as usize; list_index = list_index.saturating_sub(1); if list_index >= self.component.states.list_len { list_index = self.component.states.list_len; } Complex Mouse Selection Logic The mouse_select_row function in FileTree has complex logic for calculating node selection that may be prone to errors when handling scrolling and view offsets, especially with large trees. fn mouse_select_row(&mut self, row: u16) -> CmdResult { let mut list_index = (row - self.current_rect.y) as usize; list_index = list_index.saturating_sub(1); let render_area_h = self.current_rect.height as usize - 2; // adjust for border if list_index >= render_area_h { list_index = render_area_h - 1; } // the tree view auto-scrolls when selecting a node, we need to compensate for that in our // selection. See `calc_rows_to_skip` in `TreeWidget` for where this comes from. let nodes = self.node_list(); let offset_max = nodes.len().saturating_sub(render_area_h); let num_lines_to_show_at_top = render_area_h / 2; let root = self.component.tree().root().clone(); let prev = self.component.tree_state().selected().unwrap(); let prev_index = nodes.iter().position(|n| n == &&prev.to_string()).unwrap() + 1; let current_offset = prev_index.saturating_sub(num_lines_to_show_at_top).min(offset_max); list_index += current_offset; // current offset is how far the view is currently scrolled let selected = root.query(nodes[list_index]).unwrap(); if prev != selected.id() { self.attr( Attribute::Custom(TREE_INITIAL_NODE), AttrValue::String(selected.id().to_string()), ); } if self.component.tree_state().is_open(selected) { self.perform(Cmd::Custom(TREE_CMD_CLOSE)) } else { self.open_current_node() } } Mouse Capture Setup The mouse capture setup in main.rs enables mouse support but doesn't appear to handle potential errors during the enablement process, which could lead to silent failures. let mut adapter = CrosstermTerminalAdapter::new()?; adapter.enable_mouse_capture()?; let mut bridge = TerminalBridge::init(adapter).expect("Cannot initialize terminal"); bridge.enable_mouse_capture()?;

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 00d455b
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix mouse selection index bounds checking The list index calculation for mouse events does not properly handle cases where the calculated index exceeds the list length. This can lead to incorrect selection behavior or panics when accessing list items. coman/src/components/context_menu.rs [138-166] Event::Mouse(MouseEvent { kind, column: col, row, .. }) => { if !self.current_rect.contains(RectPosition { x: col, y: row }) { CmdResult::None } else { let mut list_index = (row - self.current_rect.y) as usize; list_index = list_index.saturating_sub(1); - if list_index >= self.component.states.list_len { - list_index = self.component.states.list_len; - } + list_index = list_index.min(self.component.states.list_len.saturating_sub(1)); match kind { MouseEventKind::Moved => { self.component.states.list_index = list_index; CmdResult::Changed(self.component.state()) } MouseEventKind::Down(MouseButton::Left) => { return match self.current_view { View::Workloads => ContextMenu::workload_actions(list_index), View::Files => ContextMenu::fileview_actions(list_index), }; } MouseEventKind::ScrollUp => self.perform(Cmd::Move(Direction::Up)), MouseEventKind::ScrollDown => self.perform(Cmd::Move(Direction::Down)), _ => CmdResult::None, } } } Suggestion importance[1-10]: 8 __ Why: The suggestion identifies a critical bug in the mouse event handling logic where the calculated list_index could exceed the bounds of the list, potentially causing panics or incorrect behavior. The fix ensures the index is clamped to the valid range using .min(self.component.states.list_len.saturating_sub(1)), which significantly improves robustness.	Medium
General	Add error handling for mouse capture setup The mouse capture enabling calls should be wrapped in a try block to handle potential errors gracefully during terminal initialization. coman/src/main.rs [174-177] let mut adapter = CrosstermTerminalAdapter::new()?; -adapter.enable_mouse_capture()?; +adapter.enable_mouse_capture().map_err(|e| { + eprintln!("Failed to enable mouse capture on adapter: {}", e); + e +})?; let mut bridge = TerminalBridge::init(adapter).expect("Cannot initialize terminal"); -bridge.enable_mouse_capture()?; +bridge.enable_mouse_capture().map_err(|e| { + eprintln!("Failed to enable mouse capture on bridge: {}", e); + e +})?; Suggestion importance[1-10]: 6 __ Why: The suggestion proposes improving error handling for mouse capture setup, which is a good practice for robust terminal initialization. However, since the code already uses ? for error propagation, the added map_err calls don't provide significant value and are more verbose without substantial benefit.	Low
	Validate mouse click boundaries in file tree The mouse selection logic in FileTree does not properly validate that the clicked position is within the component's boundaries before processing the click, which could lead to incorrect behavior or errors. coman/src/components/file_tree.rs [257-274] +Event::Mouse(MouseEvent { + kind, column: col, row, .. +}) => { + if !self.current_rect.contains(RectPosition { x: col, y: row }) { + CmdResult::None + } else { + match kind { + MouseEventKind::Down(MouseButton::Left) => self.mouse_select_row(row), + MouseEventKind::Down(MouseButton::Right) => { + self.mouse_select_row(row); + return Some(Msg::Menu(MenuMsg::Opened)); + } + MouseEventKind::ScrollDown => self.perform(Cmd::Scroll(Direction::Down)), + MouseEventKind::ScrollUp => self.perform(Cmd::Scroll(Direction::Up)), + _ => CmdResult::None, + } + } +} - Suggestion importance[1-10]: 3 __ Why: While the existing code already checks self.current_rect.contains(...), the suggestion suggests adding additional validation, but this check is already present. The suggestion doesn't introduce any meaningful improvement or fix, making it low impact.	Low

---

Previous suggestions

Suggestions up to commit 00d455b

Category	Suggestion	Impact
Possible issue	Fix potential out-of-bounds list index The calculation of list_index can result in an out-of-bounds access when the mouse click occurs outside the valid range of items. The assignment list_index = self.component.states.list_len should instead cap the value at self.component.states.list_len.saturating_sub(1) to prevent invalid indexing. coman/src/components/context_menu.rs [144-147] let mut list_index = (row - self.current_rect.y) as usize; list_index = list_index.saturating_sub(1); if list_index >= self.component.states.list_len { - list_index = self.component.states.list_len; + list_index = self.component.states.list_len.saturating_sub(1); } Suggestion importance[1-10]: 8 __ Why: The suggestion identifies a critical bug where list_index could exceed the valid range of items, potentially causing an out-of-bounds access. Correcting it to use saturating_sub(1) ensures safe indexing.	Medium
	Correct mouse row selection in file tree The logic for calculating list_index in mouse_select_row does not properly account for the actual number of rendered rows, which may lead to incorrect selection behavior. The condition should ensure list_index does not exceed the available nodes. coman/src/components/file_tree.rs [146-153] let mut list_index = (row - self.current_rect.y) as usize; list_index = list_index.saturating_sub(1); let render_area_h = self.current_rect.height as usize - 2; // adjust for border if list_index >= render_area_h { - list_index = render_area_h - 1; + list_index = render_area_h.saturating_sub(1); +} +let nodes = self.node_list(); +if list_index >= nodes.len() { + list_index = nodes.len().saturating_sub(1); } Suggestion importance[1-10]: 7 __ Why: The suggestion improves the robustness of row selection logic in mouse_select_row. It adds a check to ensure list_index doesn't exceed the number of available nodes, preventing incorrect selections.	Medium
General	Simplify mouse capture setup The mouse capture enabling calls are duplicated and could be simplified by ensuring mouse capture is enabled once during initialization. This avoids redundancy and potential conflicts. coman/src/main.rs [174-177] let mut adapter = CrosstermTerminalAdapter::new()?; adapter.enable_mouse_capture()?; let mut bridge = TerminalBridge::init(adapter).expect("Cannot initialize terminal"); -bridge.enable_mouse_capture()?; Suggestion importance[1-10]: 5 __ Why: The suggestion proposes simplifying redundant mouse capture enabling calls. While beneficial for code clarity, it's a minor improvement and doesn't address any functional issues.	Low

[github-actions]

Persistent review updated to latest commit 00d455b

[github-actions]

Using ? here might mask specific terminal initialization failures; consider logging or handling the error more explicitly.

Suggested change

	adapter.enable_mouse_capture()?;
	let mut adapter = CrosstermTerminalAdapter::new()?;

#ai-review-inline

[github-actions]

Adding mouse capture to the terminal bridge enhances user interaction capabilities in the TUI.

Suggested change

	bridge.enable_mouse_capture()?;
	bridge.enable_mouse_capture()?;

#ai-review-inline

[github-actions]

The explicit call to enable_mouse_capture() after initialization ensures proper mouse support for TUI interactions.

Suggested change

	let mut bridge = TerminalBridge::init(adapter).expect("Cannot initialize terminal");
	adapter.enable_mouse_capture()?;

#ai-review-inline

[github-actions]

Consider adding error handling for enable_mouse_capture() instead of using ? which may hide specific terminal initialization issues.

Suggested change

	let mut adapter = CrosstermTerminalAdapter::new()?;
	let adapter = CrosstermTerminalAdapter::new()?;

#ai-review-inline