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.
This commit is contained in:
Harrison Booth
2026-10-27 19:10:49 -05:00
parent 952e4db95f
commit faf4c0ee52
2 changed files with 42 additions and 2 deletions
+12 -2
View File
@@ -477,10 +477,20 @@ fn fx_slider(
} else {
ui.data_mut(|d| d.remove::<f64>(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::<bool>(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::<f64>(sid));
if r.drag_stopped() || (r.lost_focus() && edited) || (r.changed() && !r.dragged() && r.has_focus()) {
ui.data_mut(|d| {
d.remove::<f64>(sid);
d.remove::<bool>(edited_id);
});
run(app, "shape.effects", json!({key: {prop: v * scale}}));
}
});
@@ -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");
}