From 6fa0a76c4b68606b6c92b3828269f1db3f2fcb9d Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sat, 7 Nov 2026 19:15:56 -0600 Subject: [PATCH 1/7] Painting tools: Shift+Alt+click adds a colour sampler (#11890) With a tool that has the Alt eyedropper (Brush, Pencil, Gradient, Paint Bucket, Background Eraser), Shift+Alt+click adds a colour sampler at the clicked pixel through view.colorSamplers.add, as the Color Sampler tool places one: the foreground colour stays and nothing is painted. A point refused at the limit reports in the status bar as that tool's does. Alt+click without Shift still sets the foreground. --- .../src/brush_shift_alt_sampler_tests.rs | 99 +++++++++++++++++++ crates/ui-egui/src/canvas/cursors.rs | 15 +++ 2 files changed, 114 insertions(+) create mode 100644 crates/ui-egui/src/brush_shift_alt_sampler_tests.rs diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs new file mode 100644 index 0000000000..bed8198531 --- /dev/null +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -0,0 +1,99 @@ +//! #11890: Shift+Alt+click with a painting tool that has the Alt eyedropper adds a colour sampler +//! at the clicked pixel, as the Color Sampler tool places one: the foreground colour stays and +//! nothing is painted. Alt+click without Shift still sets the foreground; plain clicks paint. + +use egui::Modifiers; +use serde_json::{Value, json}; + +use crate::PhotocraftApp; +use crate::canvas::{ToolEvent, tool_event}; +use crate::state::Tool; + +const SHIFT_ALT: Modifiers = Modifiers { alt: true, ctrl: false, shift: true, mac_cmd: false, command: false }; + +/// A 400 × 300 document, a red layer to sample, blue foreground. +fn app(tool: Tool) -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 400, "height": 300})).unwrap(); + app.run("edit.fill", json!({"color": "#ff0000"})).unwrap(); + app.run("tools.setColors", json!({"foreground": "#0000ff"})).unwrap(); + app.ui.tool = tool; + app.sync_views(); + app +} + +fn click(app: &mut PhotocraftApp, x: f64, y: f64, mods: Modifiers) { + tool_event(app, ToolEvent::Down { x, y, pressure: 1.0 }, mods); + // A click's own jitter: a move or two on the spot. + tool_event(app, ToolEvent::Move { x, y, pressure: 1.0 }, mods); + tool_event(app, ToolEvent::Up { x, y }, mods); +} + +fn samplers(app: &mut PhotocraftApp) -> Vec { + let r = app.run("view.colorSamplers.list", json!({})).unwrap(); + r.get("samplers").and_then(Value::as_array).cloned().unwrap_or_default() +} + +fn foreground(app: &PhotocraftApp) -> [f32; 4] { + app.session.tools.foreground +} + +fn steps(app: &PhotocraftApp) -> usize { + app.session.active().unwrap().history.past_len() +} + +#[test] +fn shift_alt_click_adds_a_sampler_with_every_alt_eyedropper_tool() { + for tool in [Tool::Brush, Tool::Pencil, Tool::Gradient, Tool::PaintBucket, Tool::BackgroundEraser] { + let mut app = app(tool); + let fg = foreground(&app); + let before = steps(&app); + click(&mut app, 199.7, 120.2, SHIFT_ALT); + let got = samplers(&mut app); + // The point the Color Sampler tool's command places for the same click. + let mut twin = self::app(tool); + twin.run("view.colorSamplers.add", json!({"x": 199.7, "y": 120.2})).unwrap(); + assert_eq!(got, samplers(&mut twin), "{tool:?}: one sampler at the clicked pixel"); + assert_eq!(got.len(), 1, "{tool:?}"); + assert_eq!(foreground(&app), fg, "{tool:?}: the foreground stays"); + assert_eq!(steps(&app), before, "{tool:?}: nothing painted"); + assert!(!app.alt_sampling, "{tool:?}: the gesture ended"); + } +} + +#[test] +fn at_the_limit_the_click_adds_nothing_and_says_so() { + let mut app = app(Tool::Brush); + for i in 0..10 { + app.run("view.colorSamplers.add", json!({"x": 10 + i * 20, "y": 10})).unwrap(); + } + let fg = foreground(&app); + click(&mut app, 200.0, 150.0, SHIFT_ALT); + assert_eq!(samplers(&mut app).len(), 10); + assert!(app.ui.status.contains("limit"), "the Color Sampler tool's limit message: {:?}", app.ui.status); + assert!(!app.ui.status_error, "reported as the Color Sampler tool does"); + assert_eq!(foreground(&app), fg); +} + +#[test] +fn alt_click_still_samples_and_a_plain_click_still_paints() { + let mut app = app(Tool::Brush); + click(&mut app, 50.0, 50.0, Modifiers::ALT); + assert_eq!(foreground(&app)[..3], [1.0, 0.0, 0.0], "Alt+click picks the red under the pointer"); + assert!(samplers(&mut app).is_empty()); + let before = steps(&app); + click(&mut app, 80.0, 80.0, Modifiers::NONE); + assert_eq!(steps(&app), before + 1, "a plain click paints a dab"); + assert!(samplers(&mut app).is_empty()); +} + +#[test] +fn a_shift_alt_click_off_the_canvas_adds_nothing_and_never_panics() { + let mut app = app(Tool::Brush); + let fg = foreground(&app); + for (x, y) in [(-20.0, 50.0), (450.0, 50.0), (f64::NAN, 10.0), (10.0, f64::INFINITY)] { + click(&mut app, x, y, SHIFT_ALT); + } + assert!(samplers(&mut app).is_empty()); + assert_eq!(foreground(&app), fg); +} diff --git a/crates/ui-egui/src/canvas/cursors.rs b/crates/ui-egui/src/canvas/cursors.rs index e4f0336b1d..e695561fbc 100644 --- a/crates/ui-egui/src/canvas/cursors.rs +++ b/crates/ui-egui/src/canvas/cursors.rs @@ -196,6 +196,17 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: return false; } match ev { + // ⇧⌥-click adds a colour sampler at the clicked pixel instead, as the Color Sampler tool + // places one: no foreground change, nothing painted (#11890). A refused point (the + // document's limit) reports in the status bar as that tool's does. + ToolEvent::Down { x, y, .. } if mods.shift => { + if let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { + app.ui.status = e; + app.ui.status_error = false; + } + } + // While ⇧⌥ stay held after that press, moves sample nothing. + ToolEvent::Move { .. } if mods.shift => {} // Without ⌥: the sample sets the foreground colour, whatever the Eyedropper would do. ToolEvent::Down { x, y, .. } | ToolEvent::Move { x, y, .. } => sample_eyedropper(app, x, y, egui::Modifiers::NONE), ToolEvent::Up { .. } => app.alt_sampling = false, @@ -203,6 +214,10 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: true } +#[cfg(test)] +#[path = "../brush_shift_alt_sampler_tests.rs"] +mod brush_shift_alt_sampler_tests; + /// Sample the image or targeted raster mask at (x, y) using the configured Sample Size. /// Write a valid sample to the foreground swatch, or to the background swatch with Alt. pub(super) fn sample_eyedropper(app: &mut PhotocraftApp, x: f64, y: f64, mods: egui::Modifiers) { From 43589073d8aa918ece141fe1affe29a3e0efd114 Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sat, 7 Nov 2026 21:57:49 -0600 Subject: [PATCH 2/7] Shift+Alt+click on an existing colour sampler samples the foreground as before (CodeRabbit review on crates/ui-egui/src/canvas/cursors.rs:203) A press on an existing point added a second point on top of it (or reported the limit). #11890 didn't measure that case, so it keeps today's Alt eyedropper sample. The hit test uses the Color Sampler tool's reach. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/brush_shift_alt_sampler_tests.rs | 11 +++++++++++ crates/ui-egui/src/canvas/cursors.rs | 12 ++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs index bed8198531..783459b1bc 100644 --- a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -97,3 +97,14 @@ fn a_shift_alt_click_off_the_canvas_adds_nothing_and_never_panics() { assert!(samplers(&mut app).is_empty()); assert_eq!(foreground(&app), fg); } + +#[test] +fn shift_alt_click_on_a_sampler_samples_the_foreground_as_before() { + // A press on an existing point wasn't measured, so it keeps the Alt eyedropper's sample. + let mut app = app(Tool::Brush); + app.run("view.colorSamplers.add", json!({"x": 120, "y": 80})).unwrap(); + let before = samplers(&mut app); + click(&mut app, 121.0, 80.5, SHIFT_ALT); + assert_eq!(samplers(&mut app), before, "no second point on the first"); + assert_eq!(foreground(&app)[..3], [1.0, 0.0, 0.0], "the red under the pointer, as an Alt+click"); +} diff --git a/crates/ui-egui/src/canvas/cursors.rs b/crates/ui-egui/src/canvas/cursors.rs index e695561fbc..7df53c01a9 100644 --- a/crates/ui-egui/src/canvas/cursors.rs +++ b/crates/ui-egui/src/canvas/cursors.rs @@ -198,8 +198,9 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: match ev { // ⇧⌥-click adds a colour sampler at the clicked pixel instead, as the Color Sampler tool // places one: no foreground change, nothing painted (#11890). A refused point (the - // document's limit) reports in the status bar as that tool's does. - ToolEvent::Down { x, y, .. } if mods.shift => { + // document's limit) reports in the status bar as that tool's does. A press on an existing + // point wasn't measured, so it samples the foreground as before. + ToolEvent::Down { x, y, .. } if mods.shift && !on_sampler(app, x, y) => { if let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { app.ui.status = e; app.ui.status_error = false; @@ -214,6 +215,13 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: true } +/// Is document point (x, y) on a colour sampler, within the Color Sampler tool's reach of one (7 +/// screen pixels)? +fn on_sampler(app: &PhotocraftApp, x: f64, y: f64) -> bool { + let tol = 7.0 / f64::from(app.point_zoom().max(0.01)); + app.session.active().is_some_and(|d| d.color_samplers.iter().any(|q| (q[0] - x).hypot(q[1] - y) <= tol)) +} + #[cfg(test)] #[path = "../brush_shift_alt_sampler_tests.rs"] mod brush_shift_alt_sampler_tests; From 2f546f9f65a4d378e18f4bfa7c188b637e35367f Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sat, 7 Nov 2026 21:58:50 -0600 Subject: [PATCH 3/7] Test that a refused Shift+Alt+click at the sampler limit paints nothing (CodeRabbit review on crates/ui-egui/src/brush_shift_alt_sampler_tests.rs:75) Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/brush_shift_alt_sampler_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs index 783459b1bc..f4aea13e10 100644 --- a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -67,9 +67,10 @@ fn at_the_limit_the_click_adds_nothing_and_says_so() { for i in 0..10 { app.run("view.colorSamplers.add", json!({"x": 10 + i * 20, "y": 10})).unwrap(); } - let fg = foreground(&app); + let (fg, before) = (foreground(&app), steps(&app)); click(&mut app, 200.0, 150.0, SHIFT_ALT); assert_eq!(samplers(&mut app).len(), 10); + assert_eq!(steps(&app), before, "nothing painted"); assert!(app.ui.status.contains("limit"), "the Color Sampler tool's limit message: {:?}", app.ui.status); assert!(!app.ui.status_error, "reported as the Color Sampler tool does"); assert_eq!(foreground(&app), fg); From 990776c1596ce4b28342dfc49710831291c11b44 Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sat, 7 Nov 2026 23:40:13 -0600 Subject: [PATCH 4/7] A Shift+Alt press off the canvas places no sampler and says nothing (CodeRabbit review on cursors.rs:203) Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/brush_shift_alt_sampler_tests.rs | 11 +++++++++++ crates/ui-egui/src/canvas/cursors.rs | 9 ++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs index f4aea13e10..5f4d5db751 100644 --- a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -109,3 +109,14 @@ fn shift_alt_click_on_a_sampler_samples_the_foreground_as_before() { assert_eq!(samplers(&mut app), before, "no second point on the first"); assert_eq!(foreground(&app)[..3], [1.0, 0.0, 0.0], "the red under the pointer, as an Alt+click"); } + +#[test] +fn a_shift_alt_click_off_the_canvas_reports_nothing() { + // The Color Sampler tool places no point off the canvas and says nothing; so does this press. + let mut app = app(Tool::Brush); + app.ui.status = "Ready".into(); + for (x, y) in [(-20.0, 50.0), (450.0, 50.0), (f64::NAN, 10.0)] { + click(&mut app, x, y, SHIFT_ALT); + assert_eq!(app.ui.status, "Ready", "({x}, {y}): the status bar is left as it was"); + } +} diff --git a/crates/ui-egui/src/canvas/cursors.rs b/crates/ui-egui/src/canvas/cursors.rs index 7df53c01a9..e03c4bf487 100644 --- a/crates/ui-egui/src/canvas/cursors.rs +++ b/crates/ui-egui/src/canvas/cursors.rs @@ -198,10 +198,13 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: match ev { // ⇧⌥-click adds a colour sampler at the clicked pixel instead, as the Color Sampler tool // places one: no foreground change, nothing painted (#11890). A refused point (the - // document's limit) reports in the status bar as that tool's does. A press on an existing - // point wasn't measured, so it samples the foreground as before. + // document's limit) reports in the status bar as that tool's does; off the canvas, as that + // tool, it places none and says nothing. A press on an existing point wasn't measured, so + // it samples the foreground as before. ToolEvent::Down { x, y, .. } if mods.shift && !on_sampler(app, x, y) => { - if let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { + let size = app.session.active().map(|d| d.doc.size); + let inside = size.is_some_and(|s| (0.0..f64::from(s.width)).contains(&x) && (0.0..f64::from(s.height)).contains(&y)); + if inside && let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { app.ui.status = e; app.ui.status_error = false; } From 93a5bab92265e73dde821009bc939d0ee079feb2 Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sun, 8 Nov 2026 00:39:16 -0600 Subject: [PATCH 5/7] A Shift+Alt sampler refusal other than the limit keeps its error styling (CodeRabbit review on cursors.rs:207) Co-Authored-By: Claude Opus 5.5 --- .../src/brush_shift_alt_sampler_tests.rs | 23 +++++++++++++++++++ crates/ui-egui/src/canvas/cursors.rs | 5 +++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs index 5f4d5db751..21dbe605d4 100644 --- a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -120,3 +120,26 @@ fn a_shift_alt_click_off_the_canvas_reports_nothing() { assert_eq!(app.ui.status, "Ready", "({x}, {y}): the status bar is left as it was"); } } + +#[test] +fn a_shift_alt_click_refused_for_another_reason_keeps_the_error_styling() { + // Only the limit is the Color Sampler tool's plain report. A Pen draft whose commit failed, + // left in another document, refuses every command until it is dealt with: that stays an error. + let mut app = app(Tool::Pen); + for x in [30.0, 1e40] { + tool_event(&mut app, ToolEvent::Down { x, y: 60.0, pressure: 1.0 }, Modifiers::NONE); + tool_event(&mut app, ToolEvent::Up { x, y: 60.0 }, Modifiers::NONE); + } + crate::vector_ui::pen_commit(&mut app, false); + assert!(app.ui.pen.as_ref().is_some_and(|p| p.commit_failed), "a failed Pen draft"); + app.session.execute("file.new", json!({"width": 400, "height": 300})).unwrap(); + app.ui.tool = Tool::Brush; + click(&mut app, 200.0, 150.0, SHIFT_ALT); + assert!(samplers_of(&app).is_empty(), "nothing added"); + assert!(!app.ui.status.is_empty(), "the refusal is reported"); + assert!(app.ui.status_error, "and keeps its error styling: {:?}", app.ui.status); +} + +fn samplers_of(app: &PhotocraftApp) -> Vec<[f64; 2]> { + app.session.active().map(|d| d.color_samplers.clone()).unwrap_or_default() +} diff --git a/crates/ui-egui/src/canvas/cursors.rs b/crates/ui-egui/src/canvas/cursors.rs index e03c4bf487..2a22f09667 100644 --- a/crates/ui-egui/src/canvas/cursors.rs +++ b/crates/ui-egui/src/canvas/cursors.rs @@ -205,8 +205,11 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: let size = app.session.active().map(|d| d.doc.size); let inside = size.is_some_and(|s| (0.0..f64::from(s.width)).contains(&x) && (0.0..f64::from(s.height)).contains(&y)); if inside && let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { + // Only the limit is a plain report; any other refusal (a failed Pen draft left in + // another document) stays an error. + let full = app.session.active().is_some_and(|d| d.color_samplers.len() >= photocraft_engine::sampler_cmds::MAX_SAMPLERS); app.ui.status = e; - app.ui.status_error = false; + app.ui.status_error = !full; } } // While ⇧⌥ stay held after that press, moves sample nothing. From b42472d936f28ea3a128a8bad4c267a41fb4e99f Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sun, 8 Nov 2026 01:32:10 -0600 Subject: [PATCH 6/7] Shift+Alt sampler test: reset the error styling before the refused click (CodeRabbit review on crates/ui-egui/src/brush_shift_alt_sampler_tests.rs:137) The failed Pen draft had already set status_error, so the assertion held even if the click left the status alone. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/brush_shift_alt_sampler_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs index 21dbe605d4..9544628cab 100644 --- a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -134,6 +134,8 @@ fn a_shift_alt_click_refused_for_another_reason_keeps_the_error_styling() { assert!(app.ui.pen.as_ref().is_some_and(|p| p.commit_failed), "a failed Pen draft"); app.session.execute("file.new", json!({"width": 400, "height": 300})).unwrap(); app.ui.tool = Tool::Brush; + // The Pen failure styled the status bar as an error already: only the click may set it again. + app.ui.status_error = false; click(&mut app, 200.0, 150.0, SHIFT_ALT); assert!(samplers_of(&app).is_empty(), "nothing added"); assert!(!app.ui.status.is_empty(), "the refusal is reported"); From bc669007efe006974cc5268285cb4c45081c5ef5 Mon Sep 17 00:00:00 2001 From: David Mann Date: Wed, 11 Nov 2026 02:08:56 +0100 Subject: [PATCH 7/7] PR #12245: keep the Pen refusal an error when the document is full Shift+Alt+click with ten samplers and a failed Pen draft in another document showed the Pen's refusal as the plain sampler-limit report, because the limit was read from the sampler count. It is now read from the refusal itself: view.colorSamplers.add's limit message comes from sampler_cmds::limit_message(), and only that message is a plain report. Co-Authored-By: hectorviov <1501713+hectorviov@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/sampler_cmds.rs | 8 ++++++- .../src/brush_shift_alt_sampler_tests.rs | 23 +++++++++++++++++++ crates/ui-egui/src/canvas/cursors.rs | 6 ++--- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/sampler_cmds.rs b/crates/engine/src/cmd/sampler_cmds.rs index d2c291910a..e5300fef6d 100644 --- a/crates/engine/src/cmd/sampler_cmds.rs +++ b/crates/engine/src/cmd/sampler_cmds.rs @@ -16,6 +16,12 @@ use crate::{DocState, EngineError, Result, Session}; /// Photoshop's Color Sampler limit: at most 10 points per document. pub const MAX_SAMPLERS: usize = 10; +/// The refusal `view.colorSamplers.add` gives at [`MAX_SAMPLERS`], so a caller can tell it from +/// other failures. +pub fn limit_message() -> String { + format!("Color Sampler limit reached: at most {MAX_SAMPLERS} points per document") +} + fn bad(cmd: &str, msg: impl Into) -> EngineError { EngineError::BadParams { cmd: cmd.into(), msg: msg.into() } } @@ -97,7 +103,7 @@ fn add(s: &mut Session, p: &Value) -> Result { change(s, |st| { let pos = pixel(&st.doc, x, y).ok_or_else(|| EngineError::Other("the sample point is outside the canvas".into()))?; if st.color_samplers.len() >= MAX_SAMPLERS { - return Err(EngineError::Other(format!("Color Sampler limit reached: at most {MAX_SAMPLERS} points per document"))); + return Err(EngineError::Other(limit_message())); } st.color_samplers.push(pos); let i = st.color_samplers.len() - 1; diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs index 9544628cab..6af1042626 100644 --- a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -142,6 +142,29 @@ fn a_shift_alt_click_refused_for_another_reason_keeps_the_error_styling() { assert!(app.ui.status_error, "and keeps its error styling: {:?}", app.ui.status); } +#[test] +fn a_full_document_refused_for_another_reason_keeps_the_error_styling() { + // Ten points and a failed Pen draft in another document: the refusal is the Pen's, not the + // limit's, so it stays an error. + let mut app = app(Tool::Pen); + for x in [30.0, 1e40] { + tool_event(&mut app, ToolEvent::Down { x, y: 60.0, pressure: 1.0 }, Modifiers::NONE); + tool_event(&mut app, ToolEvent::Up { x, y: 60.0 }, Modifiers::NONE); + } + crate::vector_ui::pen_commit(&mut app, false); + assert!(app.ui.pen.as_ref().is_some_and(|p| p.commit_failed), "a failed Pen draft"); + app.session.execute("file.new", json!({"width": 400, "height": 300})).unwrap(); + for i in 0..10 { + app.session.execute("view.colorSamplers.add", json!({"x": 10 + i * 20, "y": 10})).unwrap(); + } + app.ui.tool = Tool::Brush; + app.ui.status_error = false; + click(&mut app, 200.0, 150.0, SHIFT_ALT); + assert_eq!(samplers_of(&app).len(), 10, "nothing added"); + assert!(!app.ui.status.contains("limit"), "the Pen's refusal, not the limit: {:?}", app.ui.status); + assert!(app.ui.status_error, "and keeps its error styling: {:?}", app.ui.status); +} + fn samplers_of(app: &PhotocraftApp) -> Vec<[f64; 2]> { app.session.active().map(|d| d.color_samplers.clone()).unwrap_or_default() } diff --git a/crates/ui-egui/src/canvas/cursors.rs b/crates/ui-egui/src/canvas/cursors.rs index 83f4cdbfef..5c10cfd699 100644 --- a/crates/ui-egui/src/canvas/cursors.rs +++ b/crates/ui-egui/src/canvas/cursors.rs @@ -222,10 +222,10 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: let inside = size.is_some_and(|s| (0.0..f64::from(s.width)).contains(&x) && (0.0..f64::from(s.height)).contains(&y)); if inside && let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { // Only the limit is a plain report; any other refusal (a failed Pen draft left in - // another document) stays an error. - let full = app.session.active().is_some_and(|d| d.color_samplers.len() >= photocraft_engine::sampler_cmds::MAX_SAMPLERS); + // another document) stays an error, also when the document is full. + let limit = e == photocraft_engine::sampler_cmds::limit_message(); app.ui.status = e; - app.ui.status_error = !full; + app.ui.status_error = !limit; } } // While ⇧⌥ stay held after that press, moves sample nothing.