diff --git a/crates/engine/src/selection/selection_colors.rs b/crates/engine/src/selection/selection_colors.rs index b653f333b..54b9703b8 100644 --- a/crates/engine/src/selection/selection_colors.rs +++ b/crates/engine/src/selection/selection_colors.rs @@ -134,7 +134,7 @@ pub fn specs() -> Vec { spec!( "selection.createColorStyle", "Create selection color style", - "{targets, name, description?} → {id}; makes a color style of the paint the targets share and applies it to them as selection.applyColorStyle does; a name another style has is refused", + "{targets, name, description?} → {id}; makes a color style of the paint the targets share and applies it to them as selection.applyColorStyle does; a name another style has, or a target whose list holds other paints, is refused", true, create_style ), @@ -252,6 +252,9 @@ fn create_style(s: &mut Session, p: &Value) -> Result { if targets.iter().any(|t| t.paint(doc).as_ref() != Some(&paint)) { return Err(bad(CMD, "the targets no longer share one paint")); } + if let Some(why) = create_refusal(doc, &targets) { + return Err(bad(CMD, why)); + } let name = crate::cmd::variables::clean_name(&req_string(CMD, p, "name")?).ok_or_else(|| bad(CMD, "name can't be empty"))?; if doc.styles.iter().any(|st| st.name == name) { return Err(bad(CMD, format!("a style named `{name}` already exists"))); @@ -270,6 +273,16 @@ fn create_style(s: &mut Session, p: &Value) -> Result { Ok(json!({"id": id})) } +/// Why a color style can't be made from `targets` (`selection.createColorStyle`), if it can't: a +/// linked style replaces the whole fill or stroke list, so a target whose list holds other paints +/// would lose them. The Selection colors menu leaves Create style out for such rows. +pub fn create_refusal(doc: &Document, targets: &[PaintTarget]) -> Option<&'static str> { + targets + .iter() + .any(|t| t.paints(doc).is_none_or(|paints| paints.len() != 1)) + .then_some("a style can only be made from a fill or stroke list that holds this one paint") +} + /// Apply paint style `style` to every target: a fill or stroke list is linked to it; a text range /// takes its resolved paints and keeps its typography. fn link_style(s: &mut Session, targets: &[PaintTarget], style: &Style) -> Result<()> { diff --git a/crates/engine/tests/it/selection_colors.rs b/crates/engine/tests/it/selection_colors.rs index 41204e5db..7fdbfb635 100644 --- a/crates/engine/tests/it/selection_colors.rs +++ b/crates/engine/tests/it/selection_colors.rs @@ -2,7 +2,7 @@ use figcraft_color::Paint; use figcraft_doc::NodeId; use figcraft_engine::{ Session, - selection_colors::{SelectionColor, collect}, + selection_colors::{self, SelectionColor, collect}, }; use serde_json::{Value, json}; @@ -505,6 +505,30 @@ fn creating_a_style_refuses_taken_or_empty_names_and_stale_rows_and_changes_noth assert_eq!(fill(&s, a), "FF0000"); } +#[test] +fn creating_a_style_refuses_a_row_whose_list_holds_other_paints() { + let mut s = Session::new(); + let a = rectangle(&mut s, "#ff0000"); + let b = rectangle(&mut s, "#ff0000"); + run(&mut s, "fill.add", json!({"id":a,"paint":"#0000ff"})); + let red = group(&s, &[a, b], "FF0000"); + let before = s.undo_depth(); + let styles = s.doc().unwrap().styles.len(); + let err = s.execute("selection.createColorStyle", &json!({"targets":red.targets,"name":"Red"})).unwrap_err(); + assert!(err.to_string().contains("holds this one paint"), "{err}"); + assert!(selection_colors::create_refusal(s.doc().unwrap(), &red.targets).is_some()); + assert_eq!(s.undo_depth(), before); + let doc = s.doc().unwrap(); + assert_eq!(doc.styles.len(), styles); + assert_eq!(doc.node(a).unwrap().fills.len(), 2, "the layer keeps its other fill"); + assert!(doc.node(b).unwrap().styles.fill.is_none()); + // The single-paint layer alone still makes a style. + let b_only: Vec<_> = red.targets.iter().filter(|t| t.id == b).cloned().collect(); + assert!(selection_colors::create_refusal(s.doc().unwrap(), &b_only).is_none()); + run(&mut s, "selection.createColorStyle", json!({"targets":b_only,"name":"Red"})); + assert!(s.doc().unwrap().node(b).unwrap().styles.fill.is_some()); +} + #[test] fn a_linked_row_switches_to_another_style() { let mut s = Session::new(); diff --git a/crates/ui-egui/src/panels/selection_colors.rs b/crates/ui-egui/src/panels/selection_colors.rs index 7a8189ddb..4d5682e35 100644 --- a/crates/ui-egui/src/panels/selection_colors.rs +++ b/crates/ui-egui/src/panels/selection_colors.rs @@ -7,7 +7,7 @@ use egui::{Id, Ui}; use figcraft_color::{Color, Paint}; use figcraft_doc::{Document, NodeId, PublishedLibrary, Style, StyleKind, VariableType}; use figcraft_engine::library; -use figcraft_engine::selection_colors::{PaintTarget, SelectionColor, bind_refusal, collect, in_scope}; +use figcraft_engine::selection_colors::{PaintTarget, SelectionColor, bind_refusal, collect, create_refusal, in_scope}; use serde_json::json; use crate::a11y::Tip; @@ -122,7 +122,7 @@ pub fn section(app: &mut FigApp, ui: &mut Ui, doc: &Arc, ids: &[NodeId style_items(app, ui, doc, group, &t); if linked_name.is_none() { ui.separator(); - if crate::right::menu_item(ui, tr("Create style"), "") { + if create_refusal(doc, &group.targets).is_none() && crate::right::menu_item(ui, tr("Create style"), "") { open_create_form(app, ui, doc, group); ui.close(); } @@ -536,6 +536,17 @@ mod tests { assert!(p.frame(vec![]).iter().any(|(s, _)| s == "Brand")); } + #[test] + fn create_style_is_left_out_for_a_row_whose_list_holds_other_paints() { + let (mut p, a, _) = Panel::new(); + p.app.run("fill.add", json!({"id":a,"paint":"#00ff00"})); + let red = text_at(&p.frame(vec![]), "FF0000"); + p.click(Pos2::new(228.0, red.y)); + let texts = p.frame(vec![]); + assert!(texts.iter().any(|(s, _)| s == "No color styles"), "the Styles menu opens"); + assert!(!texts.iter().any(|(s, _)| s == "Create style")); + } + #[test] fn a_linked_rows_styles_menu_switches_its_style() { let (mut p, a, _) = Panel::new();