Merge pull request #2840 from KaboomFox/plugin-dialog-scrolls

Dialogs: a plug-in's parameters scroll when they don't fit the window

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

@@ -370,54 +370,59 @@ fn typed_number(x: f64) -> Value {
 }
 
 /// Editor for a plug-in's parameters from its schema (plug-in filter and effect dialogs): numbers
-/// within their range, whole numbers, checkboxes and dropdowns, in declaration order. Returns
-/// true when a value changed. The labels (from the parameter keys) and choices are the plug-in's
-/// text, shown as they are.
+/// within their range, whole numbers, checkboxes and dropdowns, in declaration order, a row each.
+/// Returns true when a value changed. The labels (from the parameter keys) and choices are the
+/// plug-in's text, shown as they are. A plug-in may declare more of them
+/// ([`MAX_PARAMS`](vectorcraft_plugins::manifest::MAX_PARAMS)) than the window has room for, and
+/// the rows then scroll ([`super::modal::scrolled_rows`]).
 pub(super) fn schema_fields(ui: &mut egui::Ui, d: &mut Dialog, specs: &[(String, vectorcraft_plugins::ParamSpec)]) -> bool {
     use vectorcraft_plugins::ParamSpec;
     let t = Tokens::get(ui.ctx());
     let mut changed = false;
-    egui::Grid::new("plugin-grid").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| {
-        for (k, spec) in specs {
-            ui.label(egui::RichText::new(humanize(k)).color(t.text));
-            let cur = d.fields.get(k).cloned().unwrap_or_else(|| spec.default_value());
-            let new = match spec {
-                ParamSpec::Number { min, max, .. } => {
-                    let x = cur.as_f64().unwrap_or(*min);
-                    crate::widgets::range_field(ui, ("plugin-num", k), x, *min..=*max, "", 2, 100.0).map(|x| json!(x))
-                }
-                ParamSpec::Int { min, max, .. } => {
-                    let x = cur.as_i64().unwrap_or(*min).clamp(*min, *max);
-                    // An f64 holds every whole number up to 2^53, so the numeric field (arrows, wheel, arithmetic) is exact there.
-                    // Past it, the number typed is read as an integer.
-                    const EXACT: i64 = 1 << 53;
-                    if -EXACT <= *min && *max <= EXACT {
-                        crate::widgets::range_field(ui, ("plugin-int", k), x as f64, *min as f64..=*max as f64, "", 0, 100.0).map(|x| json!(x as i64))
-                    } else {
-                        let shown = x.to_string();
-                        crate::widgets::take_dialog_focus(ui, ui.id().with(("plugin-int", k)), &shown);
-                        crate::widgets::text_field(ui, ("plugin-int", k), Some(&shown), 100.0, 1)
-                            .and_then(|text| text.parse::<i64>().ok())
-                            .map(|x| json!(x.clamp(*min, *max)))
+    super::modal::scrolled_rows(ui, "plugin-fields", |ui| {
+        egui::Grid::new("plugin-grid").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| {
+            for (k, spec) in specs {
+                ui.label(egui::RichText::new(humanize(k)).color(t.text));
+                let cur = d.fields.get(k).cloned().unwrap_or_else(|| spec.default_value());
+                let new = match spec {
+                    ParamSpec::Number { min, max, .. } => {
+                        let x = cur.as_f64().unwrap_or(*min);
+                        crate::widgets::range_field(ui, ("plugin-num", k), x, *min..=*max, "", 2, 100.0).map(|x| json!(x))
                     }
+                    ParamSpec::Int { min, max, .. } => {
+                        let x = cur.as_i64().unwrap_or(*min).clamp(*min, *max);
+                        // An f64 holds every whole number up to 2^53, so the numeric field (arrows, wheel, arithmetic) is exact there.
+                        // Past it, the number typed is read as an integer.
+                        const EXACT: i64 = 1 << 53;
+                        if -EXACT <= *min && *max <= EXACT {
+                            crate::widgets::range_field(ui, ("plugin-int", k), x as f64, *min as f64..=*max as f64, "", 0, 100.0)
+                                .map(|x| json!(x as i64))
+                        } else {
+                            let shown = x.to_string();
+                            crate::widgets::take_dialog_focus(ui, ui.id().with(("plugin-int", k)), &shown);
+                            crate::widgets::text_field(ui, ("plugin-int", k), Some(&shown), 100.0, 1)
+                                .and_then(|text| text.parse::<i64>().ok())
+                                .map(|x| json!(x.clamp(*min, *max)))
+                        }
+                    }
+                    ParamSpec::Bool { .. } => {
+                        let b = cur.as_bool().unwrap_or(false);
+                        crate::widgets::check_box(ui, &humanize(k), b, true).then(|| json!(!b))
+                    }
+                    ParamSpec::Choice { options, .. } => {
+                        let labels: Vec<&str> = options.iter().map(String::as_str).collect();
+                        crate::widgets::dropdown_names(ui, ("plugin-choice", k), cur.as_str().unwrap_or_default(), &labels, 140.0)
+                            .and_then(|i| options.get(i))
+                            .map(|o| json!(o))
+                    }
+                };
+                if let Some(v) = new {
+                    d.fields.insert(k.clone(), v);
+                    changed = true;
                 }
-                ParamSpec::Bool { .. } => {
-                    let b = cur.as_bool().unwrap_or(false);
-                    crate::widgets::check_box(ui, &humanize(k), b, true).then(|| json!(!b))
-                }
-                ParamSpec::Choice { options, .. } => {
-                    let labels: Vec<&str> = options.iter().map(String::as_str).collect();
-                    crate::widgets::dropdown_names(ui, ("plugin-choice", k), cur.as_str().unwrap_or_default(), &labels, 140.0)
-                        .and_then(|i| options.get(i))
-                        .map(|o| json!(o))
-                }
-            };
-            if let Some(v) = new {
-                d.fields.insert(k.clone(), v);
-                changed = true;
+                ui.end_row();
             }
-            ui.end_row();
-        }
+        });
     });
     changed
 }

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

@@ -1,7 +1,8 @@
 //! The modal dialog window: a backdrop that keeps the rest of the app from taking clicks, and a
 //! window with no title bar that opens centred and moves by dragging its heading (the band from its
 //! top edge down to the heading), as a title bar would. Where it was moved to stays, per window, for
-//! the session, and the window never leaves the screen.
+//! the session, and the window never leaves the screen. Rows too many for the screen's height
+//! scroll inside it ([`scrolled_rows`]), so its heading and buttons stay in view.
 
 use egui::{Id, Rect, Vec2};
 
@@ -22,7 +23,7 @@ pub(crate) fn show_at<R>(ctx: &egui::Context, title: &str, id: Id, initial: Vec2
         // Modal, but the canvas isn't dimmed so previews stay readable (as in the reference app).
         ui.allocate_rect(ctx.content_rect(), egui::Sense::click());
     });
-    egui::Window::new(title)
+    let shown = egui::Window::new(title)
         .id(id)
         .order(egui::Order::Foreground)
         .collapsible(false)
@@ -30,8 +31,9 @@ pub(crate) fn show_at<R>(ctx: &egui::Context, title: &str, id: Id, initial: Vec2
         .title_bar(false)
         .anchor(egui::Align2::CENTER_CENTER, offset(ctx, id, initial))
         .frame(egui::Frame::window(&ctx.global_style()).fill(t.panel).inner_margin(egui::Margin::same(margin)))
-        .show(ctx, add)
-        .and_then(|r| r.inner)
+        .show(ctx, add)?;
+    keep_rest(ctx, id, shown.response.rect.height());
+    shown.inner
 }
 
 /// The layer of the backdrop behind a modal dialog window, which takes the clicks the rest of
@@ -68,6 +70,42 @@ fn handle(id: Id) -> Id {
     id.with("modal-drag")
 }
 
+/// The least height of [`scrolled_rows`] in a window too short for the rest of its contents: about
+/// three rows.
+const MIN_ROWS_H: f32 = 100.0;
+
+/// Under a modal window's id: how tall its [`scrolled_rows`] were drawn this frame, and how tall
+/// the rest of it was when last drawn.
+const ROWS_KEY: &str = "modal-rows";
+const REST_KEY: &str = "modal-rest";
+
+/// Rows of the modal window `ui` is in (one list of them per window) that scroll if the window
+/// would be taller than the screen. Their height is the screen's less the rest of the window as
+/// last drawn (a frame late, as only the drawn window knows its height), and at least
+/// [`MIN_ROWS_H`]. Rows that fit are as tall as they are.
+pub(crate) fn scrolled_rows<R>(ui: &mut egui::Ui, salt: impl egui::AsIdSalt, add: impl FnOnce(&mut egui::Ui) -> R) -> R {
+    let id = ui.layer_id().id;
+    let rest = ui.ctx().data(|m| m.get_temp::<f32>(id.with(REST_KEY))).unwrap_or(0.0);
+    let room = (ui.ctx().content_rect().height() - rest - 2.0 * super::EDGE_GAP).max(MIN_ROWS_H);
+    // As tall as the room at once: by itself a scroll area grows a frame at a time with the window.
+    let rows = ui.scope(|ui| egui::ScrollArea::vertical().id_salt(salt).max_height(room).min_scrolled_height(room).show(ui, add).inner);
+    ui.ctx().data_mut(|m| m.insert_temp(id.with(ROWS_KEY), rows.response.rect.height()));
+    rows.inner
+}
+
+/// After modal window `id` is drawn `height` tall: keep the height of all of it but its
+/// [`scrolled_rows`] for the next frame. If the rows were sized by a different one (the window just
+/// opened, or what is around them changed), the next frame is drawn at once, for the rows to fit
+/// without waiting for input.
+fn keep_rest(ctx: &egui::Context, id: Id, height: f32) {
+    let Some(rows) = ctx.data_mut(|m| m.remove_temp::<f32>(id.with(ROWS_KEY))) else { return };
+    let rest = (height - rows).max(0.0);
+    let used = ctx.data_mut(|m| std::mem::replace(m.get_temp_mut_or(id.with(REST_KEY), 0.0), rest));
+    if (used - rest).abs() > 0.5 {
+        ctx.request_repaint();
+    }
+}
+
 /// Where window `id` sits, from the screen's centre: `nudge` down until it is first moved. While
 /// its [`drag_band`] is dragged, where it sat when the drag began plus how far the pointer has moved
 /// since the press, kept on screen (applied before the window places itself, so it follows the

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

@@ -1,5 +1,6 @@
 //! #423: modal dialogs move by their heading (and stay on screen and where they were moved), and
-//! the form dialogs' choice fields are dropdowns (Offset Path's Joins).
+//! the form dialogs' choice fields are dropdowns (Offset Path's Joins). A plug-in's dialog of more
+//! settings than the window has room for fits it, its settings scrolling.
 
 use egui::{Event, Pos2, Rect, Vec2, vec2};
 use serde_json::json;
@@ -117,6 +118,70 @@ fn dialogs_move_by_their_heading_stay_on_screen_and_where_they_were_moved() {
     }
 }
 
+/// Open the dialog of a plug-in of `kind` (`effect` or `filter`) with `count` number settings over a
+/// selection, settle it and `check` it, given its window and heading. The other tests share the
+/// plug-in registry, and the plug-in is removed after.
+fn check_plugin_dialog(kind: &str, count: usize, check: impl FnOnce(&egui::Context, &mut VectorcraftApp, egui::Id, &str)) {
+    use vectorcraft_plugins::wat as tpl;
+    let params: Vec<String> = (0..count).map(|i| format!(r#""setting{i}":{{"type":"number","min":0,"max":10,"default":1}}"#)).collect();
+    let id = format!("test.ui.{kind}{count}");
+    let src = tpl::module(&tpl::manifest(&id, kind, &format!("{{{}}}", params.join(","))), tpl::ECHO, "", 1);
+    let (ctx, mut app) = (ctx(), app_with_selection());
+    app.run("plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&wat::parse_str(src).unwrap())})).unwrap();
+    let (command, params, dialog) = match kind {
+        "effect" => ("effect.dialog", json!({"effect": vectorcraft_plugins::effect::effect_id(&id)}), "effect"),
+        _ => ("plugin.dialog", json!({"id": id}), plugin::KIND),
+    };
+    app.run(command, params).unwrap();
+    settle(&ctx, &mut app);
+    check(&ctx, &mut app, egui::Id::new(("dialog", dialog)), &format!("Test {id}"));
+    app.run("plugin.remove", json!({"id": id})).unwrap();
+}
+
+/// A plug-in can declare more settings than the window has room for, a row each. Its dialog, an
+/// effect's or a filter's, fits the window all the same, heading and buttons included. The
+/// settings take the height there is, and the wheel scrolls them to the last one. A dialog of a few
+/// settings is as tall as they are.
+#[test]
+fn a_plug_in_dialog_of_many_settings_fits_the_window() {
+    use vectorcraft_plugins::manifest::MAX_PARAMS;
+    let screen = Rect::from_min_size(Pos2::ZERO, SCREEN);
+    let shown = |texts: &[(String, Rect)], text: &str| texts.iter().find(|(t, r)| t == text && screen.contains_rect(*r)).map(|(_, r)| *r);
+    for kind in ["effect", "filter"] {
+        check_plugin_dialog(kind, MAX_PARAMS, |ctx, app, window, heading| {
+            let last = format!("Setting{}:", MAX_PARAMS - 1);
+            let texts = frame(ctx, app, vec![]);
+            let rect = rect_of(ctx, window);
+            assert!(screen.contains_rect(rect) && rect.height() > 0.75 * SCREEN.y, "{kind}: the dialog is at {rect:?} in the {SCREEN:?} window");
+            for text in [heading, "Setting0:", "OK", "Cancel"] {
+                assert!(shown(&texts, text).is_some(), "{kind}: {text} isn't on screen: {texts:?}");
+            }
+            assert!(shown(&texts, &last).is_none(), "{kind}: {last} is out of view at first");
+            let over = shown(&texts, "Setting0:").unwrap().center();
+            let wheel = Event::MouseWheel {
+                unit: egui::MouseWheelUnit::Line,
+                delta: vec2(0.0, -200.0),
+                phase: egui::TouchPhase::Move,
+                modifiers: Default::default(),
+            };
+            frame(ctx, app, vec![Event::PointerMoved(over), wheel]);
+            for _ in 0..30 {
+                frame(ctx, app, vec![]);
+            }
+            let texts = frame(ctx, app, vec![]);
+            for text in [heading, last.as_str(), "OK", "Cancel"] {
+                assert!(shown(&texts, text).is_some(), "{kind}, scrolled to the end: {text} isn't on screen: {texts:?}");
+            }
+            assert_eq!(rect_of(ctx, window), rect, "{kind}: scrolling leaves the window as it was");
+        });
+    }
+    check_plugin_dialog("effect", 4, |ctx, app, window, _| {
+        let texts = frame(ctx, app, vec![]);
+        let rect = rect_of(ctx, window);
+        assert!(rect.height() < SCREEN.y / 2.0 && shown(&texts, "Setting3:").is_some(), "4 settings: {rect:?} {texts:?}");
+    });
+}
+
 /// Offset Path's Joins is a dropdown of Miter, Round and Bevel (it was a text field), listed after
 /// Offset; choosing Round runs the command with round joins.
 #[test]

crates/ui-egui/src/i18n/tests.rs

@@ -954,8 +954,11 @@ fn menu_catalogs_translate_every_menu_label() {
     let language_name = |l: &str| Lang::all().any(|lang| lang.name() == l);
     // Font names and sizes, and installed plug-ins' own names, are not interface text.
     let interface = |(label, id, p): &(&str, &str, serde_json::Value)| {
-        let plugin = *id == "plugin.dialog" || p.get("effect").and_then(serde_json::Value::as_str).is_some_and(|e| e.starts_with("plugin."));
-        *id != "text.setStyle" && !plugin && shown.contains(*label) && !MENU_KEEP_AS_IS.contains(label) && !language_name(label)
+        *id != "text.setStyle"
+            && !crate::menus::lists_plugin(id, p)
+            && shown.contains(*label)
+            && !MENU_KEEP_AS_IS.contains(label)
+            && !language_name(label)
     };
     let mut all: Vec<String> = labels.iter().filter(|l| interface(l)).map(|(l, ..)| l.to_string()).collect();
     all.extend(toggled_labels());

crates/ui-egui/src/menus.rs

@@ -1917,6 +1917,13 @@ fn shows_a_name(id: &str, label: &str) -> bool {
         || (matches!(id, "effect.apply" | "effect.dialog") && vectorcraft_effects::plugin_effects().iter().any(|e| e.label == label))
 }
 
+/// Does the menu item `id` with `params` list an installed plug-in (an Object › Plug-ins filter or
+/// an Effect › Plug-ins effect), by the plug-in's own name? Told by the item itself, so it holds
+/// while another thread installs or removes plug-ins.
+pub(crate) fn lists_plugin(id: &str, params: &Value) -> bool {
+    id == "plugin.dialog" || params.get("effect").and_then(Value::as_str).and_then(vectorcraft_plugins::effect::plugin_id).is_some()
+}
+
 /// The label of a menu item as drawn: [`dynamic_label`] in the UI language. Labels assembled
 /// around a name (Undo *Move*, Reset *Essentials*, Last Effect: *Drop Shadow*) are translated as
 /// templates so the name can move; names that are user data (files, views, fonts, custom
@@ -3952,17 +3959,17 @@ pub const DYNAMIC_LABELS: &[&str] = &[
 
 /// Every English string the menus, the command palette, the panel registry, the toolbar and the
 /// Preferences dialog can show: what a complete language catalog has to cover. Names that are user
-/// data (fonts, recent files, saved views, libraries), the point sizes and the view angles are
-/// left out.
+/// data (fonts, recent files, saved views, libraries, installed plug-ins), the point sizes and the
+/// view angles are left out.
 pub fn menu_strings() -> std::collections::BTreeSet<String> {
     use std::collections::BTreeSet;
-    fn user_data(id: &str) -> bool {
-        hidden_when_disabled(id) || id.starts_with("type.recentFont")
+    fn user_data(id: &str, params: &Value) -> bool {
+        hidden_when_disabled(id) || id.starts_with("type.recentFont") || lists_plugin(id, params)
     }
     fn walk(items: &[Item], under: &str, out: &mut BTreeSet<String>) {
         for it in items {
             match it {
-                Item::Cmd(l, id, _) => {
+                Item::Cmd(l, id, params) => {
                     let size = l.strip_suffix(" pt").is_some_and(|n| n.chars().all(|c| c.is_ascii_digit()));
                     // View › Rotate View lists angles: an optional minus sign, digits and a degree sign.
                     let angle = l
@@ -3971,7 +3978,7 @@ pub fn menu_strings() -> std::collections::BTreeSet<String> {
                         .is_some_and(|n| !n.is_empty() && n.chars().all(|c| c.is_ascii_digit()));
                     // VectorCraft › Language lists each language by its own name.
                     let language_name = *id == "app.language" && crate::i18n::Lang::all().any(|lang| lang.name() == *l);
-                    if !user_data(id) && !size && !angle && !language_name && *l != "—" {
+                    if !user_data(id, params) && !size && !angle && !language_name && *l != "—" {
                         out.insert(l.to_string());
                     }
                 }

crates/ui-egui/src/tests_plugins.rs

@@ -58,6 +58,12 @@ fn plugins_are_listed_in_the_object_and_effect_menus() {
     assert_eq!((fx.command.as_deref(), &fx.params), (Some("effect.dialog"), &json!({"effect": "plugin.test.ui.echo"})));
     // Menu clicks on items with params don't turn into the generic parameter dialog.
     assert_eq!(menus::click_target("Desaturate…", "plugin.dialog", &filter.params).0, "plugin.dialog");
+    // Their names are the plug-ins' own, not interface text a language catalog has to cover (they
+    // are installed for the whole test run, while the catalog tests run).
+    let strings = menus::menu_strings();
+    for name in ["Desaturate…", "Test test.ui.echo…"] {
+        assert!(!strings.contains(name), "{name} is a menu string to translate");
+    }
 }
 
 #[test]

docs/plugins.md

@@ -33,7 +33,8 @@ Live effects are applied like any effect: `effect.apply {"effect": "plugin.<id>"
 In the app, installed filters appear under **Object › Plug-ins**, next to **Install Plug-in…** and **Reload
 Plug-ins**; live effects under **Effect › Plug-ins** and in the Appearance panel's effect menu. A plug-in with
 parameters gets a dialog generated from its schema (numbers, whole numbers, checkboxes, dropdowns) with a live
-preview; without parameters it runs at once. **File › Open** of a `.wasm` file installs it too (on the web this is
+preview; without parameters it runs at once. Parameters too many for the window's height scroll inside the dialog,
+which keeps its heading and buttons in view. **File › Open** of a `.wasm` file installs it too (on the web this is
 how plug-ins get in).
 
 **Preference folder.** Edit › Preferences › Performance & Storage › *Additional Plug-ins Folder*: when set, every