Merge PR #2892

crates/ui-egui/src/panels/comp_settings.rs

@@ -469,13 +469,26 @@ 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)
+}
+
+/// 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 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::<f32>(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`
@@ -490,14 +503,17 @@ pub fn show(
     preview: Option<&mut bool>,
 ) -> Option<serde_json::Value> {
     let mut out = None;
-    ui.horizontal(|ui| {
+    // 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));
         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 +527,24 @@ 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<egui::Vec2> = [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));
+    // 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| {

crates/ui-egui/src/panels/dialogs.rs

@@ -446,7 +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;
-    modal_placed(ctx, "Composition Settings", vec2(600.0, 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;

crates/ui-egui/tests/it/ui_comp_presets.rs

@@ -103,3 +103,96 @@ 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:?}");
+    }
+}
+
+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());
+    }
+}