From bcfb25ca93a3dd1a4916239703d7d506c3461f28 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:24:41 -0400 Subject: [PATCH 1/3] Dialogs: share the buttons-first layout Settings uses Settings keeps its buttons in view by laying them out first, at the bottom of its body, and giving its pages the height left above them, clipped to it. That layout is now dialogs::above_footer, so other dialogs can keep their buttons in view the same way. Settings looks and works as before. --- crates/ui-egui/src/panels/dialogs.rs | 16 ++++++++ crates/ui-egui/src/panels/settings.rs | 54 ++++++++++++--------------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index e95a667ec..6f735e9c1 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -192,6 +192,22 @@ pub(crate) fn modal_fit(ctx: &egui::Context, title: &str, size: egui::Vec2, t: & }); } +/// The buttons of a [`modal_fit`] body of `height`, kept in view: `footer` lays them out first, +/// at the bottom (bottom up: its first row is the lowest), and the rest of the body gets the room +/// above them. Returns that room (a left-to-right row, clipped to it, so a long or wide body +/// scrolls or clips under the buttons instead of pushing them out of the dialog) and its height. +pub(crate) fn above_footer(ui: &mut egui::Ui, height: f32, footer: impl FnOnce(&mut egui::Ui)) -> (egui::Ui, f32) { + let area = ui.allocate_space(vec2(ui.available_width(), height)).1; + let mut foot = ui.new_child(egui::UiBuilder::new().max_rect(area).layout(egui::Layout::bottom_up(egui::Align::Min))); + footer(&mut foot); + foot.add_space(8.0); + let body_h = (foot.min_rect().top() - area.top()).max(0.0); + let room = Rect::from_min_size(area.min, vec2(area.width(), body_h)); + let mut body = ui.new_child(egui::UiBuilder::new().max_rect(room).layout(egui::Layout::left_to_right(egui::Align::Min))); + body.set_clip_rect(room.intersect(body.clip_rect())); + (body, body_h) +} + pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context) { let Some(d) = app.dialog else { return }; let t = app.tokens; diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index f7a45febb..5f115b842 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -10,7 +10,7 @@ use crate::i18n::{tr, tr_args}; use effectcraft_engine::prefs::{APPEARANCE_MODES, DARK_THEMES, Item, Kind, LIGHT_THEMES, Page, pages}; use effectcraft_engine::segment::Task; -use egui::{Color32, Rect, RichText, vec2}; +use egui::{Color32, RichText, vec2}; use serde_json::{Value, json}; use crate::theme::{ThemeKind, Tokens}; @@ -633,37 +633,31 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let mut acts: Vec = vec![]; let (mut ok, mut cancel) = (false, false); super::dialogs::modal_fit(ctx, "Settings", vec2(820.0, 600.0), t, |ui, height| { - // The buttons go first, from the bottom up, and the pages get the height they leave: at - // a high UI Scale the pages scroll and the buttons stay in view. - let area = ui.allocate_space(vec2(ui.available_width(), height)).1; - let mut foot = ui.new_child(egui::UiBuilder::new().max_rect(area).layout(egui::Layout::bottom_up(egui::Align::Min))); - foot.horizontal(|ui| { - let r = ui.add_enabled(idx > 0, egui::Button::new(tr("Previous"))); - app.auto.add("settings.previous", r.rect, "Previous"); - if r.clicked() { - page = all[idx - 1].id.to_string(); - } - let r = ui.add_enabled(idx + 1 < all.len(), egui::Button::new(tr("Next"))); - app.auto.add("settings.next", r.rect, "Next"); - if r.clicked() { - page = all[idx + 1].id.to_string(); - } - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - let r = ui.add(egui::Button::new(RichText::new(tr(" OK ")).color(Color32::WHITE)).fill(t.accent)); - app.auto.add("settings.ok", r.rect, "OK"); - ok |= r.clicked(); - let r = ui.button(tr("Cancel")); - app.auto.add("settings.cancel", r.rect, "Cancel"); - cancel |= r.clicked(); + // The buttons go first and the pages get the height they leave: at a high UI Scale the + // pages scroll and the buttons stay in view. + let (mut body, body_h) = super::dialogs::above_footer(ui, height, |foot| { + foot.horizontal(|ui| { + let r = ui.add_enabled(idx > 0, egui::Button::new(tr("Previous"))); + app.auto.add("settings.previous", r.rect, "Previous"); + if r.clicked() { + page = all[idx - 1].id.to_string(); + } + let r = ui.add_enabled(idx + 1 < all.len(), egui::Button::new(tr("Next"))); + app.auto.add("settings.next", r.rect, "Next"); + if r.clicked() { + page = all[idx + 1].id.to_string(); + } + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let r = ui.add(egui::Button::new(RichText::new(tr(" OK ")).color(Color32::WHITE)).fill(t.accent)); + app.auto.add("settings.ok", r.rect, "OK"); + ok |= r.clicked(); + let r = ui.button(tr("Cancel")); + app.auto.add("settings.cancel", r.rect, "Cancel"); + cancel |= r.clicked(); + }); }); + foot.separator(); }); - foot.separator(); - foot.add_space(8.0); - let body_h = (foot.min_rect().top() - area.top()).max(0.0); - // (A child, clipped to its share, so a wide page can't widen the dialog under the buttons.) - let pages = Rect::from_min_size(area.min, vec2(area.width(), body_h)); - let mut body = ui.new_child(egui::UiBuilder::new().max_rect(pages).layout(egui::Layout::left_to_right(egui::Align::Min))); - body.set_clip_rect(pages.intersect(body.clip_rect())); { let ui = &mut body; // Page list (left, like After Effects). From bf911d18bc37e0ee1ab518e2297a704c1ba2a4a8 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:24:43 -0400 Subject: [PATCH 2/3] 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 1366c2ae8..6e9b28e49 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 aa4128cbf..6fcd4bcd0 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); From de0f5736209519ec0b43e17659a087719f24f892 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 22:42:53 -0400 Subject: [PATCH 3/3] Layer Styles: the controls scroll both ways above the buttons (#1062) Keeping OK and Cancel in view took away the outer scroll area that also scrolled the dialog sideways, and the controls only scrolled up and down. In a narrow window (960x600 at 200 % UI Scale leaves the controls about 190 points next to the 200-point list) they ran past the dialog's right edge with no way to reach them. The controls now scroll both ways in the room left under their heading, with no minimum scrolled size, so a small budget can't hide the end of them; the list scrolls in its height the same way. Each control also registers dialog.layerStyle.value.