From cab353eb5b85176cbca99599f7290fbecbce3163 Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:40:51 +0100 Subject: [PATCH 1/2] UI: Composition Settings widens to fit a longer translation (#2841) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dialog was a fixed 600 points wide inside a scroll area, so a row wider than that in another language (French "Conserver la fréquence d'images en cas d'imbrication ou dans la file d'attente de rendu", the French and Spanish Lock Aspect Ratio row) pushed OK past the right edge, reachable only by scrolling sideways. The dialog now measures the widest row of its three tabs (as it already measured their heights) and takes that width, or 600 points if that is more, capped by the window as before. English keeps 600 points. --- crates/ui-egui/src/panels/comp_settings.rs | 34 +++++++++++++++------- crates/ui-egui/src/panels/dialogs.rs | 4 ++- crates/ui-egui/tests/it/ui_comp_presets.rs | 33 +++++++++++++++++++++ 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/panels/comp_settings.rs b/crates/ui-egui/src/panels/comp_settings.rs index 7e49d8a7d..26d3c3a69 100644 --- a/crates/ui-egui/src/panels/comp_settings.rs +++ b/crates/ui-egui/src/panels/comp_settings.rs @@ -469,13 +469,24 @@ fn tab_body( } } -/// The height `tab` would take at `top`, from laying it out unseen: it follows the font, the -/// language and the scale. Nothing it registers or changes is kept. -fn tab_height(ui: &mut egui::Ui, tab: Tab, d: &CompDraft, t: &Tokens, custom: &[CompPreset], top: f32) -> f32 { +/// The width and height `tab` would take at `top`, from laying it out unseen: they follow the +/// font, the language and the scale. Nothing it registers or changes is kept. +fn tab_size(ui: &mut egui::Ui, tab: Tab, d: &CompDraft, t: &Tokens, custom: &[CompPreset], top: f32) -> egui::Vec2 { let rect = egui::Rect::from_min_size(egui::pos2(ui.max_rect().left(), top), vec2(ui.max_rect().width(), 0.0)); let mut child = ui.new_child(egui::UiBuilder::new().max_rect(rect).id_salt("comp-tab-measure").invisible()); tab_body(&mut child, tab, &mut d.clone(), t, &mut Registry::default(), custom, &mut Vec::new()); - child.cursor().top() - top + vec2(child.min_rect().width(), child.cursor().top() - top) +} + +fn content_width_id() -> egui::Id { + egui::Id::unique("comp-settings-content-width") +} + +/// The width the dialog's content took when it was last shown: its widest row on any tab. A +/// translation longer than the English (French, #2841) is wider than the 600-point dialog, so the +/// dialog grows to it rather than hiding OK past its right edge. +pub fn content_width(ctx: &egui::Context) -> f32 { + ctx.data(|d| d.get_temp::(content_width_id())).unwrap_or(0.0) } /// Draw the dialog. Returns `Some(params)` when OK is pressed, `Some(Null)` on Cancel. `custom` @@ -490,14 +501,16 @@ pub fn show( preview: Option<&mut bool>, ) -> Option { let mut out = None; - ui.horizontal(|ui| { + // The dialog's visible area, which Cancel and OK stay inside. + auto.add("dialog.comp.view", ui.clip_rect().intersect(ui.max_rect()), "Composition Settings"); + let name_row = ui.horizontal(|ui| { ui.label(tr("Composition Name:")); let r = ui.add(egui::TextEdit::singleline(&mut d.name).desired_width(300.0)); crate::widgets::focus_on_open(ui.ctx(), &r, &d.name); auto.add("dialog.comp.name", r.rect, "Composition Name"); }); ui.add_space(8.0); - ui.horizontal(|ui| { + let tab_row = ui.horizontal(|ui| { for (tab, label, key) in [(Tab::Basic, "Basic", "basic"), (Tab::Advanced, "Advanced", "advanced"), (Tab::Renderer, "3D Renderer", "renderer")] { let r = ui.selectable_label(d.tab == tab, tr(label)); auto.add(&format!("dialog.comp.tab.{key}"), r.rect, label); @@ -511,11 +524,12 @@ pub fn show( let body_top = ui.cursor().top(); tab_body(ui, d.tab, d, t, auto, custom, cmds); let shown = ui.cursor().top() - body_top; - let tallest = [Tab::Basic, Tab::Advanced, Tab::Renderer] - .into_iter() - .filter(|tab| *tab != d.tab) - .fold(shown, |h, tab| h.max(tab_height(ui, tab, d, t, custom, body_top))); + let sizes: Vec = [Tab::Basic, Tab::Advanced, Tab::Renderer].into_iter().map(|tab| tab_size(ui, tab, d, t, custom, body_top)).collect(); + let tallest = sizes.iter().fold(shown, |h, s| h.max(s.y)); ui.add_space((tallest - shown).max(0.0)); + // Every tab takes the width of the widest one too, so the dialog doesn't change size with them. + let widest = sizes.iter().fold(name_row.response.rect.width().max(tab_row.response.rect.width()), |w, s| w.max(s.x)); + ui.ctx().data_mut(|m| m.insert_temp(content_width_id(), widest.ceil())); ui.add_space(14.0); ui.horizontal(|ui| { ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index ac26bd0c1..e5c8950e7 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -446,7 +446,9 @@ fn comp_settings(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens, exis let live = existing && super::live_preview::active(app); let place = super::live_preview::place(app); let mut preview = app.dialog_state.live.on; - modal_placed(ctx, "Composition Settings", vec2(600.0, 470.0), t, place, |ui| { + // 600 points wide, or as wide as the content took last time (with the 18-point margins). + let width = (super::comp_settings::content_width(ctx) + 36.0).max(600.0); + modal_placed(ctx, "Composition Settings", vec2(width, 470.0), t, place, |ui| { result = super::comp_settings::show(ui, &mut d, t, &mut app.auto, &custom, &mut cmds, live.then_some(&mut preview)); }); app.dialog_state.live.on = preview; diff --git a/crates/ui-egui/tests/it/ui_comp_presets.rs b/crates/ui-egui/tests/it/ui_comp_presets.rs index 9e71f8fd7..a109d5324 100644 --- a/crates/ui-egui/tests/it/ui_comp_presets.rs +++ b/crates/ui-egui/tests/it/ui_comp_presets.rs @@ -103,3 +103,36 @@ fn composition_settings_buttons_stay_put_between_tabs() { } } } + +/// Composition Settings keeps Cancel and OK in view in every interface language: a translation +/// wider than the English (French, #2841) widens the dialog instead of pushing OK past its right +/// edge, where it could be reached only by scrolling sideways. +#[test] +fn composition_settings_buttons_fit_every_language() { + // English, then French (#2841) and the other catalogs whose rows overflowed the 600 points. + for language in ["en", "fr", "es", "ka", "cs", "pt-br"] { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Edit", "width": 2560, "height": 1440, "frameRate": 60, "duration": 4})).unwrap(); + s.prefs.general.language = language.into(); + let mut h = + Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).with_step_dt(1.0 / 60.0).allow_missing_glyphs().build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(3); + open_settings(&mut h); + let mut oks = vec![]; + for tab in ["basic", "advanced", "renderer"] { + click(&mut h, &format!("dialog.comp.tab.{tab}")); + let rect = |id: &str| h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).rect; + let (view, ok, cancel) = (rect("dialog.comp.view"), rect("dialog.comp.ok"), rect("dialog.comp.cancel")); + for (name, r) in [("OK", ok), ("Cancel", cancel)] { + assert!(r[0] >= view[0] && r[0] + r[2] <= view[0] + view[2] + 0.5, "{language} {tab}: {name} {r:?} is outside the dialog {view:?}"); + } + if language == "en" { + // English still fits the 600-point dialog (18-point margins). + assert_eq!(view[2], 600.0 - 36.0, "{tab}"); + } + oks.push(ok); + } + // The dialog takes the width of its widest tab, so OK stays put between tabs. + assert!(oks.windows(2).all(|w| w[0] == w[1]), "{language}: OK moves between tabs: {oks:?}"); + } +} From f6fd6de3bbab90e894355fc748760bdf2cfc9542 Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:03:48 +0100 Subject: [PATCH 2/2] Follow-up to #2841: Composition Settings settles after a width change - `dialog.comp.view` is the scroll area's viewport (its clip rect). Cut by the content's max rect, it lost the scrolled amount at the bottom. - When the dialog's width changes, the pass laid out at the old width is discarded and a repaint requested. - The measured width keeps room for a vertical scroll bar. Without it, switching the language with the dialog open left the two scroll bars trading places every frame: the UI never settled and OK was clipped every other frame. - `content_width` becomes `dialog_width`, which returns the dialog's width, so the 600-point rule is in one place. Tests: the view keeps its place when the dialog scrolls, and switching to French and back with the dialog open settles with OK inside it. --- crates/ui-egui/src/panels/comp_settings.rs | 31 ++++++++--- crates/ui-egui/src/panels/dialogs.rs | 4 +- crates/ui-egui/tests/it/ui_comp_presets.rs | 60 ++++++++++++++++++++++ 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/panels/comp_settings.rs b/crates/ui-egui/src/panels/comp_settings.rs index 26d3c3a69..b778a3999 100644 --- a/crates/ui-egui/src/panels/comp_settings.rs +++ b/crates/ui-egui/src/panels/comp_settings.rs @@ -478,15 +478,17 @@ fn tab_size(ui: &mut egui::Ui, tab: Tab, d: &CompDraft, t: &Tokens, custom: &[Co vec2(child.min_rect().width(), child.cursor().top() - top) } +/// Where the width of the dialog's content, measured when it was last shown, is kept. fn content_width_id() -> egui::Id { egui::Id::unique("comp-settings-content-width") } -/// The width the dialog's content took when it was last shown: its widest row on any tab. A -/// translation longer than the English (French, #2841) is wider than the 600-point dialog, so the -/// dialog grows to it rather than hiding OK past its right edge. -pub fn content_width(ctx: &egui::Context) -> f32 { - ctx.data(|d| d.get_temp::(content_width_id())).unwrap_or(0.0) +/// The dialog's width: 600 points, or more when the widest row on any of its tabs (as last shown), +/// a scroll bar and the modal's two 18-point margins need it. A translation longer than the English (French, +/// #2841) widens the dialog rather than hiding OK past its right edge. +pub fn dialog_width(ctx: &egui::Context) -> f32 { + let content = ctx.data(|d| d.get_temp::(content_width_id())).unwrap_or(0.0); + (content + 36.0).max(600.0) } /// Draw the dialog. Returns `Some(params)` when OK is pressed, `Some(Null)` on Cancel. `custom` @@ -501,8 +503,9 @@ pub fn show( preview: Option<&mut bool>, ) -> Option { let mut out = None; - // The dialog's visible area, which Cancel and OK stay inside. - auto.add("dialog.comp.view", ui.clip_rect().intersect(ui.max_rect()), "Composition Settings"); + // The dialog's visible area, which Cancel and OK stay inside: the scroll area's viewport, + // which stays put when the content scrolls. + auto.add("dialog.comp.view", ui.clip_rect(), "Composition Settings"); let name_row = ui.horizontal(|ui| { ui.label(tr("Composition Name:")); let r = ui.add(egui::TextEdit::singleline(&mut d.name).desired_width(300.0)); @@ -529,7 +532,19 @@ pub fn show( ui.add_space((tallest - shown).max(0.0)); // Every tab takes the width of the widest one too, so the dialog doesn't change size with them. let widest = sizes.iter().fold(name_row.response.rect.width().max(tab_row.response.rect.width()), |w, s| w.max(s.x)); - ui.ctx().data_mut(|m| m.insert_temp(content_width_id(), widest.ceil())); + // Room for a vertical scroll bar too: without it, a bar shown while the dialog settles narrows + // the content into a horizontal bar, which takes the height back from it, and the two bars + // trade places every frame. + let needed = widest + ui.spacing().scroll.allocated_width(); + let ctx = ui.ctx().clone(); + let before = dialog_width(&ctx); + ctx.data_mut(|m| m.insert_temp(content_width_id(), needed.ceil())); + if dialog_width(&ctx) != before { + // This pass was laid out at the old width: lay it out again at the new one instead of + // showing it, and paint again in case egui declines the extra pass. + ctx.request_discard("Composition Settings width"); + ctx.request_repaint(); + } ui.add_space(14.0); ui.horizontal(|ui| { ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index e5c8950e7..d3ee74e58 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -446,9 +446,7 @@ fn comp_settings(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens, exis let live = existing && super::live_preview::active(app); let place = super::live_preview::place(app); let mut preview = app.dialog_state.live.on; - // 600 points wide, or as wide as the content took last time (with the 18-point margins). - let width = (super::comp_settings::content_width(ctx) + 36.0).max(600.0); - modal_placed(ctx, "Composition Settings", vec2(width, 470.0), t, place, |ui| { + modal_placed(ctx, "Composition Settings", vec2(super::comp_settings::dialog_width(ctx), 470.0), t, place, |ui| { result = super::comp_settings::show(ui, &mut d, t, &mut app.auto, &custom, &mut cmds, live.then_some(&mut preview)); }); app.dialog_state.live.on = preview; diff --git a/crates/ui-egui/tests/it/ui_comp_presets.rs b/crates/ui-egui/tests/it/ui_comp_presets.rs index a109d5324..262d2183b 100644 --- a/crates/ui-egui/tests/it/ui_comp_presets.rs +++ b/crates/ui-egui/tests/it/ui_comp_presets.rs @@ -136,3 +136,63 @@ fn composition_settings_buttons_fit_every_language() { assert!(oks.windows(2).all(|w| w[0] == w[1]), "{language}: OK moves between tabs: {oks:?}"); } } + +fn rect_of(h: &Harness<'_, EffectcraftApp>, id: &str) -> egui::Rect { + let r = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).rect; + egui::Rect::from_min_size(pos2(r[0], r[1]), egui::vec2(r[2], r[3])) +} + +/// In a window too short for it, the dialog scrolls: its registered view stays the scroll +/// area's viewport (not a part of it shifted by the scroll), and OK scrolls into it. +#[test] +fn composition_settings_view_stays_put_when_it_scrolls() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Edit", "width": 2560, "height": 1440, "frameRate": 60, "duration": 4})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 520.0)).with_step_dt(1.0 / 60.0).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(3); + open_settings(&mut h); + let (view, ok) = (rect_of(&h, "dialog.comp.view"), rect_of(&h, "dialog.comp.ok")); + assert!(ok.bottom() > view.bottom(), "the dialog must be taller than the window for this test: OK {ok:?}, view {view:?}"); + h.input_mut().events.push(Event::PointerMoved(view.center())); + h.input_mut().events.push(Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: egui::vec2(0.0, -2000.0), + modifiers: Modifiers::NONE, + phase: egui::TouchPhase::Move, + source: egui::MouseWheelSource::Wheel, + }); + h.run_steps(30); + let (scrolled, ok) = (rect_of(&h, "dialog.comp.view"), rect_of(&h, "dialog.comp.ok")); + // The scroll bar may take a few points on the right while it is shown. + assert_eq!((scrolled.y_range(), scrolled.left()), (view.y_range(), view.left()), "the view is the viewport, wherever the content is scrolled to"); + assert!(scrolled.contains_rect(ok), "OK {ok:?} scrolls into the view {scrolled:?}"); +} + +/// Changing the interface language with the dialog open refits it to the new text and then +/// settles, with OK inside it: its scroll bars don't trade places every frame while it grows. +/// Switching back to English returns it to 600 points. +#[test] +fn composition_settings_refits_when_the_language_changes() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Edit", "width": 2560, "height": 1440, "frameRate": 60, "duration": 4})).unwrap(); + let mut h = Harness::builder() + .with_size(egui::vec2(1600.0, 1000.0)) + .with_step_dt(1.0 / 60.0) + .with_max_steps(30) + // `run` checks accessibility after settling; that is not what this test is about. + .with_accessibility_check(false) + .build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(3); + open_settings(&mut h); + click(&mut h, "dialog.comp.tab.advanced"); + h.run_ok().expect("the open dialog settles"); + assert_eq!(rect_of(&h, "dialog.comp.view").width(), 600.0 - 36.0); + for (language, wider) in [("fr", true), ("en", false)] { + h.state_mut().session.prefs.general.language = language.into(); + // Steps while anything asks for a repaint, and fails if that never stops. + let steps = h.try_run().unwrap_or_else(|e| panic!("{language}: {e}")); + let (view, ok) = (rect_of(&h, "dialog.comp.view"), rect_of(&h, "dialog.comp.ok")); + assert!(view.contains_rect(ok), "{language}: after {steps} steps OK {ok:?} is outside the dialog {view:?}"); + assert_eq!(view.width() > 600.0 - 36.0, wider, "{language}: the dialog is {} wide", view.width()); + } +}