From 535d423d6cb003805569fc3ea8bbe65f0b8279da Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 05:41:40 -0800 Subject: [PATCH] Appearance: Add New Fill and Add New Stroke params text match the code appearance.addFill copies the active fill row only when the call has no `ids`, no `id` and no `target: "contents"`; the params text now names `id` too. appearance.addStroke gives the new stroke at least 1 pt, so a 0.5 pt top stroke gives a 1 pt stroke; the params text now says so. Tests pin both: `id` alone leaves the active row alone and puts the new fill on top, and Add New Stroke on a 0.5 pt stroke adds a 1 pt one. --- crates/engine/src/cmd/appearance.rs | 4 ++-- crates/engine/src/tests/appearance.rs | 30 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 27f5aee81..4fb14ec31 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -45,7 +45,7 @@ pub fn specs() -> Vec { "Add New Fill", ["Window", "Appearance"], Some("Cmd+/"), - "{ids?|id?, target?: \"object\"|\"contents\"} add a fill on top of each selected object's own stack, on an image right below its Contents row, with its top fill's paint (type without one: its characters' fill; else the default fill). With a fill row active (appearance.setActiveItem), no `ids` and no `target: \"contents\"`, the new fill takes that row's paint, goes right above it, and becomes the active row; appearance.duplicateItem copies a whole row", + "{ids?|id?, target?: \"object\"|\"contents\"} add a fill on top of each selected object's own stack, on an image right below its Contents row, with its top fill's paint (type without one: its characters' fill; else the default fill). With a fill row active (appearance.setActiveItem), no `ids`, no `id` and no `target: \"contents\"`, the new fill takes that row's paint, goes right above it, and becomes the active row; appearance.duplicateItem copies a whole row", has_doc, |s, p| add_item(s, p, true) ), @@ -54,7 +54,7 @@ pub fn specs() -> Vec { "Add New Stroke", ["Window", "Appearance"], Some("Cmd+Alt+/"), - "{ids?|id?, target?} add a stroke on top of each selected object's own stack, on an image right below its Contents row, with its top stroke's paint and weight (else the default stroke)", + "{ids?|id?, target?} add a stroke on top of each selected object's own stack, on an image right below its Contents row, with its top stroke's paint (else the default stroke) and that stroke's weight when it has paint, at least 1 pt", has_doc, |s, p| add_item(s, p, false) ), diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index 4fea39bdf..9bed4eb34 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -323,6 +323,36 @@ fn add_fill_copies_the_active_fill_row_right_above_it() { assert_eq!(s.appearance_item(), Some(0)); } +/// With a fill row active, Add New Fill given `id` alone also puts a copy of the top fill on top, +/// and the active row stays. +#[test] +fn add_fill_with_id_leaves_the_active_fill_row_alone() { + let mut s = session(); + let id = rect(&mut s, 100.0); + run(&mut s, "paint.setFill", json!({"color": "#ff0000"})); + run(&mut s, "appearance.addFill", json!({})); + run(&mut s, "appearance.setItem", json!({"index": 2, "color": "#0000ff"})); // [red, Stroke, blue] + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + run(&mut s, "appearance.addFill", json!({"id": id.0})); + let n = node(&s, id); + assert_eq!(n.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "stroke", "fill", "fill"]); + assert_eq!(fill_paint(&n, 0).color().unwrap().to_hex(), "#ff0000"); + assert_eq!(fill_paint(&n, 3).color().unwrap().to_hex(), "#0000ff"); + assert_eq!(s.appearance_item(), Some(0)); +} + +/// Add New Stroke copies the top stroke's weight but makes the new stroke at least 1 pt. +#[test] +fn add_stroke_is_at_least_one_point() { + let mut s = session(); + let id = rect(&mut s, 100.0); + run(&mut s, "stroke.set", json!({"weight": 0.5})); + run(&mut s, "appearance.addStroke", json!({})); // [Fill, Stroke, Stroke] + let n = node(&s, id); + assert_eq!(stroke(&n, 1).width, 0.5); + assert_eq!(stroke(&n, 2).width, 1.0); +} + /// The new fill above the active fill row takes only the row's paint: the row's opacity, blend /// mode, visibility, overprint and effects are not copied (Duplicate Item copies those). #[test]