diff --git a/crates/engine/src/cmd/edit/props.rs b/crates/engine/src/cmd/edit/props.rs index 9b6921d3f..2ec716818 100644 --- a/crates/engine/src/cmd/edit/props.rs +++ b/crates/engine/src/cmd/edit/props.rs @@ -267,9 +267,11 @@ pub(super) fn paint_list(s: &mut Session, p: &Value, fills: bool, add: bool) -> let doc = s.doc_mut()?; for i in ids { let n = doc.get_mut(i)?; - // A removal that names no paint changes nothing, the characters' own fills included. + // A removal that names no paint among the rows shown changes nothing, the characters' own + // fills included; characters coloured differently show no rows at all ("Click + to + // replace mixed content"). if !add { - let shown = if fills { crate::cmd::lists::character_fills(n).map_or(n.fills.len(), |f| f.len()) } else { n.strokes.len() }; + let shown = if fills { crate::cmd::lists::character_fills(n).map_or(0, |f| f.len()) } else { n.strokes.len() }; if index.unwrap_or(shown.saturating_sub(1)) >= shown { continue; } diff --git a/crates/engine/src/cmd/image.rs b/crates/engine/src/cmd/image.rs index 9f6b942a4..865e4d19a 100644 --- a/crates/engine/src/cmd/image.rs +++ b/crates/engine/src/cmd/image.rs @@ -184,9 +184,10 @@ fn replace(s: &mut Session, p: &Value) -> Result { Ok(json!({"image": image, "width": width, "height": height, "changed": changed})) } -/// The index of a layer's top visible image fill (fills paint bottom to top). +/// The index of a layer's top visible image fill among the fills it shows (fills paint bottom to +/// top): a text's characters' shared list, so none while they are coloured differently. pub fn top_image_fill(n: &Node) -> Option { - n.fills.iter().rposition(|f| f.visible && matches!(f.kind, PaintKind::Image(_))) + crate::cmd::lists::character_fills(n)?.iter().rposition(|f| f.visible && matches!(f.kind, PaintKind::Image(_))) } fn crop(s: &mut Session, p: &Value) -> Result { @@ -195,17 +196,17 @@ fn crop(s: &mut Session, p: &Value) -> Result { // A selection with nothing to crop is refused as unavailable, not as bad input. let refuse = |why: &str| EngineError::Disabled(cmd.into(), why.into()); let [id] = ids[..] else { return Err(refuse("select one layer to crop its image")) }; - // A text's fills are its characters' (the Design panel's rows): the crop edits that list. - if crate::cmd::lists::has_character_fills(s.doc()?.get(id)?) { - crate::cmd::lists::take_character_fills(s.doc_mut()?.get_mut(id)?); - } let doc = s.doc()?; let n = doc.get(id)?; + // A text's fills are its characters' (the Design panel's rows): the crop edits that list, and + // characters coloured differently have no row to crop ("Click + to replace mixed content"). + let shown = + crate::cmd::lists::character_fills(n).ok_or_else(|| refuse("the characters have different fills, so there is no one fill to crop"))?; let index = match get(p, "index") { None | Some(Value::Null) => top_image_fill(n).ok_or_else(|| refuse("the layer has no image fill to crop"))?, Some(_) => req_index(cmd, p, "index")?, }; - let mut paint = n.fills.get(index).cloned().ok_or_else(|| bad(cmd, "no fill at that index"))?; + let mut paint = shown.get(index).cloned().ok_or_else(|| bad(cmd, "no fill at that index"))?; let PaintKind::Image(img) = &mut paint.kind else { return Err(bad(cmd, "that fill is not an image")) }; // An image already in Crop mode only enters crop mode: no edit, no undo step. if img.scale_mode != ImageScaleMode::Crop { @@ -213,6 +214,8 @@ fn crop(s: &mut Session, p: &Value) -> Result { img.crop = Some(img.placement_crop(f64::from(data.width), f64::from(data.height), n.width, n.height)); img.scale_mode = ImageScaleMode::Crop; let n = s.doc_mut()?.get_mut(id)?; + // The characters' shared list becomes the layer's, so the crop shows on every one of them. + crate::cmd::lists::take_character_fills(n); if let Some(f) = n.fills.get_mut(index) { *f = paint; } diff --git a/crates/engine/src/text_fill_tests.rs b/crates/engine/src/text_fill_tests.rs index 3e77b47d8..348fcc9e7 100644 --- a/crates/engine/src/text_fill_tests.rs +++ b/crates/engine/src/text_fill_tests.rs @@ -69,6 +69,16 @@ fn a_layer_fill_edit_reaches_every_character() { s.execute("text.style", &json!({"id": t, "start": 1, "end": 5, "textFills": ["#FFFF00"]})).unwrap(); s.execute("fill.remove", &json!({"ids": [t], "index": 7})).unwrap(); assert_eq!(fills_shown(&s, t), ["123456", "FFFF00", "FFFF00", "FFFF00", "FFFF00"], "nothing removed, nothing taken"); + // Characters coloured differently show no fill rows at all: a removal with or without an + // index leaves the layer's hidden fill and the characters' own alone, and there is no row to + // crop (the command is unavailable, as the menu has it). + s.execute("fill.remove", &json!({"ids": [t], "index": 0})).unwrap(); + s.execute("fill.remove", &json!({"ids": [t]})).unwrap(); + assert_eq!(fills_shown(&s, t), ["123456", "FFFF00", "FFFF00", "FFFF00", "FFFF00"], "mixed: nothing to remove"); + assert_eq!(node(&s).fills.len(), 1, "the layer's own fill stays"); + assert!(matches!(s.execute("image.crop", &json!({"ids": [t]})), Err(crate::EngineError::Disabled(..))), "mixed: nothing to crop"); + assert_eq!(fills_shown(&s, t), ["123456", "FFFF00", "FFFF00", "FFFF00", "FFFF00"], "and nothing taken"); + assert_eq!(crate::cmd::image::top_image_fill(&node(&s)), None, "the menu finds no image to crop either"); s.execute("text.style", &json!({"id": t, "start": 0, "end": 5, "textFills": ["#00FF00"]})).unwrap(); s.execute("fill.copy", &json!({"id": t})).unwrap(); let copied = s.clipboard.paint.clone().and_then(|(_, p)| p.solid_color()).map(|c| c.to_hex());