M8: after Crop is chosen in the image fill's menu, a drag on the image crops it, and no internal gesture message shows (#2307)

With the colour picker open on an image fill in Crop mode, a press on the layer closed the picker, and closing it committed the drag as a plain move: the whole layer moved instead of the picture inside it. The picker now treats a crop drag as its own control, as it does the open gradient's handles (#1852), so the press doesn't close it and the drag moves the crop.

A drag whose gesture something else committed under the held button went on calling the engine's preview, whose "no gesture in progress" error reached the canvas as a message. That error is now its own EngineError::NoGesture, which the canvas drops: the drag it belonged to is over, and it says nothing to the user.

Tests: crop_image_tests::crop_from_the_colour_picker_then_a_drag_on_the_image_moves_the_crop_not_the_layer (failed before: the crop didn't move) and a_drag_whose_gesture_was_committed_under_the_button_shows_no_message (failed before: "no gesture in progress").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Model-Effort: Opus 5.5 drove and finished; Sonnet 5.5 wrote at medium (cut off by an API error)
This commit is contained in:
Trevor Brown
2026-11-13 05:38:36 -05:00
co-authored by Claude Opus 5.5
parent 011f08e588
commit bdf6ff9223
7 changed files with 71 additions and 4 deletions
+4
View File
@@ -14,6 +14,10 @@ pub enum EngineError {
NoNode(NodeId),
#[error("{0}")]
Other(String),
/// A preview was asked for with no gesture open (something committed it under a held button).
/// Not for the user: the canvas drops it, as the drag it belonged to is over.
#[error("no gesture in progress")]
NoGesture,
/// A command run in the background sent a request to a server and runs again with the answer
/// ([`background`]); never returned to other callers.
#[error("waiting for the server")]
+1 -1
View File
@@ -65,7 +65,7 @@ impl Session {
/// Replace the document with `f(snapshot)` for a live preview during a gesture.
pub fn preview(&mut self, f: impl FnOnce(&mut Document, &mut Vec<NodeId>) -> Result<()>) -> Result<()> {
let ds = self.ds_mut()?;
let Some(it) = ds.interaction.clone() else { return Err(EngineError::Other("no gesture in progress".into())) };
let Some(it) = ds.interaction.clone() else { return Err(EngineError::NoGesture) };
let mut doc = (*it.doc).clone();
let mut sel = it.selection.clone();
// What the gesture changes on the selected layers is an explicit edit.
+7
View File
@@ -206,6 +206,13 @@ pub fn gradient_dragging(ctx: &egui::Context) -> bool {
matches!(ctx.data(|d| d.get_temp::<Drag>(drag_id())), Some(Drag::Gradient { .. }))
}
/// Whether the open image fill's crop (its handles, or a drag on the image) is being dragged, the
/// press that starts it included: the open colour picker holds that paint in Crop mode, so the
/// press is its own control, not one outside it that would close it and commit the drag (#2307).
pub fn crop_dragging(ctx: &egui::Context) -> bool {
matches!(ctx.data(|d| d.get_temp::<Drag>(drag_id())), Some(Drag::Crop { .. }))
}
/// Whether the document has a gesture open: a handle drag edits only inside its own.
fn gesture_open(app: &FigApp) -> bool {
app.session.ds().is_ok_and(|d| d.interaction.is_some())
+5 -1
View File
@@ -398,7 +398,11 @@ fn send(app: &mut FigApp, kind: PointerKind, pos: Pos2, m: &egui::Modifiers) {
app.canvas.pressure = None;
}
app.canvas.last_pointer = Some(Point::new(ev.x, ev.y));
if let Err(e) = app.session.pointer(&ev) {
// A gesture something else committed under the held button (a colour picker closing on this
// press) leaves the rest of its drag with nothing to edit: that is not worth saying.
if let Err(e) = app.session.pointer(&ev)
&& !matches!(e, figcraft_engine::EngineError::NoGesture)
{
app.toast(e.to_string());
}
app.canvas.last_hover = hover.then(|| HoverKey::of(app, ev));
+50
View File
@@ -194,6 +194,56 @@ fn crop_chosen_in_the_colour_picker_enters_crop_mode() {
assert!((scaled[0] - after[0] * 5.0 / 6.0).abs() < 1e-6 && (scaled[3] - after[3] * 5.0 / 6.0).abs() < 1e-6, "{after:?} -> {scaled:?}");
}
/// The reporter's sequence (#2307): the colour picker open on an image fill held in Crop mode
/// (Crop chosen in its mode dropdown, or reopened on a fill that already is), then a press on the
/// layer and a drag. The drag moves the picture inside the layer (its crop), not the layer, the
/// press doesn't close the picker under it, and no internal gesture message reaches the user.
#[test]
fn crop_from_the_colour_picker_then_a_drag_on_the_image_moves_the_crop_not_the_layer() {
let (mut u, layer, _) = setup();
u.app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills: true, index: 0, ids: vec![layer] });
for _ in 0..4 {
u.frame(vec![]);
}
choose_mode(&mut u, "Fill", "Crop");
// Crop mode on the canvas ended (a click away from the image); the picker opens on the Crop fill.
u.app.ui.crop = None;
u.app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills: true, index: 0, ids: vec![layer] });
for _ in 0..4 {
u.frame(vec![]);
}
let (_, Some(before)) = mode(&u, layer) else { panic!("a crop") };
let at = u.app.session.doc().unwrap().node(layer).unwrap().transform.as_coeffs();
let from = u.app.to_screen(100.0, 50.0);
u.drag_to(from, from + vec2(30.0, 10.0));
u.frame(vec![]);
u.button(from + vec2(30.0, 10.0), false);
let n = u.app.session.doc().unwrap().node(layer).unwrap().clone();
assert_eq!(n.transform.as_coeffs(), at, "the layer stays where it is");
assert_eq!((n.width, n.height), (200.0, 100.0), "and keeps its size");
let (m, Some(after)) = mode(&u, layer) else { panic!("a crop") };
assert_eq!(m, ImageScaleMode::Crop);
assert_ne!(after, before, "the drag moved the picture inside the layer");
assert_eq!(u.app.session.selection(), vec![layer]);
assert!(u.app.ui.toast.as_ref().is_none_or(|(t, _)| !t.contains("gesture")), "no internal gesture message: {:?}", u.app.ui.toast);
}
/// A drag the colour picker's close committed under the held button (a press on a layer closing
/// the picker) leaves the rest of its drag with no gesture: that says nothing to the user.
#[test]
fn a_drag_whose_gesture_was_committed_under_the_button_shows_no_message() {
let (mut u, layer, _) = setup();
u.app.run("select.set", json!({"ids": [layer]}));
let from = u.app.to_screen(100.0, 50.0);
u.button(from, true);
u.app.session.commit_interaction().unwrap();
for k in 1..=10 {
u.frame(vec![Event::PointerMoved(from + vec2(3.0, 1.0) * k as f32)]);
}
u.button(from + vec2(30.0, 10.0), false);
assert!(u.app.ui.toast.is_none(), "no message: {:?}", u.app.ui.toast);
}
/// The first paint, an image, of `id`'s fills (`fills`) or strokes, or of region `region` of a vector.
fn image_at(u: &Ui, id: NodeId, fills: bool, region: Option<usize>) -> figcraft_color::ImagePaint {
let n = u.app.session.doc().unwrap().node(id).unwrap();
+2 -1
View File
@@ -1041,7 +1041,8 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
let picking = app.session.tool.pick_only || app.session.tool.picked.is_some();
// Nor may a press on the open gradient's handles or stops on the canvas: they are this picker's
// controls, and the canvas took the press before the picker drew (#1852).
let own = picking || crate::handles::gradient_dragging(ctx);
// Nor may one on the crop of an image paint held in Crop mode (#2307).
let own = picking || crate::handles::gradient_dragging(ctx) || crate::handles::crop_dragging(ctx);
pop = pop.close_on_outside(!own);
// Only the opening control is held out: any other sidebar or canvas press dismisses it.
let mut held: Vec<Rect> = app.ui.picker_button.into_iter().collect();
+2 -1
View File
File diff suppressed because one or more lines are too long