From badbe64440f3c033d7929e63eabf54f7a0e3d51d Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown.dev@gmail.com>
Date: Mon, 2 Nov 2026 03:23:36 -0500
Subject: [PATCH] Fill section: a removal that names no paint leaves the
 characters' fills, and Copy fill and Crop image use the list the rows show

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---
 crates/engine/src/cmd/edit/props.rs  | 10 +++++++++-
 crates/engine/src/cmd/image.rs       |  4 ++++
 crates/engine/src/cmd/lists.rs       | 11 +++++++++--
 crates/engine/src/text_fill_tests.rs |  9 +++++++++
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/crates/engine/src/cmd/edit/props.rs b/crates/engine/src/cmd/edit/props.rs
index b31312a01..9b6921d3f 100644
--- a/crates/engine/src/cmd/edit/props.rs
+++ b/crates/engine/src/cmd/edit/props.rs
@@ -267,6 +267,13 @@ 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.
+        if !add {
+            let shown = if fills { crate::cmd::lists::character_fills(n).map_or(n.fills.len(), |f| f.len()) } else { n.strokes.len() };
+            if index.unwrap_or(shown.saturating_sub(1)) >= shown {
+                continue;
+            }
+        }
         if fills {
             crate::cmd::lists::take_character_fills(n);
         }
@@ -350,7 +357,8 @@ pub(super) fn paint_copy(s: &mut Session, p: &Value, fills: bool) -> Result<Valu
     let (paint, stroke) = {
         let doc = s.doc()?;
         let n = doc.get(id)?;
-        let list = if fills { &n.fills } else { &n.strokes };
+        // A text's fills are what its characters show (the list the Design panel's rows are).
+        let list = if fills { crate::cmd::lists::character_fills(n).unwrap_or_default() } else { n.strokes.clone() };
         let at = at.unwrap_or(list.len().saturating_sub(1));
         let paint = list.get(at).cloned().ok_or_else(|| bad(cmd, format!("no {} at index {at}", if fills { "fill" } else { "stroke" })))?;
         (paint, (!fills).then(|| n.stroke.clone()))
diff --git a/crates/engine/src/cmd/image.rs b/crates/engine/src/cmd/image.rs
index c2f10103f..9f6b942a4 100644
--- a/crates/engine/src/cmd/image.rs
+++ b/crates/engine/src/cmd/image.rs
@@ -195,6 +195,10 @@ fn crop(s: &mut Session, p: &Value) -> Result<Value> {
     // 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)?;
     let index = match get(p, "index") {
diff --git a/crates/engine/src/cmd/lists.rs b/crates/engine/src/cmd/lists.rs
index 8f4cb37b3..455a02567 100644
--- a/crates/engine/src/cmd/lists.rs
+++ b/crates/engine/src/cmd/lists.rs
@@ -201,14 +201,21 @@ pub fn character_fills(n: &Node) -> Option<Vec<figcraft_color::Paint>> {
     spans.all(|f| *f == first).then_some(first)
 }
 
+/// Whether any character of a text carries fills of its own (on its base style or a run).
+pub(crate) fn has_character_fills(n: &Node) -> bool {
+    match &n.kind {
+        NodeKind::Text(t) => t.style.fills.is_some() || t.runs.iter().any(|r| r.style.fills.is_some()),
+        _ => false,
+    }
+}
+
 /// Before a layer-level edit of a text's fills (the Fill section, `fill.*`, `node.set`): the
 /// characters' own fills go, so the edit shows on every one of them. One list they all shared
 /// becomes the layer's first, so the edit starts from what the characters showed; characters
 /// that differed fall back to the layer's fills, which the edit then replaces ("Click + to
 /// replace mixed content"). Not a text, or no character with fills of its own: nothing changes.
 pub(crate) fn take_character_fills(n: &mut Node) {
-    let NodeKind::Text(t) = &n.kind else { return };
-    if t.style.fills.is_none() && t.runs.iter().all(|r| r.style.fills.is_none()) {
+    if !has_character_fills(n) {
         return;
     }
     if let Some(shared) = character_fills(n)
diff --git a/crates/engine/src/text_fill_tests.rs b/crates/engine/src/text_fill_tests.rs
index 6ef49b9e3..3e77b47d8 100644
--- a/crates/engine/src/text_fill_tests.rs
+++ b/crates/engine/src/text_fill_tests.rs
@@ -64,6 +64,15 @@ fn a_layer_fill_edit_reaches_every_character() {
     s.execute("text.style", &json!({"id": t, "start": 1, "end": 5, "textFills": ["#FFFF00"]})).unwrap();
     s.execute("node.set", &json!({"id": t, "props": {"fill": "#123456"}})).unwrap();
     assert_eq!(fills_shown(&s, t), ["123456"; 5]);
+    // A removal that names no paint leaves the characters' own fills alone; a copy takes the fill
+    // the characters show, the list the Design panel's rows are.
+    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");
+    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());
+    assert_eq!(copied.as_deref(), Some("00FF00"), "the characters' fill is copied, not the layer's older one");
     // A rectangle is left alone by all of this.
     let r = id_of(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"]).unwrap();
     s.execute("fill.set", &json!({"ids": [r], "color": "#ABCDEF"})).unwrap();