fix(plotstyle): validate and stage table edits

Validate plot-style color, lineweight, and screening fields atomically and report invalid input without changing the table. Keep successful Save and Save As edits as the close baseline; discard edits made after that baseline when the panel closes. CTB files are app-level state, so these changes do not dirty the drawing or create drawing undo entries.

test: invalid screening retains prior value and reports error

test: invalid color retains prior value and reports error

test: invalid lineweight retains prior value and reports error

test: closing panel discards unsaved edits

test: closing panel keeps values from saved baseline
This commit is contained in:
Karim Jerbi committed 2026-10-18 09:08:08 +01:00
1 parent 8db77b2398
commit 248c9aec7b
3 files changed
+200 -38

No files matched your search

+4
View File
@@ -1364,6 +1364,9 @@ pub(super) struct OpenCADStudio {
// ── Plot Style Table ────────────────────────────────────────────────── // ── Plot Style Table ──────────────────────────────────────────────────
/// Currently loaded CTB/STB table (None = no override). /// Currently loaded CTB/STB table (None = no override).
active_plot_style: Option<crate::io::plot_style::PlotStyleTable>, active_plot_style: Option<crate::io::plot_style::PlotStyleTable>,
/// Table state at Plot Style editor open / last save. Edits preview live,
/// but closing the editor restores this baseline until Save succeeds.
pub(super) plotstyle_panel_baseline: Option<Option<crate::io::plot_style::PlotStyleTable>>,
// ── MLineStyle Dialog ───────────────────────────────────────────────── // ── MLineStyle Dialog ─────────────────────────────────────────────────
mlstyle_selected: String, mlstyle_selected: String,
@@ -5107,6 +5110,7 @@ impl OpenCADStudio {
) )
}) })
.ok(), .ok(),
plotstyle_panel_baseline: None,
// Color scheme (default: Oxocarbon) // Color scheme (default: Oxocarbon)
active_theme: Theme::Oxocarbon, active_theme: Theme::Oxocarbon,
ui_theme: config::UiThemeConfig::default(), ui_theme: config::UiThemeConfig::default(),
+186 -38
View File
@@ -7469,46 +7469,85 @@ impl OpenCADStudio {
}) })
} }
pub(super) fn commit_plotstyle_panel_baseline(&mut self) {
if self.plotstyle_panel_baseline.is_some() {
self.plotstyle_panel_baseline = Some(self.active_plot_style.clone());
}
}
pub(super) fn on_plot_style_panel_apply(&mut self) -> Task<Message> { pub(super) fn on_plot_style_panel_apply(&mut self) -> Task<Message> {
let aci = self.plotstyle_panel_aci as usize; let color_text = self.ps_color_buf.trim();
if let Some(table) = self.active_plot_style.as_mut() { let color = if color_text.is_empty() {
if let Some(entry) = table.aci_entries.get_mut(aci) { None
// Parse color.
let color_str = self.ps_color_buf.trim();
if color_str.is_empty() {
entry.color = None;
} else if color_str.starts_with('#') && color_str.len() == 7 {
let r = u8::from_str_radix(&color_str[1..3], 16).unwrap_or(0);
let g = u8::from_str_radix(&color_str[3..5], 16).unwrap_or(0);
let b = u8::from_str_radix(&color_str[5..7], 16).unwrap_or(0);
entry.color = Some([r, g, b]);
}
if let Ok(lw) = self.ps_lineweight_buf.trim().parse::<u8>() {
entry.lineweight = lw;
}
if let Ok(sc) = self.ps_screening_buf.trim().parse::<u8>() {
entry.screening = sc.min(100);
}
}
} else { } else {
// No table loaded: create an identity table and apply. let Some(hex) = color_text.strip_prefix('#').filter(|hex| hex.len() == 6) else {
let mut table = crate::io::plot_style::PlotStyleTable::identity("Custom.ctb"); self.command_line.push_error(
if let Some(entry) = table.aci_entries.get_mut(aci) { crate::t!("Enter a plot color as #RRGGBB, or clear it to use object color.")
let color_str = self.ps_color_buf.trim(); .as_ref(),
if color_str.starts_with('#') && color_str.len() == 7 { );
let r = u8::from_str_radix(&color_str[1..3], 16).unwrap_or(0); return Task::none();
let g = u8::from_str_radix(&color_str[3..5], 16).unwrap_or(0); };
let b = u8::from_str_radix(&color_str[5..7], 16).unwrap_or(0); let Ok(rgb) = u32::from_str_radix(hex, 16) else {
entry.color = Some([r, g, b]); self.command_line.push_error(
} crate::t!("Enter a plot color as #RRGGBB, or clear it to use object color.")
if let Ok(lw) = self.ps_lineweight_buf.trim().parse::<u8>() { .as_ref(),
entry.lineweight = lw; );
} return Task::none();
if let Ok(sc) = self.ps_screening_buf.trim().parse::<u8>() { };
entry.screening = sc.min(100); Some([
} ((rgb >> 16) & 0xFF) as u8,
} ((rgb >> 8) & 0xFF) as u8,
self.active_plot_style = Some(table); (rgb & 0xFF) as u8,
])
};
let Ok(lineweight) = self.ps_lineweight_buf.trim().parse::<u8>() else {
self.command_line
.push_error(crate::t!("Choose a valid plot lineweight.").as_ref());
return Task::none();
};
if lineweight != 255 && lineweight as usize >= crate::io::plot_style::LW_TABLE.len() {
self.command_line
.push_error(crate::t!("Choose a valid plot lineweight.").as_ref());
return Task::none();
}
let Ok(screening) = self.ps_screening_buf.trim().parse::<u8>() else {
self.command_line.push_error(
crate::t!("Screening must be a whole number from 0 to 100.").as_ref(),
);
return Task::none();
};
if screening > 100 {
self.command_line.push_error(
crate::t!("Screening must be a whole number from 0 to 100.").as_ref(),
);
return Task::none();
}
let aci = self.plotstyle_panel_aci as usize;
if aci == 0 {
self.command_line
.push_error(crate::t!("Select a plot style color index from 1 to 255.").as_ref());
return Task::none();
}
let created = self.active_plot_style.is_none();
if created {
self.active_plot_style = Some(crate::io::plot_style::PlotStyleTable::identity(
"Custom.ctb",
));
}
if let Some(entry) = self
.active_plot_style
.as_mut()
.and_then(|table| table.aci_entries.get_mut(aci))
{
entry.color = color;
entry.lineweight = lineweight;
entry.screening = screening;
}
if created {
self.command_line self.command_line
.push_output(crate::tf!("Created new CTB table, ACI {aci} updated.").as_ref()); .push_output(crate::tf!("Created new CTB table, ACI {aci} updated.").as_ref());
} }
@@ -9084,6 +9123,115 @@ mod background_task_tests {
} }
} }
#[cfg(test)]
mod plot_style_panel_tests {
use crate::app::{Message, OpenCADStudio};
fn app_with_test_table() -> OpenCADStudio {
let mut app = OpenCADStudio::new_for_test();
let mut table = crate::io::plot_style::PlotStyleTable::identity("Test.ctb");
table.aci_entries[1].screening = 65;
app.plot_dialog.style_name = table.name.clone();
app.active_plot_style = Some(table);
app
}
#[test]
fn invalid_screening_keeps_last_valid_value_and_reports_error() {
let mut app = app_with_test_table();
let old_error_revision = app.command_line.error_revision;
let _ = app.update(Message::PlotStylePanelScreenBuf("200".into()));
assert_eq!(
app.active_plot_style.as_ref().unwrap().aci_entries[1].screening,
65,
"invalid input must not overwrite the last valid screening"
);
assert!(
app.command_line.error_revision > old_error_revision,
"invalid input should report feedback on the command line"
);
}
#[test]
fn invalid_color_is_reported_without_changing_the_entry() {
let mut app = app_with_test_table();
let entry = &mut app.active_plot_style.as_mut().unwrap().aci_entries[1];
entry.color = Some([12, 34, 56]);
entry.lineweight = 4;
app.ps_lineweight_buf = "5".into();
app.ps_screening_buf = "70".into();
let old_error_revision = app.command_line.error_revision;
let _ = app.update(Message::PlotStylePanelColorBuf("#fff".into()));
let entry = &app.active_plot_style.as_ref().unwrap().aci_entries[1];
assert_eq!(entry.color, Some([12, 34, 56]));
assert_eq!(entry.lineweight, 4);
assert_eq!(entry.screening, 65);
assert!(app.command_line.error_revision > old_error_revision);
}
#[test]
fn invalid_lineweight_is_reported_without_changing_the_entry() {
let mut app = app_with_test_table();
let entry = &mut app.active_plot_style.as_mut().unwrap().aci_entries[1];
entry.color = Some([12, 34, 56]);
entry.lineweight = 4;
app.ps_color_buf = "#010203".into();
app.ps_lineweight_buf = "200".into();
app.ps_screening_buf = "70".into();
let old_error_revision = app.command_line.error_revision;
let _ = app.update(Message::PlotStylePanelLwBuf("200".into()));
let entry = &app.active_plot_style.as_ref().unwrap().aci_entries[1];
assert_eq!(entry.color, Some([12, 34, 56]));
assert_eq!(entry.lineweight, 4);
assert_eq!(entry.screening, 65);
assert!(app.command_line.error_revision > old_error_revision);
}
#[test]
fn closing_plot_style_panel_restores_unsaved_table_edits() {
let mut app = app_with_test_table();
let _ = app.update(Message::PlotStylePanelOpen);
let _ = app.update(Message::PlotStylePanelScreenBuf("72".into()));
assert_eq!(
app.active_plot_style.as_ref().unwrap().aci_entries[1].screening,
72
);
app.close_active_modal();
assert_eq!(
app.active_plot_style.as_ref().unwrap().aci_entries[1].screening,
65,
"closing without saving should restore the table from editor open"
);
}
#[test]
fn closing_after_save_keeps_saved_table_and_discards_later_edits() {
let mut app = app_with_test_table();
let _ = app.update(Message::PlotStylePanelOpen);
let _ = app.update(Message::PlotStylePanelScreenBuf("72".into()));
app.commit_plotstyle_panel_baseline();
let _ = app.update(Message::PlotStylePanelScreenBuf("80".into()));
app.close_active_modal();
assert_eq!(
app.active_plot_style.as_ref().unwrap().aci_entries[1].screening,
72,
"closing should keep the last successfully saved table state"
);
}
}
impl OpenCADStudio { impl OpenCADStudio {
/// Web: one warning lists every unresolved xref and the way to fix it — /// Web: one warning lists every unresolved xref and the way to fix it —
/// dropping the files on the page registers their bytes, and reopening /// dropping the files on the page registers their bytes, and reopening
+10
View File
@@ -159,6 +159,9 @@ impl OpenCADStudio {
} }
} }
if self.active_modal == Some(Plotstyle) { if self.active_modal == Some(Plotstyle) {
if let Some(baseline) = self.plotstyle_panel_baseline.take() {
self.active_plot_style = baseline;
}
if let Some((plot_offset, plot_resize)) = self.plotstyle_parent_plot_geometry.take() { if let Some((plot_offset, plot_resize)) = self.plotstyle_parent_plot_geometry.take() {
self.active_modal = Some(Plot); self.active_modal = Some(Plot);
@@ -11245,6 +11248,9 @@ impl OpenCADStudio {
.as_ref(), .as_ref(),
); );
self.active_plot_style = Some(table); self.active_plot_style = Some(table);
if self.active_modal == Some(super::ModalKind::Plotstyle) {
self.plotstyle_panel_baseline = Some(self.active_plot_style.clone());
}
self.plot_dialog.plot_styles = crate::io::plot_style::available_ctb_names(); self.plot_dialog.plot_styles = crate::io::plot_style::available_ctb_names();
Task::none() Task::none()
} }
@@ -11261,6 +11267,7 @@ impl OpenCADStudio {
self.active_plot_style = None; self.active_plot_style = None;
self.plot_dialog.style_name.clear(); self.plot_dialog.style_name.clear();
self.plot_dialog.style_missing = false; self.plot_dialog.style_missing = false;
self.commit_plotstyle_panel_baseline();
self.command_line self.command_line
.push_output(crate::t!("Plot style table cleared.").as_ref()); .push_output(crate::t!("Plot style table cleared.").as_ref());
Task::none() Task::none()
@@ -11329,6 +11336,7 @@ impl OpenCADStudio {
} }
self.active_modal = Some(super::ModalKind::Plotstyle); self.active_modal = Some(super::ModalKind::Plotstyle);
self.plotstyle_panel_baseline = Some(self.active_plot_style.clone());
Task::none() Task::none()
} }
Message::PlotStylePanelClose => { Message::PlotStylePanelClose => {
@@ -11404,6 +11412,7 @@ impl OpenCADStudio {
self.tabs[self.active_tab] self.tabs[self.active_tab]
.scene .scene
.invalidate_display_plot_style(); .invalidate_display_plot_style();
self.commit_plotstyle_panel_baseline();
self.command_line.push_output( self.command_line.push_output(
crate::tf!("Plot style table saved to \"{}\".", path.display()) crate::tf!("Plot style table saved to \"{}\".", path.display())
@@ -11455,6 +11464,7 @@ impl OpenCADStudio {
self.tabs[self.active_tab] self.tabs[self.active_tab]
.scene .scene
.invalidate_display_plot_style(); .invalidate_display_plot_style();
self.commit_plotstyle_panel_baseline();
self.command_line.push_output( self.command_line.push_output(
crate::tf!("Plot style table saved to \"{}\".", path.display()) crate::tf!("Plot style table saved to \"{}\".", path.display())
.as_ref(), .as_ref(),