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);