From faf4c0ee52d2a30b76b7da7aadd92fe366843274 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:10:49 -0500 Subject: [PATCH] fix(format): Reflection slider with only focus does not re-add the reflection A reflection slider that had focus and lost it to the None button committed its stale value. Commit on focus loss only after an edit the user made, and cover it with a kittest. --- crates/ui-egui/src/panels/format_shape.rs | 14 ++++++++-- crates/ui-egui/src/reflection_pane_tests.rs | 30 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/format_shape.rs b/crates/ui-egui/src/panels/format_shape.rs index 48d5df9d..002226d9 100644 --- a/crates/ui-egui/src/panels/format_shape.rs +++ b/crates/ui-egui/src/panels/format_shape.rs @@ -477,10 +477,20 @@ fn fx_slider( } else { ui.data_mut(|d| d.remove::(sid)); } + // A slider that only had focus when another control took it (the Reflection "None" button) + // must not commit its stale value: only an edit the user made is committed on losing focus. + let edited_id = sid.with("edited"); + if r.changed() { + ui.data_mut(|d| d.insert_temp(edited_id, true)); + } + let edited = ui.data_mut(|d| d.get_temp::(edited_id)).unwrap_or(false); // `changed()` alone also fires when the slider only rounds the shown value (55.000000000000007 to 55), // which re-added a reflection the pane had just cleared; a keyboard edit has focus. - if r.drag_stopped() || r.lost_focus() || (r.changed() && !r.dragged() && r.has_focus()) { - ui.data_mut(|d| d.remove::(sid)); + if r.drag_stopped() || (r.lost_focus() && edited) || (r.changed() && !r.dragged() && r.has_focus()) { + ui.data_mut(|d| { + d.remove::(sid); + d.remove::(edited_id); + }); run(app, "shape.effects", json!({key: {prop: v * scale}})); } }); diff --git a/crates/ui-egui/src/reflection_pane_tests.rs b/crates/ui-egui/src/reflection_pane_tests.rs index dbedd524..dda97afd 100644 --- a/crates/ui-egui/src/reflection_pane_tests.rs +++ b/crates/ui-egui/src/reflection_pane_tests.rs @@ -41,3 +41,33 @@ fn reflection_none_clears_after_a_second_preset() { h.run_steps(4); assert!(!has_reflection(&h), "None clears the reflection"); } + +#[test] +fn reflection_none_stays_cleared_when_a_slider_only_had_focus() { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + let id = s.execute("shape.insert", &json!({"preset": "rect", "rect": [72.0, 36.0, 144.0, 108.0]})).unwrap()["id"].as_u64().unwrap(); + s.execute("edit.select", &json!({"ids": [id]})).unwrap(); + let mut app = SlideApp::new(s, Services::default()); + app.ui.pane = Some("format".into()); + app.ui.format_tab = "effects".into(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + app, + ); + h.run_steps(3); + h.get_by_label("Tight").click(); + h.run_steps(4); + // Focus a reflection slider without editing it, then click "None": the slider's value must not + // be written back over the cleared reflection. + // The reflection's Size comes before the glow's (the same label). + h.get_all_by_role_and_label(egui::accesskit::Role::Slider, "Size").next().unwrap().focus(); + h.run_steps(4); + assert!(h.get_all_by_role_and_label(egui::accesskit::Role::Slider, "Size").next().unwrap().is_focused(), "the reflection size slider has focus"); + h.get_all_by_label("None").last().unwrap().click(); + h.run_steps(4); + assert!(!has_reflection(&h), "a slider that only had focus does not re-add the reflection"); +}