diff --git a/crates/fresh-editor/src/view/controls/toggle/render.rs b/crates/fresh-editor/src/view/controls/toggle/render.rs index 653351d0d0..748172eccf 100644 --- a/crates/fresh-editor/src/view/controls/toggle/render.rs +++ b/crates/fresh-editor/src/view/controls/toggle/render.rs @@ -6,8 +6,19 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; use ratatui::Frame; +use crate::primitives::display_width::str_width; + use super::{FocusState, ToggleColors, ToggleLayout, ToggleState}; +fn pad_to_display_width(label: &str, width: u16) -> String { + let width = width as usize; + let padding = width.saturating_sub(str_width(label)); + let mut padded = String::with_capacity(label.len() + padding); + padded.push_str(label); + padded.extend(std::iter::repeat_n(' ', padding)); + padded +} + /// Render a toggle control /// /// # Arguments @@ -65,12 +76,11 @@ pub fn render_toggle_aligned( }; // Format: "Label: [v]" / "Label: [ ]" with optional padding. - let actual_label_width = label_width.unwrap_or(state.label.len() as u16); - let padded_label = format!( - "{:width$}", - state.label, - width = actual_label_width as usize - ); + let label_display_width = str_width(&state.label) as u16; + let actual_label_width = label_width + .unwrap_or(label_display_width) + .max(label_display_width); + let padded_label = pad_to_display_width(&state.label, actual_label_width); // Compact checkbox glyph — matches the widget framework's // `[v]` / `[ ]` convention so an empty checkbox is not visually @@ -132,3 +142,16 @@ pub fn render_toggle_aligned( full_area, } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn pad_to_display_width_uses_terminal_columns() { + let padded = pad_to_display_width("你好", 6); + + assert_eq!(str_width(&padded), 6); + assert_eq!(padded, "你好 "); + } +} diff --git a/crates/fresh-editor/src/view/settings/layout.rs b/crates/fresh-editor/src/view/settings/layout.rs index 39229957b2..a0432f2db6 100644 --- a/crates/fresh-editor/src/view/settings/layout.rs +++ b/crates/fresh-editor/src/view/settings/layout.rs @@ -508,15 +508,16 @@ mod tests { 0, "/test".to_string(), Rect::new(35, 10, 50, 2), - ControlLayoutInfo::Toggle(Rect::new(37, 11, 15, 1)), + ControlLayoutInfo::Toggle(Rect::new(49, 11, 3, 1)), None, ); - // Click on toggle control - assert_eq!(layout.hit_test(40, 11), Some(SettingsHit::ControlToggle(0))); + // Click on toggle chip + assert_eq!(layout.hit_test(50, 11), Some(SettingsHit::ControlToggle(0))); - // Click on item but not on toggle + // Click on item/label but not on toggle assert_eq!(layout.hit_test(35, 10), Some(SettingsHit::Item(0))); + assert_eq!(layout.hit_test(40, 11), Some(SettingsHit::Item(0))); } /// Reproducer for issue #1825: clicking on the value area between the diff --git a/crates/fresh-editor/src/view/settings/mouse.rs b/crates/fresh-editor/src/view/settings/mouse.rs index a6f2a2e7ba..c626101020 100644 --- a/crates/fresh-editor/src/view/settings/mouse.rs +++ b/crates/fresh-editor/src/view/settings/mouse.rs @@ -291,6 +291,12 @@ impl Editor { SettingsHit::CategorySection(cat_idx, section_idx) => { if let Some(ref mut state) = self.settings_state { state.jump_to_section(cat_idx, section_idx); + // Mouse clicks in the left tree should keep the tree as + // the focused panel, just like clicking a top-level + // category. `jump_to_section` is also used by search and + // keyboard flows where moving focus to the settings body + // is correct, so restore the mouse-specific focus here. + state.focus.set(FocusPanel::Categories); } } SettingsHit::CategoriesPanel | SettingsHit::CategoriesScrollbar => { @@ -1011,10 +1017,10 @@ impl Editor { // Fallback: if the row text is shorter than the field // (common — items rarely fill 30 chars), the user clicks // the [x] which is right after `]`. Use a hand-rolled - // approximation: text_indent + field_width + 1 ≤ col. + // approximation: text_indent + field_width < col. let text_indent = _layout.dialog_x + 2 + 3 /* focus indicator */ + 2 /* TextList indent */ + 1 /* `[` */; let estimated_field_width = 28u16; - col >= text_indent + estimated_field_width + 1 + col > text_indent + estimated_field_width }; match (on_add_row, item_row_idx, in_trailing_button) { diff --git a/crates/fresh-editor/src/view/settings/render.rs b/crates/fresh-editor/src/view/settings/render.rs index fb772fb8be..1b44c4de29 100644 --- a/crates/fresh-editor/src/view/settings/render.rs +++ b/crates/fresh-editor/src/view/settings/render.rs @@ -4,7 +4,7 @@ use rust_i18n::t; -use crate::primitives::display_width::str_width; +use crate::primitives::display_width::{char_width, str_width}; use super::entry_dialog::EntryDialogState; use super::items::SettingControl; @@ -120,6 +120,39 @@ fn truncate_chars_with_ellipsis(s: &str, max_chars: usize) -> String { } } +/// Truncate `s` to at most `max_width` terminal columns, appending `"..."`. +/// +/// Unlike [`truncate_chars_with_ellipsis`], this is display-width aware so a +/// CJK or emoji label cannot overflow a fixed-width TUI cell after truncation. +fn truncate_display_width_with_ellipsis(s: &str, max_width: usize) -> String { + if str_width(s) <= max_width { + return s.to_string(); + } + if max_width == 0 { + return String::new(); + } + + let ellipsis = "..."; + let ellipsis_width = str_width(ellipsis); + if max_width <= ellipsis_width { + return ".".repeat(max_width); + } + + let target_width = max_width - ellipsis_width; + let mut kept = String::new(); + let mut used = 0usize; + for ch in s.chars() { + let width = char_width(ch); + if used + width > target_width { + break; + } + kept.push(ch); + used += width; + } + kept.push_str(ellipsis); + kept +} + /// Render the settings modal pub fn render_settings( frame: &mut Frame, @@ -408,7 +441,7 @@ fn render_categories_horizontal( for (i, page) in state.pages.iter().enumerate() { let is_selected = i == state.selected_category; - let has_modified = page.items.iter().any(|item| item.modified); + let has_modified = state.page_has_pending_changes(i); let indicator = if has_modified { "● " } else { " " }; let name = &page.name; @@ -524,6 +557,7 @@ fn render_categories( has_changes: bool, indent_cols: u16, is_category: bool, + is_plugin_category: bool, cat_idx: Option, section_idx: Option, label: String, @@ -552,9 +586,10 @@ fn render_categories( // Category row is "selected" iff the keyboard cursor // is sitting on it (no section is the cursor target). is_selected: idx == selected_category && tree_cursor.is_none(), - has_changes: page.items.iter().any(|i| i.modified), + has_changes: state.page_has_pending_changes(idx), indent_cols: 0, is_category: true, + is_plugin_category: page.name.starts_with("Plugin: "), cat_idx: Some(idx), section_idx: None, label: page.name.clone(), @@ -579,6 +614,7 @@ fn render_categories( has_changes: false, indent_cols: 4, is_category: false, + is_plugin_category: false, cat_idx: Some(cat_idx), section_idx: Some(section_idx), label: section.name.clone(), @@ -666,7 +702,18 @@ fn render_categories( } else { spans.push(Span::styled(" ", style)); } - spans.push(Span::styled(data.label.clone(), style)); + let label = if data.is_plugin_category { + let prefix_width: usize = spans + .iter() + .map(|span| str_width(span.content.as_ref())) + .sum(); + let label_width = row_area.width as usize; + let label_width = label_width.saturating_sub(prefix_width); + truncate_display_width_with_ellipsis(&data.label, label_width) + } else { + data.label.clone() + }; + spans.push(Span::styled(label, style)); frame.render_widget(Paragraph::new(Line::from(spans)), row_area); @@ -724,20 +771,30 @@ fn render_settings_panel( theme: &Theme, layout: &mut SettingsLayout, ) { - let page = match state.current_page() { - Some(p) => p, + let (page_title, page_nullable) = match state.current_page() { + Some(p) => (p.name.clone(), p.nullable), None => return, }; - // Page description suppressed: it duplicated the category name visible - // in the sidebar and pushed the actual settings down without adding - // information. The category names + section headers carry enough - // context. let mut y = area.y; let header_start_y = y; + // Right-panel page title is the full context fallback for sidebar labels, + // which are width-clamped because plugin names are external input. + if area.height > 0 && area.width > 0 { + let title = truncate_display_width_with_ellipsis(&page_title, area.width as usize); + let title_style = Style::default() + .fg(theme.editor_fg) + .add_modifier(Modifier::BOLD); + frame.render_widget( + Paragraph::new(title).style(title_style), + Rect::new(area.x, y, area.width, 1), + ); + y += 1; + } + // "Clear" button for nullable categories (e.g., Option) - if page.nullable && state.current_category_has_values() { + if page_nullable && state.current_category_has_values() { let btn_text = format!("[{}]", t!("settings.btn_clear_category")); let btn_len = btn_text.len() as u16; let is_hovered = matches!(state.hover_hit, Some(SettingsHit::ClearCategoryButton)); @@ -798,15 +855,20 @@ fn render_settings_panel( .filter_map(|item| { // Only consider single-row controls for alignment match &item.control { - SettingControl::Toggle(s) => Some(s.label.len() as u16), - SettingControl::Number(s) => Some(s.label.len() as u16), - SettingControl::Dropdown(s) => Some(s.label.len() as u16), - SettingControl::Text(s) => Some(s.label.len() as u16), + SettingControl::Toggle(s) => Some(str_width(&s.label) as u16), + SettingControl::Number(s) => Some(str_width(&s.label) as u16), + SettingControl::Dropdown(s) => Some(str_width(&s.label) as u16), + SettingControl::Text(s) => Some(str_width(&s.label) as u16), // Multi-row controls have their labels on separate lines _ => None, } }) .max(); + let pending_dirty_by_item: Vec = page + .items + .iter() + .map(|item| state.path_has_pending_change(&item.path)) + .collect(); // Use ScrollablePanel to render items with automatic scroll handling let panel_layout = state.scroll_panel.render( @@ -823,6 +885,10 @@ fn render_settings_panel( &render_ctx, theme, max_label_width, + pending_dirty_by_item + .get(info.index) + .copied() + .unwrap_or(false), ) }, theme, @@ -910,6 +976,7 @@ fn render_setting_item_pure( ctx: &RenderContext, theme: &Theme, label_width: Option, + pending_dirty: bool, ) -> SettingItemLayoutInfo { let plan = item.layout_box(area.width, &item.style); let style = item.style; @@ -1062,7 +1129,7 @@ fn render_setting_item_pure( Rect::new(inner_area.x, inner_area.y, 1, 1), ); } - if item.modified && label_row_visible && inner_area.width >= 2 { + if pending_dirty && label_row_visible && inner_area.width >= 2 { frame.render_widget( Paragraph::new("●").style(Style::default().fg(theme.settings_selected_fg)), Rect::new(inner_area.x + 1, inner_area.y, 1, 1), @@ -1086,8 +1153,7 @@ fn render_setting_item_pure( &item.name, content_skip_top, theme, - label_width - .map(|w| w.saturating_sub(style.card_border_cols + style.focus_indicator_cols)), + label_width, item.read_only, item.is_null, ); @@ -1237,7 +1303,7 @@ fn render_control( } let colors = ToggleColors::from_theme(theme); let toggle_layout = render_toggle_aligned(frame, area, state, &colors, label_width); - ControlLayoutInfo::Toggle(toggle_layout.full_area) + ControlLayoutInfo::Toggle(toggle_layout.checkbox_area) } SettingControl::Number(state) => { @@ -4161,6 +4227,28 @@ mod tests { assert_eq!(out.chars().count(), 5); } + #[test] + fn truncate_display_width_with_ellipsis_ascii_truncates_to_width() { + let out = truncate_display_width_with_ellipsis("Plugin: very-long-plugin-name", 18); + assert_eq!(out, "Plugin: very-lo..."); + assert!(str_width(&out) <= 18); + } + + #[test] + fn truncate_display_width_with_ellipsis_handles_tiny_widths() { + assert_eq!(truncate_display_width_with_ellipsis("abcdef", 0), ""); + assert_eq!(truncate_display_width_with_ellipsis("abcdef", 1), "."); + assert_eq!(truncate_display_width_with_ellipsis("abcdef", 2), ".."); + assert_eq!(truncate_display_width_with_ellipsis("abcdef", 3), "..."); + } + + #[test] + fn truncate_display_width_with_ellipsis_multicolumn_does_not_overflow() { + let out = truncate_display_width_with_ellipsis("Plugin: 你好世界📦📦", 14); + assert!(out.ends_with("...")); + assert!(str_width(&out) <= 14, "{out:?} was too wide"); + } + // Basic compile test - actual rendering tests would need a test backend #[test] fn test_control_layout_info() { diff --git a/crates/fresh-editor/src/view/settings/state.rs b/crates/fresh-editor/src/view/settings/state.rs index 9de12020e5..79c2ef5e7d 100644 --- a/crates/fresh-editor/src/view/settings/state.rs +++ b/crates/fresh-editor/src/view/settings/state.rs @@ -364,6 +364,67 @@ impl SettingsState { ); } + fn paths_intersect(a: &str, b: &str) -> bool { + if a.is_empty() || b.is_empty() { + return a == b; + } + if a == b { + return true; + } + let a_prefix = format!("{}/", a.trim_end_matches('/')); + let b_prefix = format!("{}/", b.trim_end_matches('/')); + a.starts_with(&b_prefix) || b.starts_with(&a_prefix) + } + + /// True when this JSON pointer has an unsaved change in the current + /// Settings session. This is intentionally separate from `item.modified`, + /// which tracks whether a value is defined in the target config layer. + pub fn path_has_pending_change(&self, path: &str) -> bool { + self.pending_changes + .keys() + .any(|pending| Self::paths_intersect(path, pending)) + || self + .pending_deletions + .iter() + .any(|pending| Self::paths_intersect(path, pending)) + } + + pub fn page_has_pending_changes(&self, page_idx: usize) -> bool { + let Some(page) = self.pages.get(page_idx) else { + return false; + }; + (!page.path.is_empty() && self.path_has_pending_change(&page.path)) + || page + .items + .iter() + .any(|item| self.path_has_pending_change(&item.path)) + } + + fn schema_default_for_path(&self, path: &str) -> Option { + self.pages + .iter() + .flat_map(|page| &page.items) + .find(|item| item.path == path) + .and_then(|item| item.default.clone()) + } + + /// Return the value this setting had when Settings was opened. + /// + /// Built-in settings are usually materialized in `original_config`. + /// Plugin settings can exist only as schema defaults until the user saves + /// an override, so the schema default is part of the original effective + /// value for dirty-state comparisons. + fn effective_original_value(&self, path: &str) -> Option { + self.original_config + .pointer(path) + .cloned() + .or_else(|| self.schema_default_for_path(path)) + } + + fn value_matches_effective_original(&self, path: &str, value: &serde_json::Value) -> bool { + self.effective_original_value(path).as_ref() == Some(value) + } + /// Hide the settings panel pub fn hide(&mut self) { self.visible = false; @@ -953,9 +1014,7 @@ impl SettingsState { /// Record a pending change for a setting pub fn set_pending_change(&mut self, path: &str, value: serde_json::Value) { - // Check if this is the same as the original value - let original = self.original_config.pointer(path); - if original == Some(&value) { + if self.value_matches_effective_original(path, &value) { self.pending_changes.remove(path); } else { self.pending_changes.insert(path.to_string(), value); @@ -1119,6 +1178,25 @@ impl SettingsState { }); if let Some((path, default)) = reset_info { + let original_source = self.get_layer_source(&path); + + if original_source != self.target_layer { + // The row is only modified because of an unsaved pending edit. + // Reset should cancel that edit and return to the inherited + // resolved value, not record a no-op deletion against a layer + // that did not define the setting in the first place. + self.pending_changes.remove(&path); + self.pending_deletions.remove(&path); + let original = self.effective_original_value(&path).unwrap_or(default); + if let Some(item) = self.current_item_mut() { + update_control_from_value(&mut item.control, &original); + item.modified = false; + item.layer_source = original_source; + item.is_null = item.nullable && original.is_null(); + } + return; + } + // Mark this path for deletion from the target layer self.pending_deletions.insert(path.clone()); // Remove any pending change for this path @@ -1224,17 +1302,33 @@ impl SettingsState { }); if let Some((path, value)) = change_info { + let original_value = self.effective_original_value(&path); + let matches_original = original_value.as_ref() == Some(&value); + let original_source = self.get_layer_source(&path); + // When user changes a value, it becomes "modified" (defined in target layer) // Remove from pending deletions if it was scheduled for removal self.pending_deletions.remove(&path); + self.set_pending_change(&path, value); // Update the item's state if let Some(item) = self.current_item_mut() { - item.modified = true; // New semantic: value is now defined in target layer - item.layer_source = target_layer; // Value now comes from target layer - item.is_null = false; // Explicit value clears the inherited state + if matches_original { + item.modified = !item.is_auto_managed && original_source == target_layer; + item.layer_source = original_source; + item.is_null = item.nullable + && original_value + .as_ref() + .map(|v| v.is_null()) + .unwrap_or_else(|| { + item.default.as_ref().map(|d| d.is_null()).unwrap_or(true) + }); + } else { + item.modified = true; // New semantic: value is now defined in target layer + item.layer_source = target_layer; // Value now comes from target layer + item.is_null = false; // Explicit value clears the inherited state + } } - self.set_pending_change(&path, value); } } @@ -3117,6 +3211,28 @@ mod tests { } "#; + const TEST_SCHEMA_THEME_DEFAULT: &str = r#" +{ + "type": "object", + "properties": { + "theme": { + "type": "string", + "enum": ["dark", "light", "high-contrast"], + "default": "high-contrast" + } + }, + "$defs": {} +} +"#; + + fn open_theme_dropdown_state() -> SettingsState { + let config = test_config(); + let mut state = SettingsState::new(TEST_SCHEMA_THEME_DEFAULT, &config).unwrap(); + state.show(); + state.toggle_focus(); + state + } + #[test] fn test_dropdown_toggle() { let config = test_config(); @@ -3221,6 +3337,41 @@ mod tests { assert_eq!(after_change, after_confirm); } + #[test] + fn dropdown_reverting_to_original_value_clears_pending_and_row_modified() { + let mut state = open_theme_dropdown_state(); + + state.dropdown_select(0); // dark + assert!(state.has_changes()); + assert!(state.current_item().unwrap().modified); + + state.dropdown_select(2); // high-contrast, matching Config::default() + assert!(!state.has_changes()); + let item = state.current_item().unwrap(); + assert!(!item.modified); + assert_eq!(item.layer_source, ConfigLayer::System); + } + + #[test] + fn reset_after_unsaved_inherited_dropdown_change_cancels_pending_edit() { + let mut state = open_theme_dropdown_state(); + + state.dropdown_select(1); // light + assert!(state.has_changes()); + assert!(state.current_item().unwrap().modified); + + state.reset_current_to_default(); + assert!(!state.has_changes()); + let item = state.current_item().unwrap(); + assert!(!item.modified); + assert_eq!(item.layer_source, ConfigLayer::System); + if let SettingControl::Dropdown(dropdown) = &item.control { + assert_eq!(dropdown.selected_value(), Some("high-contrast")); + } else { + panic!("theme should render as a dropdown"); + } + } + #[test] fn test_number_editing() { let config = test_config(); diff --git a/crates/fresh-editor/tests/e2e/settings.rs b/crates/fresh-editor/tests/e2e/settings.rs index f4d630b107..02185ac9bb 100644 --- a/crates/fresh-editor/tests/e2e/settings.rs +++ b/crates/fresh-editor/tests/e2e/settings.rs @@ -1,6 +1,6 @@ //! E2E tests for the settings modal -use crate::common::harness::EditorTestHarness; +use crate::common::harness::{EditorTestHarness, HarnessOptions}; use crossterm::event::{KeyCode, KeyModifiers}; /// Test opening settings modal with Ctrl+, @@ -223,6 +223,160 @@ fn test_settings_toggle_shows_modified() { .unwrap(); } +fn find_setting_label_and_chip( + harness: &EditorTestHarness, + label: &str, +) -> Option<(u16, u16, u16)> { + let height = harness.buffer().area.height; + for y in 0..height { + let line = harness.get_row_text(y); + let chars: Vec = line.chars().collect(); + let needle: Vec = label.chars().collect(); + let Some(label_col) = (0..chars.len().saturating_sub(needle.len().saturating_sub(1))) + .find(|&i| chars[i..i + needle.len()] == needle[..]) + else { + continue; + }; + let bracket_col = chars[label_col + needle.len()..] + .iter() + .position(|&c| c == '[') + .map(|offset| label_col + needle.len() + offset)?; + return Some((label_col as u16, bracket_col as u16, y)); + } + None +} + +fn screen_contains_text_at_or_after_col( + harness: &EditorTestHarness, + text: &str, + min_col: u16, +) -> bool { + harness.screen_to_string().lines().any(|line| { + line.find(text) + .map(|col| col as u16 >= min_col) + .unwrap_or(false) + }) +} + +#[test] +fn test_settings_toggle_mouse_click_only_chip_changes_value() { + let mut harness = EditorTestHarness::new(120, 40).unwrap(); + harness.open_settings().unwrap(); + + let (label_x, chip_x, row) = find_setting_label_and_chip(&harness, "Check For Updates") + .unwrap_or_else(|| { + panic!( + "could not find Check For Updates toggle. Screen:\n{}", + harness.screen_to_string() + ) + }); + + // Clicking the label/row should only select the setting, not toggle it. + harness.mouse_click(label_x, row).unwrap(); + harness.render().unwrap(); + assert!( + !harness.screen_to_string().contains("modified"), + "clicking a toggle label should not change the setting. Screen:\n{}", + harness.screen_to_string() + ); + + // Clicking inside the rendered [v]/[ ] chip should toggle and mark dirty. + harness.mouse_click(chip_x + 1, row).unwrap(); + harness.render().unwrap(); + assert!( + harness.screen_to_string().contains("modified"), + "clicking the toggle chip should change the setting. Screen:\n{}", + harness.screen_to_string() + ); +} + +#[test] +fn test_settings_toggle_reverting_value_clears_dirty_markers() { + let mut harness = EditorTestHarness::new(120, 40).unwrap(); + harness.open_settings().unwrap(); + + let (_label_x, chip_x, row) = find_setting_label_and_chip(&harness, "Check For Updates") + .unwrap_or_else(|| { + panic!( + "could not find Check For Updates toggle. Screen:\n{}", + harness.screen_to_string() + ) + }); + + harness.mouse_click(chip_x + 1, row).unwrap(); + harness.render().unwrap(); + assert!( + harness.screen_to_string().contains("modified"), + "first toggle should mark settings dirty. Screen:\n{}", + harness.screen_to_string() + ); + + harness.mouse_click(chip_x + 1, row).unwrap(); + harness.render().unwrap(); + let screen = harness.screen_to_string(); + assert!( + !screen.contains("modified"), + "second toggle restores the original value, so the title should be clean. Screen:\n{screen}" + ); + assert!( + !screen.contains(">● Check For Updates"), + "second toggle restores the original value, so the row dirty marker should clear. Screen:\n{screen}" + ); +} + +#[test] +fn test_plugin_toggle_mouse_click_chip_matches_visual_position() { + let mut harness = + EditorTestHarness::create(120, 40, HarnessOptions::new().without_empty_plugins_dir()) + .unwrap(); + harness.open_settings().unwrap(); + + harness + .send_key(KeyCode::Char('/'), KeyModifiers::NONE) + .unwrap(); + harness.type_text("AutoOpen").unwrap(); + harness.render().unwrap(); + harness.assert_screen_contains("AutoOpen"); + harness + .send_key(KeyCode::Enter, KeyModifiers::NONE) + .unwrap(); + harness.render().unwrap(); + + assert!( + screen_contains_text_at_or_after_col(&harness, "Plugin: dashboard", 32), + "plugin settings page should show its title in the right panel. Screen:\n{}", + harness.screen_to_string() + ); + + let (_label_x, chip_x, row) = + find_setting_label_and_chip(&harness, "AutoOpen").unwrap_or_else(|| { + panic!( + "could not find dashboard AutoOpen toggle. Screen:\n{}", + harness.screen_to_string() + ) + }); + + harness.mouse_click(chip_x + 1, row).unwrap(); + harness.render().unwrap(); + assert!( + harness.screen_to_string().contains("modified"), + "clicking the plugin toggle's visible chip should change it. Screen:\n{}", + harness.screen_to_string() + ); + + harness.mouse_click(chip_x + 1, row).unwrap(); + harness.render().unwrap(); + let screen = harness.screen_to_string(); + assert!( + !screen.contains("modified"), + "clicking the plugin toggle again should restore the schema default and clear dirty markers. Screen:\n{screen}" + ); + assert!( + !screen.contains(">● AutoOpen"), + "plugin setting row dirty marker should clear after restoring the schema default. Screen:\n{screen}" + ); +} + /// Test confirmation dialog shows pending changes #[test] fn test_confirmation_dialog_shows_changes() { diff --git a/crates/fresh-editor/tests/e2e/settings_tree_view.rs b/crates/fresh-editor/tests/e2e/settings_tree_view.rs index c69c78dcc8..7dd2cb3337 100644 --- a/crates/fresh-editor/tests/e2e/settings_tree_view.rs +++ b/crates/fresh-editor/tests/e2e/settings_tree_view.rs @@ -174,14 +174,9 @@ fn tree_keyboard_nav_scrolls_body_both_directions() { harness.screen_to_string() ); - // BackTab from Settings goes directly to Categories (focus_prev wraps - // back through the panel cycle). - harness - .send_key(KeyCode::BackTab, KeyModifiers::NONE) - .unwrap(); - harness.render().unwrap(); - // Down on the tree → body should show the NEXT section (Editing). + // Section clicks keep focus in the tree, so keyboard navigation can + // continue from the clicked section without an extra focus hop. let next = &EDITOR_SECTIONS[4]; harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); harness.render().unwrap(); @@ -289,6 +284,31 @@ fn click_tree_section_jumps_body_both_directions() { ); } +#[test] +fn click_tree_section_keeps_left_tree_focus_marker() { + let mut harness = EditorTestHarness::new(120, 40).unwrap(); + open_editor_expanded(&mut harness); + + let section = &EDITOR_SECTIONS[6]; // LSP + let (col, row) = find_tree_row(&harness, section.0, 120).expect("LSP section row"); + harness.mouse_click(col, row).unwrap(); + harness.render().unwrap(); + + let (tree_x_start, _) = tree_bounds(120); + assert_eq!( + harness.get_cell(tree_x_start, row).as_deref(), + Some(">"), + "mouse-clicking a section row should keep keyboard focus in the \ + left tree, matching top-level category clicks. Screen:\n{}", + harness.screen_to_string() + ); + assert!( + body_shows_section(&harness, section), + "section click should still jump the body to the clicked section. Screen:\n{}", + harness.screen_to_string() + ); +} + /// 3a. Body keyboard scroll moves the left-panel section highlight in /// both directions. #[test] @@ -302,8 +322,12 @@ fn body_keyboard_scroll_updates_tree_highlight_both_directions() { harness.mouse_click(col, row).unwrap(); harness.render().unwrap(); + // Section clicks keep focus in the tree; move to the body before + // testing body keyboard scroll. + harness.send_key(KeyCode::Tab, KeyModifiers::NONE).unwrap(); + harness.render().unwrap(); + // Press PageDown several times in the body — content scrolls forward. - // The body panel was given focus by the click, so PageDown goes to it. for _ in 0..3 { harness .send_key(KeyCode::PageDown, KeyModifiers::NONE)