From bf911d18bc37e0ee1ab518e2297a704c1ba2a4a8 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:24:43 -0400 Subject: [PATCH] Layer Styles: OK and Cancel stay in view (#1062) The Layer Style dialog's OK and Cancel came after the style list and the style's controls in one scroll area, and at its usual size they were laid out past the dialog's bottom edge: clipped, and a click on them did nothing until the dialog was scrolled. The dialog now uses above_footer: OK and Cancel sit at its bottom right and the list and the controls scroll above them, also at 200 % UI Scale. Test: layer_style_buttons_stay_in_view (both buttons painted whole and clicked, at 100 % and 200 %; fails before: Cancel isn't painted). --- .../ui-egui/src/panels/layer_styles_dialog.rs | 91 ++++++++++--------- crates/ui-egui/tests/ui_tail.rs | 40 ++++++++ 2 files changed, 89 insertions(+), 42 deletions(-) diff --git a/crates/ui-egui/src/panels/layer_styles_dialog.rs b/crates/ui-egui/src/panels/layer_styles_dialog.rs index 1366c2ae..6e9b28e4 100644 --- a/crates/ui-egui/src/panels/layer_styles_dialog.rs +++ b/crates/ui-egui/src/panels/layer_styles_dialog.rs @@ -8,7 +8,7 @@ use crate::i18n::tr; use effectcraft_engine::keyframe::Value as KV; use effectcraft_engine::project::styles::{BLENDING, STYLES}; use effectcraft_engine::project::{Layer, LayerId, ParamUi, Property}; -use egui::{Align2, vec2}; +use egui::vec2; use serde_json::{Value, json}; use crate::theme::Tokens; @@ -138,36 +138,57 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let lt = l.layer_time(time); let mut actions: Vec<(String, Value)> = Vec::new(); let mut close: Option = None; - super::dialogs::modal(ctx, &format!("Layer Style — {}", l.name), vec2(720.0, 520.0), t, |ui| { - ui.horizontal_top(|ui| { + super::dialogs::modal_fit(ctx, &format!("Layer Style — {}", l.name), vec2(720.0, 520.0), t, |ui, height| { + // OK and Cancel stay at the bottom, in view; the list and the style's controls scroll + // above them (#1062). + let (mut body, body_h) = super::dialogs::above_footer(ui, height, |foot| { + foot.horizontal(|ui| { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let ok = ui.button(tr("OK")); + app.auto.add("dialog.layerStyle.ok", ok.rect, "OK"); + if ok.clicked() { + close = Some(true); + } + let cancel = ui.button(tr("Cancel")); + app.auto.add("dialog.layerStyle.cancel", cancel.rect, "Cancel"); + if cancel.clicked() { + close = Some(false); + } + }); + }); + }); + { + let ui = &mut body; // Style list. ui.vertical(|ui| { ui.set_width(200.0); - let mut entries = vec![(BLENDING, "Blending Options")]; - entries.extend(STYLES.iter().copied()); - for (id, name) in entries { - ui.horizontal(|ui| { - if id != BLENDING { - let g = styles.as_ref().and_then(|s| s.sub(id)); - let mut on = g.is_some_and(|g| g.enabled); - if ui.checkbox(&mut on, "").changed() { - if g.is_none() { - actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": id}))); - } else { - actions.push(("layer.style.toggle".into(), json!({"layer": lid, "style": id, "value": on}))); + egui::ScrollArea::vertical().id_salt("ls-list").max_height(body_h).show(ui, |ui| { + let mut entries = vec![(BLENDING, "Blending Options")]; + entries.extend(STYLES.iter().copied()); + for (id, name) in entries { + ui.horizontal(|ui| { + if id != BLENDING { + let g = styles.as_ref().and_then(|s| s.sub(id)); + let mut on = g.is_some_and(|g| g.enabled); + if ui.checkbox(&mut on, "").changed() { + if g.is_none() { + actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": id}))); + } else { + actions.push(("layer.style.toggle".into(), json!({"layer": lid, "style": id, "value": on}))); + } } + } else { + ui.add_space(24.0); } - } else { - ui.add_space(24.0); - } - let r = ui.selectable_label(sel == id, name); - if r.clicked() { - app.dialog_state.layer_style.selected = id.to_string(); - } - let rect = r.rect; - app.auto.add(&format!("dialog.layerStyle.list.{id}"), rect, name); - }); - } + let r = ui.selectable_label(sel == id, name); + if r.clicked() { + app.dialog_state.layer_style.selected = id.to_string(); + } + let rect = r.rect; + app.auto.add(&format!("dialog.layerStyle.list.{id}"), rect, name); + }); + } + }); }); ui.separator(); // Controls of the selected style. @@ -180,7 +201,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.label(egui::RichText::new(tr("Not applied: tick it in the list to add it.")).weak()); return; }; - egui::ScrollArea::vertical().max_height(400.0).show(ui, |ui| { + egui::ScrollArea::vertical().max_height((body_h - 30.0).max(0.0)).show(ui, |ui| { egui::Grid::new("ls-grid").num_columns(2).spacing(vec2(12.0, 6.0)).show(ui, |ui| { g.walk("", &mut |_, p| { if matches!(p.ui, ParamUi::Hidden) { @@ -201,21 +222,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); }); }); - }); - ui.add_space(8.0); - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - let ok = ui.button(tr("OK")); - app.auto.add("dialog.layerStyle.ok", ok.rect, "OK"); - if ok.clicked() { - close = Some(true); - } - let cancel = ui.button(tr("Cancel")); - app.auto.add("dialog.layerStyle.cancel", cancel.rect, "Cancel"); - if cancel.clicked() { - close = Some(false); - } - let _ = Align2::LEFT_CENTER; - }); + } }); for (id, p) in actions { if let Err(e) = app.session.execute(&id, p) { diff --git a/crates/ui-egui/tests/ui_tail.rs b/crates/ui-egui/tests/ui_tail.rs index aa4128cb..6fcd4bcd 100644 --- a/crates/ui-egui/tests/ui_tail.rs +++ b/crates/ui-egui/tests/ui_tail.rs @@ -60,6 +60,16 @@ fn timeline_outline_columns_scroll_horizontally() { assert!(h.state().auto.find("timeline.outlineScroll").is_none()); } +fn click(h: &mut Harness<'_, EffectcraftApp>, at: Pos2) { + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerMoved(at)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + h.step(); + } + h.run_steps(2); +} + fn styles(h: &mut Harness<'_, EffectcraftApp>) -> Vec { let v = h.state_mut().session.execute("layer.style.list", json!({})).unwrap(); v["styles"].as_array().unwrap().iter().map(|s| s["style"].as_str().unwrap().to_string()).collect() @@ -103,6 +113,36 @@ fn layer_style_dialog_adds_previews_and_cancels() { assert_eq!(styles(&mut h), ["stroke"]); } +/// #1062: the Layer Style dialog's OK and Cancel stay in view, below the list and the style's +/// controls (they were laid out past its bottom edge, under the fold of its scroll area, so a +/// click on them did nothing until the dialog was scrolled), also at 200 % UI Scale. +#[test] +fn layer_style_buttons_stay_in_view() { + let mut h = harness(1400.0); + let lid = h.state().session.active_comp().unwrap().layers[0].id.0; + h.state_mut().session.execute("layer.select", json!({"layers": [lid]})).unwrap(); + let ctx = h.ctx.clone(); + // Whole: inside the window, and painted with none of it clipped away. + let in_view = |h: &Harness<'_, EffectcraftApp>, id: &str| { + let r = rect(h, id); + let clip = + h.output().shapes.iter().find(|c| matches!(&c.shape, egui::Shape::Rect(s) if (s.rect.center() - r.center()).length() < 0.5)).map(|c| c.clip_rect); + assert!(h.ctx.content_rect().contains_rect(r) && clip.is_some_and(|c| c.contains_rect(r)), "{id} is cut off: {r:?} in {clip:?}"); + r.center() + }; + for (scale, button, kept) in [(100, "dialog.layerStyle.ok", 1), (200, "dialog.layerStyle.cancel", 1)] { + h.state_mut().set_pref("appearance.uiScale", json!(scale)).unwrap(); + h.run_steps(3); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.style.dropShadow", json!({})).unwrap(); + h.run_steps(5); + in_view(&h, "dialog.layerStyle.cancel"); + let at = in_view(&h, button); + click(&mut h, at); + assert_eq!(h.state().dialog, None, "{scale} %: {button} closed the dialog"); + assert_eq!(styles(&mut h).len(), kept, "{scale} %: {button}"); + } +} + #[test] fn project_panel_columns_scroll_horizontally() { let mut h = harness(1400.0);