From b525fde8d1aed637c4594c3af48b15c22271dedc Mon Sep 17 00:00:00 2001 From: krinry <121806532+krinry@users.noreply.github.com> Date: Fri, 16 Oct 2026 06:45:40 +0530 Subject: [PATCH 1/2] M1: pop up colour picker when adding a fill or stroke (#404) Clicking the + button to add a fill or stroke in the Design panel now immediately opens the colour picker dialog for the newly added paint, matching Figma's behaviour. --- contributors/people.toml | 4 +++ crates/ui-egui/src/panel_remove_tests.rs | 38 ++++++++++++++++++++++++ crates/ui-egui/src/right.rs | 9 +++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/contributors/people.toml b/contributors/people.toml index bc8cf71b..29068180 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -44,3 +44,7 @@ sync_github_name = true [people.volkovskey] real_name = "Andrii Volkov" + +[people.krinry] +display_name = "krinry" +sync_github_name = true diff --git a/crates/ui-egui/src/panel_remove_tests.rs b/crates/ui-egui/src/panel_remove_tests.rs index 299a4b6f..ee289bb8 100644 --- a/crates/ui-egui/src/panel_remove_tests.rs +++ b/crates/ui-egui/src/panel_remove_tests.rs @@ -123,3 +123,41 @@ fn removing_effects_and_grids_keeps_open_settings_with_their_rows() { assert_eq!(u.app.session.doc().unwrap().node(f).unwrap().grids.len(), 1); assert!(flag(&u, "grid_open", 0) && !flag(&u, "grid_open", 1)); } + +#[test] +fn adding_a_fill_or_stroke_opens_the_color_picker() { + let mut s = Session::new(); + let r = id(s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 50, "height": 50})).unwrap()); + s.execute("select.set", &json!({"ids": [r]})).unwrap(); + let mut u = Ui::new(s); + u.ctx.enable_accesskit(); + assert_eq!(u.app.ui.picker, None); + + let out = u.frame_output(vec![]); + let add_fill = out + .platform_output + .accesskit_update + .unwrap() + .nodes + .into_iter() + .find(|(_, n)| n.label() == Some("Add fill")) + .and_then(|(_, n)| n.bounds()) + .map(|b| Pos2::new(((b.x0 + b.x1) / 2.0) as f32, ((b.y0 + b.y1) / 2.0) as f32)) + .expect("Add fill button"); + click(&mut u, add_fill); + assert_eq!(u.app.ui.picker, Some(PickerTarget::Paint { fills: true, index: 1, ids: vec![r] })); + + let out = u.frame_output(vec![]); + let add_stroke = out + .platform_output + .accesskit_update + .unwrap() + .nodes + .into_iter() + .find(|(_, n)| n.label() == Some("Add stroke")) + .and_then(|(_, n)| n.bounds()) + .map(|b| Pos2::new(((b.x0 + b.x1) / 2.0) as f32, ((b.y0 + b.y1) / 2.0) as f32)) + .expect("Add stroke button"); + click(&mut u, add_stroke); + assert_eq!(u.app.ui.picker, Some(PickerTarget::Paint { fills: false, index: 0, ids: vec![r] })); +} diff --git a/crates/ui-egui/src/right.rs b/crates/ui-egui/src/right.rs index 10847896..5899a949 100644 --- a/crates/ui-egui/src/right.rs +++ b/crates/ui-egui/src/right.rs @@ -1101,7 +1101,14 @@ fn paints(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId], fills: empty, ) { if i == 1 { - app.run(if fills { "fill.add" } else { "stroke.add" }, json!({"ids": ids})); + if app.run(if fills { "fill.add" } else { "stroke.add" }, json!({"ids": ids})).is_some() { + if let Ok(doc) = app.session.doc() { + let count = ids.first().and_then(|id| doc.node(*id)).map_or(0, |n| if fills { n.fills.len() } else { n.strokes.len() }); + if count > 0 { + app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills, index: count - 1, ids: ids.clone() }); + } + } + } } else { style_picker::open(app, ui, &ids, slot); } From 32266dcd93aae516fa04381913a0e69e79bf8f7a Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 17 Oct 2026 15:49:01 +0700 Subject: [PATCH 2/2] #624: open the picker only for a shared paint list (mixed-selection guard) --- crates/ui-egui/src/panel_remove_tests.rs | 54 +++++++++++++----------- crates/ui-egui/src/right.rs | 12 +++--- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/crates/ui-egui/src/panel_remove_tests.rs b/crates/ui-egui/src/panel_remove_tests.rs index ee289bb8..3805dca3 100644 --- a/crates/ui-egui/src/panel_remove_tests.rs +++ b/crates/ui-egui/src/panel_remove_tests.rs @@ -1,5 +1,6 @@ -//! Removing fill, effect and layout grid rows in the Design panel, driven through egui headless: an -//! open colour picker and open settings stay with their items, and go when their item goes. +//! Adding and removing fill, effect and layout grid rows in the Design panel, driven through egui headless: +//! adding a paint opens its colour picker; an open colour picker and open settings stay with their items, +//! and go when their item goes. use egui::{Event, Id, Pos2}; use figcraft_doc::NodeId; @@ -124,6 +125,14 @@ fn removing_effects_and_grids_keeps_open_settings_with_their_rows() { assert!(flag(&u, "grid_open", 0) && !flag(&u, "grid_open", 1)); } +/// The centre of the button labelled `label` (found in the AccessKit tree). +fn button(u: &mut Ui, label: &str) -> Pos2 { + let out = u.frame_output(vec![]); + let b = out.platform_output.accesskit_update.unwrap().nodes.into_iter().find(|(_, n)| n.label() == Some(label)).and_then(|(_, n)| n.bounds()); + let b = b.unwrap_or_else(|| panic!("no {label:?} button")); + Pos2::new(((b.x0 + b.x1) / 2.0) as f32, ((b.y0 + b.y1) / 2.0) as f32) +} + #[test] fn adding_a_fill_or_stroke_opens_the_color_picker() { let mut s = Session::new(); @@ -133,31 +142,28 @@ fn adding_a_fill_or_stroke_opens_the_color_picker() { u.ctx.enable_accesskit(); assert_eq!(u.app.ui.picker, None); - let out = u.frame_output(vec![]); - let add_fill = out - .platform_output - .accesskit_update - .unwrap() - .nodes - .into_iter() - .find(|(_, n)| n.label() == Some("Add fill")) - .and_then(|(_, n)| n.bounds()) - .map(|b| Pos2::new(((b.x0 + b.x1) / 2.0) as f32, ((b.y0 + b.y1) / 2.0) as f32)) - .expect("Add fill button"); + let add_fill = button(&mut u, "Add fill"); click(&mut u, add_fill); assert_eq!(u.app.ui.picker, Some(PickerTarget::Paint { fills: true, index: 1, ids: vec![r] })); - let out = u.frame_output(vec![]); - let add_stroke = out - .platform_output - .accesskit_update - .unwrap() - .nodes - .into_iter() - .find(|(_, n)| n.label() == Some("Add stroke")) - .and_then(|(_, n)| n.bounds()) - .map(|b| Pos2::new(((b.x0 + b.x1) / 2.0) as f32, ((b.y0 + b.y1) / 2.0) as f32)) - .expect("Add stroke button"); + let add_stroke = button(&mut u, "Add stroke"); click(&mut u, add_stroke); assert_eq!(u.app.ui.picker, Some(PickerTarget::Paint { fills: false, index: 0, ids: vec![r] })); } + +#[test] +fn adding_a_fill_to_mixed_fills_opens_no_picker() { + let mut s = Session::new(); + let a = id(s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 50, "height": 50})).unwrap()); + let b = id(s.execute("shape.rectangle", &json!({"x": 60, "y": 0, "width": 50, "height": 50})).unwrap()); + s.execute("fill.add", &json!({"ids": [b]})).unwrap(); + s.execute("select.set", &json!({"ids": [a, b]})).unwrap(); + let mut u = Ui::new(s); + u.ctx.enable_accesskit(); + + let add_fill = button(&mut u, "Add fill"); + click(&mut u, add_fill); + let doc = u.app.session.doc().unwrap(); + assert_eq!((doc.node(a).unwrap().fills.len(), doc.node(b).unwrap().fills.len()), (2, 3)); + assert_eq!(u.app.ui.picker, None); +} diff --git a/crates/ui-egui/src/right.rs b/crates/ui-egui/src/right.rs index 5899a949..fcb13724 100644 --- a/crates/ui-egui/src/right.rs +++ b/crates/ui-egui/src/right.rs @@ -1101,13 +1101,11 @@ fn paints(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId], fills: empty, ) { if i == 1 { - if app.run(if fills { "fill.add" } else { "stroke.add" }, json!({"ids": ids})).is_some() { - if let Ok(doc) = app.session.doc() { - let count = ids.first().and_then(|id| doc.node(*id)).map_or(0, |n| if fills { n.fills.len() } else { n.strokes.len() }); - if count > 0 { - app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills, index: count - 1, ids: ids.clone() }); - } - } + // The new paint goes on top; open its picker. A mixed selection has no one index to edit. + if app.run(if fills { "fill.add" } else { "stroke.add" }, json!({"ids": ids})).is_some() + && let Some(l) = &list + { + app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills, index: l.len(), ids: ids.clone() }); } } else { style_picker::open(app, ui, &ids, slot);