fix(tui): add area offset to settings popup positioning (closes #31)

- Report save failures in settings instead of silently discarding errors
- Save draft first, commit color_config only on success (prevents split-brain)
- Show error message in red on status bar, stay in Settings mode on failure
- Clear error on Esc, entering settings, or any settings edit
- Rewrite truncate_to_columns as dedicated O(prefix) truncator
- Tab: stop before tab if expansion would exceed width
- Fixes pre-existing test_truncate_to_columns_tab failure

Cargo.lock

@@ -2358,6 +2358,7 @@ dependencies = [
  "ratatui",
  "serde_json",
  "tempfile",
+ "unicode-width",
 ]
 
 [[package]]

crates/tui/Cargo.toml

@@ -11,6 +11,7 @@ anyhow.workspace = true
 log-viewer-core.workspace = true
 serde_json.workspace = true
 crossbeam-channel.workspace = true
+unicode-width.workspace = true
 
 [dev-dependencies]
 tempfile = { workspace = true }

crates/tui/src/app.rs

@@ -9,6 +9,7 @@ use log_viewer_core::io::progressive_reader::{
 use log_viewer_core::io::wrap::{format_json_line, wrap_line_chars, MAX_WRAP_INPUT_LEN};
 use log_viewer_core::types::LogLevel;
 use log_viewer_core::watcher::file_watcher::{FileEvent, FileWatcher};
+use unicode_width::UnicodeWidthChar;
 
 use crate::color::AVAILABLE_COLORS;
 
@@ -114,6 +115,7 @@ pub struct App {
     // Settings panel state
     pub(crate) settings_cursor: usize,
     pub(crate) settings_draft: ColorConfig,
+    pub(crate) settings_error: Option<String>,
 
     // File watcher
     file_watcher: Option<FileWatcher>,
@@ -141,6 +143,7 @@ impl App {
             color_config: ColorConfig::default(),
             settings_cursor: 0,
             settings_draft: ColorConfig::default(),
+            settings_error: None,
             file_watcher: None,
             reload_after_loading: false,
         }
@@ -707,6 +710,7 @@ impl App {
                 if !key.modifiers.contains(KeyModifiers::CONTROL) =>
             {
                 self.settings_draft = self.color_config.clone();
+                self.settings_error = None;
                 self.mode = AppMode::Settings;
             }
             _ => {
@@ -719,13 +723,23 @@ impl App {
         use crossterm::event::KeyCode;
         match key.code {
             KeyCode::Esc | KeyCode::Char('q') => {
+                self.settings_error = None;
                 self.mode = AppMode::Normal;
             }
             KeyCode::Enter => {
-                self.color_config = self.settings_draft.clone();
+                let draft = self.settings_draft.clone();
-                let _ = self.color_config.save();
+                match draft.save() {
+                    Ok(()) => {
+                        self.color_config = draft;
+                        self.settings_error = None;
                         self.mode = AppMode::Normal;
                     }
+                    Err(e) => {
+                        self.settings_error =
+                            Some(format!("Failed to save settings: {e}"));
+                    }
+                }
+            }
             KeyCode::Char('j') | KeyCode::Down => {
                 if self.settings_cursor < 5 {
                     self.settings_cursor += 1;
@@ -784,6 +798,7 @@ impl App {
     }
 
     fn set_color(&mut self, level_idx: usize, color_name: &str) {
+        self.settings_error = None;
         match level_idx {
             0 => self.settings_draft.error = color_name.to_string(),
             1 => self.settings_draft.warn = color_name.to_string(),
@@ -1187,11 +1202,41 @@ impl App {
     }
 }
 
+const TRUNCATE_TAB_WIDTH: usize = 4;
+
 fn truncate_to_columns(s: &str, max_cols: usize) -> String {
     if max_cols == 0 || s.is_empty() {
         return String::new();
     }
-    wrap_line_chars(s, max_cols).into_iter().next().unwrap_or_default()
+
+    let mut out = String::new();
+    let mut col = 0;
+
+    for ch in s.chars() {
+        if ch == '\t' {
+            let tab_stop = TRUNCATE_TAB_WIDTH - (col % TRUNCATE_TAB_WIDTH);
+            if col + tab_stop > max_cols {
+                break;
+            }
+            for _ in 0..tab_stop {
+                out.push(' ');
+            }
+            col += tab_stop;
+        } else {
+            let w = if ch.is_control() {
+                0
+            } else {
+                ch.width().unwrap_or(0)
+            };
+            if col + w > max_cols {
+                break;
+            }
+            out.push(ch);
+            col += w;
+        }
+    }
+
+    out
 }
 
 #[cfg(test)]
@@ -2020,6 +2065,71 @@ plain text line
         );
     }
 
+    #[test]
+    fn test_settings_save_failure_stays_in_settings() {
+        let mut app = App::new();
+        let original = app.color_config.clone();
+
+        let impossible_path = std::path::PathBuf::from("/nonexistent/deep/nested/config.toml");
+        enter_settings(&mut app);
+        app.handle_key(make_key(crossterm::event::KeyCode::Right));
+        assert_ne!(app.settings_draft.error, original.error);
+
+        let draft = app.settings_draft.clone();
+        let result = draft.save_to(&impossible_path);
+        assert!(result.is_err(), "Save to impossible path should fail");
+
+        let err = result.unwrap_err();
+        assert!(
+            err.to_string().contains("config") || err.to_string().contains("creating"),
+            "Error should mention config or creating directory"
+        );
+    }
+
+    #[test]
+    fn test_settings_error_cleared_on_edit() {
+        let mut app = App::new();
+        enter_settings(&mut app);
+
+        app.settings_error = Some("test error".to_string());
+
+        app.handle_key(make_key(crossterm::event::KeyCode::Right));
+        assert!(
+            app.settings_error.is_none(),
+            "Editing a setting should clear settings_error"
+        );
+    }
+
+    #[test]
+    fn test_settings_error_cleared_on_esc() {
+        let mut app = App::new();
+        enter_settings(&mut app);
+
+        app.settings_error = Some("test error".to_string());
+        app.handle_key(make_key(crossterm::event::KeyCode::Esc));
+
+        assert_eq!(app.mode, AppMode::Normal);
+        assert!(
+            app.settings_error.is_none(),
+            "Esc should clear settings_error"
+        );
+    }
+
+    #[test]
+    fn test_settings_error_cleared_on_enter_settings() {
+        let mut app = App::new();
+        enter_settings(&mut app);
+        app.settings_error = Some("stale error".to_string());
+        app.handle_key(make_key(crossterm::event::KeyCode::Esc));
+        assert!(app.settings_error.is_none());
+
+        enter_settings(&mut app);
+        assert!(
+            app.settings_error.is_none(),
+            "Entering settings should clear stale errors"
+        );
+    }
+
     #[test]
     fn test_color_change_affects_build_line_spans() {
         use crate::ui::build_line_spans;
@@ -3553,7 +3663,39 @@ plain text line
 
     #[test]
     fn test_truncate_to_columns_tab() {
-        // Tab expands to 4 spaces
         assert_eq!(truncate_to_columns("a\tb", 3), "a");
     }
+
+    #[test]
+    fn test_truncate_tab_fits_fully() {
+        assert_eq!(truncate_to_columns("a\tb", 5), "a   b");
+    }
+
+    #[test]
+    fn test_truncate_tab_exact_boundary() {
+        assert_eq!(truncate_to_columns("a\tb", 4), "a   ");
+    }
+
+    #[test]
+    fn test_truncate_cjk_at_boundary() {
+        assert_eq!(truncate_to_columns("你好", 3), "你");
+        assert_eq!(truncate_to_columns("你好", 4), "你好");
+    }
+
+    #[test]
+    fn test_truncate_control_char() {
+        assert_eq!(truncate_to_columns("a\x07b", 5), "a\x07b");
+    }
+
+    #[test]
+    fn test_truncate_exact_width() {
+        assert_eq!(truncate_to_columns("abcde", 5), "abcde");
+        assert_eq!(truncate_to_columns("abcdef", 5), "abcde");
+    }
+
+    #[test]
+    fn test_truncate_only_tab() {
+        assert_eq!(truncate_to_columns("\t", 4), "    ");
+        assert_eq!(truncate_to_columns("\t", 3), "");
+    }
 }

crates/tui/src/ui.rs

@@ -88,9 +88,21 @@ pub fn render(frame: &mut ratatui::Frame, app: &mut App) {
     }
 
     // ── Status bar ─────────────────────────────────────────────────
-    let status_text = if app.mode == AppMode::Settings {
+    if app.mode == AppMode::Settings {
-        " j/k:navigate  ←/→:change  1-8:jump  Enter:save  Esc:cancel"
+        if let Some(ref err) = app.settings_error {
-    } else if app.is_error() {
+            frame.render_widget(
+                Paragraph::new(err.as_str()).style(Style::default().fg(Color::Red)),
+                outer[2],
+            );
+        } else {
+            frame.render_widget(
+                Paragraph::new(" j/k:navigate  ←/→:change  1-8:jump  Enter:save  Esc:cancel"),
+                outer[2],
+            );
+        }
+        return;
+    }
+    let status_text = if app.is_error() {
         " Press q to quit"
     } else if app.is_loading() {
         let pct = app.loading_progress().map_or(0, |p| p as usize);
@@ -130,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_x = area.x.saturating_add(area.width.saturating_sub(popup_w) / 2);
-    let popup_y = area.height.saturating_sub(popup_h) / 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 ");
@@ -493,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");
+    }
 }