From 10323ce814d4ae03c755e7d097e8b89c8c8beef8 Mon Sep 17 00:00:00 2001 From: dailz Date: Thu, 11 Jun 2026 16:49:37 +0800 Subject: [PATCH] fix(tui): add area offset to settings popup positioning (closes #31) --- crates/tui/src/ui.rs | 74 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/ui.rs b/crates/tui/src/ui.rs index f92a540..1f3ba7f 100644 --- a/crates/tui/src/ui.rs +++ b/crates/tui/src/ui.rs @@ -142,8 +142,8 @@ pub fn render_settings(frame: &mut ratatui::Frame, app: &mut App, area: ratatui: let popup_w = ((area.width as u32 * 4 / 5).max(40)).min(area.width as u32) as u16; let popup_h = ((area.height as u32 * 4 / 5).max(14)).min(area.height as u32) as u16; - let popup_x = area.width.saturating_sub(popup_w) / 2; - let popup_y = area.height.saturating_sub(popup_h) / 2; + let popup_x = area.x.saturating_add(area.width.saturating_sub(popup_w) / 2); + let popup_y = area.y.saturating_add(area.height.saturating_sub(popup_h) / 2); let popup = ratatui::layout::Rect::new(popup_x, popup_y, popup_w, popup_h); let block = Block::new().borders(Borders::ALL).title(" Color Settings "); @@ -505,4 +505,74 @@ mod tests { let _ = std::fs::remove_file(&path); assert!(result.is_ok()); } + + // ── Issue #31: Settings popup area offset tests ──────────────── + + /// Helper: enter settings mode and render to buffer. + fn render_settings_to_buffer(app: &mut App, width: u16, height: u16) -> ratatui::buffer::Buffer { + app.mode = crate::app::AppMode::Settings; + app.settings_draft = app.color_config.clone(); + render_to_buffer(app, width, height) + } + + /// Find the top-left corner of the popup border by scanning for '┌'. + fn find_popup_top_left(buf: &ratatui::buffer::Buffer, width: u16, height: u16) -> Option<(u16, u16)> { + for row in 0..height { + for col in 0..width { + if buf.cell((col, row)).unwrap().symbol() == "┌" { + return Some((col, row)); + } + } + } + None + } + + #[test] + fn test_settings_popup_includes_area_offset() { + // In an 80x24 frame with Layout [Length(1), Min(1), Length(1)]: + // outer[0] = title bar -> y=0 + // outer[1] = content -> y=1, height=22 + // outer[2] = status bar -> y=23 + // The popup is centered within outer[1], so its y must be >= outer[1].y (which is 1). + let mut app = App::new(); + let buf = render_settings_to_buffer(&mut app, 80, 24); + + let (_px, py) = find_popup_top_left(&buf, 80, 24) + .expect("popup border '┌' should be rendered"); + + // outer[1].y == 1; the popup is centered inside a 22-row area, + // so popup_y must be at least 1 (not 0). + assert!( + py >= 1, + "popup top row should account for area.y offset, got y={py} (expected >= 1)" + ); + } + + #[test] + fn test_settings_popup_horizontal_centering_uses_area_x() { + // outer[1].x is 0 for this layout, so this mainly verifies the popup + // is centered and not shifted left. A non-zero area.x layout would + // need a different layout to trigger, but the formula is the same. + let mut app = App::new(); + let buf = render_settings_to_buffer(&mut app, 80, 24); + + let (px, _py) = find_popup_top_left(&buf, 80, 24) + .expect("popup border '┌' should be rendered"); + + // popup_w = 80*4/5 = 64, centered: (80-64)/2 = 8 + assert_eq!( + px, 8, + "popup should start at x=8 (centered 64-wide popup in 80-col area)" + ); + } + + #[test] + fn test_settings_popup_small_frame_no_panic() { + // Frame smaller than the min popup size (40x14) should not panic. + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let mut app = App::new(); + let _buf = render_settings_to_buffer(&mut app, 30, 10); + })); + assert!(result.is_ok(), "rendering settings in a small frame should not panic"); + } }