From 62dbf60eb28a5a94a41369566d600ce555c1d8e8 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Thu, 8 Oct 2026 08:56:25 +0000 Subject: [PATCH] Fix OK lost while a joint animates; Browser Joints folder inside the root component Clicking OK during the Joint dialog's motion animation was lost about half the time: the preview's busy spinner came and went in the dialog footer between frames, shifting the automatic ids of OK and Cancel, so a click pressed in one frame and released in the next landed on two different ids. The spinner's place is now always taken. Reproduced through the control channel (6 of 14 OK clicks at different animation phases lost before, 0 of 14 after) and covered by a test that presses OK with the preview busy and releases it once idle. The Joints folder is drawn by component_rows for the root, after its folders (as agreed with solvecraft-shell). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL --- crates/ui-egui/src/browser.rs | 4 +- crates/ui-egui/src/dialogs.rs | 7 +++- crates/ui-egui/src/dialogs_assembly.rs | 7 ++-- crates/ui-egui/src/dialogs_assembly_tests.rs | 40 ++++++++++++++++++++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/browser.rs b/crates/ui-egui/src/browser.rs index bb17fcb3..1566f1db 100644 --- a/crates/ui-egui/src/browser.rs +++ b/crates/ui-egui/src/browser.rs @@ -433,7 +433,6 @@ pub fn browser(app: &mut SolveApp, ui: &mut egui::Ui) { .show(ui, |ui| { ui.spacing_mut().item_spacing.y = 0.0; component_rows(app, ui, 0, 0, &mut acts); - crate::dialogs_assembly::browser_joints(app, ui); }); }); app.viewport.hover_bodies = acts.hover_bodies; @@ -535,6 +534,9 @@ fn component_rows(app: &mut SolveApp, ui: &mut egui::Ui, id: u64, depth: usize, for folder in ["bodies", "sketches", "construction"] { folder_rows(app, ui, id, folder, d, acts); } + if id == 0 { + crate::dialogs_assembly::browser_joints(app, ui, d); + } let children: Vec = app.session.doc.components.iter().filter(|c| c.parent == id).map(|c| c.id).collect(); for c in children { component_rows(app, ui, c, d, acts); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index f7838e7f..fea52c72 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1386,8 +1386,13 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { ui.painter().circle_stroke(r.center(), 7.0, Stroke::new(1.2, t.text_dim)); ui.painter().text(r.center(), egui::Align2::CENTER_CENTER, "i", egui::FontId::proportional(11.0), t.text_dim); resp.on_hover_text(heading.as_str()); + // The spinner's place is always taken: a widget that comes and goes would shift + // the automatic ids of OK and Cancel, and a click pressed in one frame and + // released in the next would be lost (an animating preview toggles it often). + let (sr, sresp) = ui.allocate_exact_size(vec2(14.0, 14.0), egui::Sense::hover()); if app.preview.busy { - ui.add(egui::Spinner::new().size(14.0)).on_hover_text("Updating the preview"); + egui::Spinner::new().size(14.0).paint_at(ui, sr); + sresp.on_hover_text("Updating the preview"); } ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { let close = if matches!(d.kind, Kind::Params { .. } | Kind::Measure { .. } | Kind::Preferences) { "Close" } else { "Cancel" }; diff --git a/crates/ui-egui/src/dialogs_assembly.rs b/crates/ui-egui/src/dialogs_assembly.rs index 05aa650d..a93f5a02 100644 --- a/crates/ui-egui/src/dialogs_assembly.rs +++ b/crates/ui-egui/src/dialogs_assembly.rs @@ -918,7 +918,7 @@ pub fn timeline_joints(app: &mut SolveApp, ui: &mut egui::Ui, p: &egui::Painter, /// The Joints folder at the end of the Browser: a row per joint (double-click edits, the menu /// edits, drives, suppresses or deletes). -pub fn browser_joints(app: &mut SolveApp, ui: &mut egui::Ui) { +pub fn browser_joints(app: &mut SolveApp, ui: &mut egui::Ui, depth: usize) { let t = Tokens::get(); let joints: Vec<(u64, String, JointKind, bool)> = app.session.doc.assembly.joints.iter().map(|j| (j.id, j.name.clone(), j.kind, j.suppressed)).collect(); @@ -954,7 +954,8 @@ pub fn browser_joints(app: &mut SolveApp, ui: &mut egui::Ui) { ); resp }; - if row(ui, 1.0, "folder", "Joints", false, Some(open)).clicked() { + let depth = depth as f32; + if row(ui, depth, "folder", "Joints", false, Some(open)).clicked() { open = !open; ui.data_mut(|d| d.insert_temp(fold_id, open)); } @@ -964,7 +965,7 @@ pub fn browser_joints(app: &mut SolveApp, ui: &mut egui::Ui) { let mut dialog: Option = None; let mut run: Option<(&'static str, Value)> = None; for (id, name, kind, suppressed) in joints { - let resp = row(ui, 2.0, "joint", &name, suppressed, None).on_hover_text(format!("{kind:?} joint")); + let resp = row(ui, depth + 1.0, "joint", &name, suppressed, None).on_hover_text(format!("{kind:?} joint")); if resp.double_clicked() { dialog = edit_joint(app, id); } diff --git a/crates/ui-egui/src/dialogs_assembly_tests.rs b/crates/ui-egui/src/dialogs_assembly_tests.rs index d0059812..451492d1 100644 --- a/crates/ui-egui/src/dialogs_assembly_tests.rs +++ b/crates/ui-egui/src/dialogs_assembly_tests.rs @@ -163,3 +163,43 @@ fn circles_from_edge_polylines() { let line: Vec = (0..10).map(|i| Vec3::new(f64::from(i), 0.0, 0.0)).collect(); assert!(circle_of(&line).is_none()); } + +/// The centre of the first text shape reading `text` in a frame's output. +fn text_at(out: &egui::FullOutput, text: &str) -> Option { + out.shapes.iter().find_map(|c| match &c.shape { + egui::Shape::Text(t) if t.galley.text() == text => Some(t.pos + t.galley.rect.center().to_vec2()), + _ => None, + }) +} + +/// OK pressed while the animated preview is computing and released once it is not (the busy +/// spinner coming and going between the two frames) still applies the joint. +#[test] +fn ok_click_survives_a_busy_preview() { + let mut app = two_boxes(); + app.start("JointAssembleCmdNew"); + let mut d = app.dialog.take().unwrap(); + d.pick(&app.session, face_sel(&app.session, Vec3::new(10.0, 10.0, 10.0)).unwrap()); + d.pick(&app.session, face_sel(&app.session, Vec3::new(40.0, 10.0, 0.0)).unwrap()); + if let Kind::Assembly(Asm::Joint(f)) = &mut d.kind { + f.kind = 1; + f.animate = true; + } + app.dialog = Some(d); + let ctx = egui::Context::default(); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1600.0, 1000.0)); + let frame = |app: &mut SolveApp, busy: bool, events: Vec| { + app.preview.busy = busy; + let input = egui::RawInput { screen_rect: Some(screen), events, ..Default::default() }; + ctx.run_ui(input, |ui| crate::dialogs::show(app, ui.ctx())) + }; + frame(&mut app, false, vec![]); + let out = frame(&mut app, false, vec![]); + let ok = text_at(&out, "OK").expect("an OK button"); + let button = |pressed| egui::Event::PointerButton { pos: ok, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; + frame(&mut app, true, vec![egui::Event::PointerMoved(ok)]); + frame(&mut app, true, vec![button(true)]); + frame(&mut app, false, vec![button(false)]); + assert_eq!(app.session.doc.assembly.joints.len(), 1, "the click applied the joint"); + assert!(app.dialog.is_none()); +}