Fix #978: keep form choices and buttons inside the dialog
Undoes the 800 px Project Settings width from #980: the form is back at the shared 480 px. A form's choice popup keeps its 240 px column and ends a long choice (a GPU adapter's name) in an ellipsis instead of widening the grid past the dialog. Only the fields scroll, and only vertically; OK and Cancel sit under the scroll area, so they stay in view. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
470ea7a3af
commit
97514ef198
|
|
@ -153,7 +153,7 @@ pub fn route_layer_settings(app: &mut EffectcraftApp, id: &str, params: &Value)
|
|||
}
|
||||
|
||||
/// The room a modal dialog may take: the window less a margin.
|
||||
fn modal_space(ctx: &egui::Context) -> egui::Vec2 {
|
||||
pub(crate) fn modal_space(ctx: &egui::Context) -> egui::Vec2 {
|
||||
(ctx.content_rect().size() - vec2(48.0, 48.0)).max(vec2(120.0, 80.0))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -750,6 +750,87 @@ pub fn open_form(app: &mut EffectcraftApp, id: &str, p: &Value) -> bool {
|
|||
true
|
||||
}
|
||||
|
||||
/// The label and control rows of a form: the part of [`show_form`] that scrolls.
|
||||
fn form_grid(ui: &mut egui::Ui, f: &mut Form, regs: &mut Vec<(String, egui::Rect, String)>, browse: &mut Option<usize>, focused: &mut bool) {
|
||||
egui::Grid::new("form-grid").num_columns(2).spacing([14.0, 10.0]).show(ui, |ui| {
|
||||
for (i, fl) in f.fields.iter_mut().enumerate() {
|
||||
ui.label(tr(&fl.label));
|
||||
let r = match &mut fl.kind {
|
||||
FieldKind::Number { value, speed } => {
|
||||
let dec = if *speed < 0.01 { 5 } else { 3 };
|
||||
let r = ui.add(crate::widgets::drag_value(egui::DragValue::new(value).speed(*speed).max_decimals(dec)));
|
||||
// The first field takes the keyboard when the form opens. (A number edits
|
||||
// as text once focused; the selection is clamped to what it shows.)
|
||||
if !std::mem::replace(focused, true) {
|
||||
crate::widgets::focus_on_open(ui.ctx(), &r, &" ".repeat(64));
|
||||
}
|
||||
r.rect
|
||||
}
|
||||
FieldKind::Text(s) => {
|
||||
let r = ui.add(egui::TextEdit::singleline(s).desired_width(220.0));
|
||||
if !std::mem::replace(focused, true) {
|
||||
crate::widgets::focus_on_open(ui.ctx(), &r, s);
|
||||
}
|
||||
r.rect
|
||||
}
|
||||
FieldKind::Path { value, .. } => {
|
||||
ui.horizontal(|ui| {
|
||||
let r = ui.add(egui::TextEdit::singleline(value).desired_width(150.0)).rect;
|
||||
let b = ui.button(tr("Browse…"));
|
||||
regs.push((format!("form.field.{}.browse", fl.key), b.rect, "Browse…".into()));
|
||||
if b.clicked() {
|
||||
*browse = Some(i);
|
||||
}
|
||||
r
|
||||
})
|
||||
.inner
|
||||
}
|
||||
FieldKind::SavePath(value) => {
|
||||
ui.horizontal(|ui| {
|
||||
let r = ui.add(egui::TextEdit::singleline(value).desired_width(150.0)).rect;
|
||||
let b = ui.button(tr("Browse…"));
|
||||
regs.push((format!("form.field.{}.browse", fl.key), b.rect, "Browse…".into()));
|
||||
if b.clicked() {
|
||||
*browse = Some(i);
|
||||
}
|
||||
r
|
||||
})
|
||||
.inner
|
||||
}
|
||||
// A fixed width: in the grid's value column a wrapping label would otherwise
|
||||
// shrink to a word per line.
|
||||
FieldKind::Note(text) => {
|
||||
ui.scope(|ui| {
|
||||
ui.set_width(220.0);
|
||||
ui.add(egui::Label::new(egui::RichText::new(tr(text.as_str())).small()).wrap())
|
||||
})
|
||||
.inner
|
||||
.rect
|
||||
}
|
||||
FieldKind::Bool(b) => ui.checkbox(b, "").rect,
|
||||
FieldKind::Choice { options, sel } => {
|
||||
let cur = options.get(*sel).map(|o| o.0.clone()).unwrap_or_default();
|
||||
// The column keeps its width: a long choice (a GPU adapter's name) ends in an
|
||||
// ellipsis instead of widening the form past the dialog (#978).
|
||||
egui::ComboBox::from_id_salt(("form", fl.key.as_str()))
|
||||
.selected_text(tr(&cur))
|
||||
.width(240.0)
|
||||
.truncate()
|
||||
.show_ui(ui, |ui| {
|
||||
for (i, (l, _)) in options.iter().enumerate() {
|
||||
ui.selectable_value(sel, i, tr(l));
|
||||
}
|
||||
})
|
||||
.response
|
||||
.rect
|
||||
}
|
||||
};
|
||||
regs.push((format!("form.field.{}", fl.key), r, fl.label.clone()));
|
||||
ui.end_row();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
pub fn show_form(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
|
||||
let mut f = app.dialog_state.form.clone();
|
||||
let (mut ok, mut close) = (false, false);
|
||||
|
|
@ -766,83 +847,13 @@ pub fn show_form(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
|
|||
let mut regs: Vec<(String, egui::Rect, String)> = vec![];
|
||||
let mut browse: Option<usize> = None;
|
||||
let mut focused = false;
|
||||
// Project Settings needs room for its labels and the selected GPU adapter name. At the
|
||||
// generic form width its grid overflows, pushing the buttons off the visible area (#978).
|
||||
let width = if f.title == "Project Settings" { 800.0 } else { 480.0 };
|
||||
super::dialogs::modal(ctx, &f.title.clone(), vec2(width, h), t, |ui| {
|
||||
egui::Grid::new("form-grid").num_columns(2).spacing([14.0, 10.0]).show(ui, |ui| {
|
||||
for (i, fl) in f.fields.iter_mut().enumerate() {
|
||||
ui.label(tr(&fl.label));
|
||||
let r = match &mut fl.kind {
|
||||
FieldKind::Number { value, speed } => {
|
||||
let dec = if *speed < 0.01 { 5 } else { 3 };
|
||||
let r = ui.add(crate::widgets::drag_value(egui::DragValue::new(value).speed(*speed).max_decimals(dec)));
|
||||
// The first field takes the keyboard when the form opens. (A number edits
|
||||
// as text once focused; the selection is clamped to what it shows.)
|
||||
if !std::mem::replace(&mut focused, true) {
|
||||
crate::widgets::focus_on_open(ui.ctx(), &r, &" ".repeat(64));
|
||||
}
|
||||
r.rect
|
||||
}
|
||||
FieldKind::Text(s) => {
|
||||
let r = ui.add(egui::TextEdit::singleline(s).desired_width(220.0));
|
||||
if !std::mem::replace(&mut focused, true) {
|
||||
crate::widgets::focus_on_open(ui.ctx(), &r, s);
|
||||
}
|
||||
r.rect
|
||||
}
|
||||
FieldKind::Path { value, .. } => {
|
||||
ui.horizontal(|ui| {
|
||||
let r = ui.add(egui::TextEdit::singleline(value).desired_width(150.0)).rect;
|
||||
let b = ui.button(tr("Browse…"));
|
||||
regs.push((format!("form.field.{}.browse", fl.key), b.rect, "Browse…".into()));
|
||||
if b.clicked() {
|
||||
browse = Some(i);
|
||||
}
|
||||
r
|
||||
})
|
||||
.inner
|
||||
}
|
||||
FieldKind::SavePath(value) => {
|
||||
ui.horizontal(|ui| {
|
||||
let r = ui.add(egui::TextEdit::singleline(value).desired_width(150.0)).rect;
|
||||
let b = ui.button(tr("Browse…"));
|
||||
regs.push((format!("form.field.{}.browse", fl.key), b.rect, "Browse…".into()));
|
||||
if b.clicked() {
|
||||
browse = Some(i);
|
||||
}
|
||||
r
|
||||
})
|
||||
.inner
|
||||
}
|
||||
// A fixed width: in the grid's value column a wrapping label would otherwise
|
||||
// shrink to a word per line.
|
||||
FieldKind::Note(text) => {
|
||||
ui.scope(|ui| {
|
||||
ui.set_width(220.0);
|
||||
ui.add(egui::Label::new(egui::RichText::new(tr(text.as_str())).small()).wrap())
|
||||
})
|
||||
.inner
|
||||
.rect
|
||||
}
|
||||
FieldKind::Bool(b) => ui.checkbox(b, "").rect,
|
||||
FieldKind::Choice { options, sel } => {
|
||||
let cur = options.get(*sel).map(|o| o.0.clone()).unwrap_or_default();
|
||||
egui::ComboBox::from_id_salt(("form", fl.key.as_str()))
|
||||
.selected_text(tr(&cur))
|
||||
.width(240.0)
|
||||
.show_ui(ui, |ui| {
|
||||
for (i, (l, _)) in options.iter().enumerate() {
|
||||
ui.selectable_value(sel, i, tr(l));
|
||||
}
|
||||
})
|
||||
.response
|
||||
.rect
|
||||
}
|
||||
};
|
||||
regs.push((format!("form.field.{}", fl.key), r, fl.label.clone()));
|
||||
ui.end_row();
|
||||
}
|
||||
// Wide enough for a label column and a 240 px popup. Only the fields scroll, and only
|
||||
// vertically: OK and Cancel sit under the scroll area, so they are always in view (#978).
|
||||
super::dialogs::modal_fit(ctx, &f.title.clone(), vec2(480.0, h), t, |ui, _| {
|
||||
let footer = 14.0 + ui.spacing().interact_size.y + 2.0 * ui.spacing().item_spacing.y;
|
||||
let max_height = (super::dialogs::modal_space(ctx).y - 84.0 - footer).max(40.0);
|
||||
egui::ScrollArea::vertical().max_height(max_height).auto_shrink([false, true]).show(ui, |ui| {
|
||||
form_grid(ui, &mut f, &mut regs, &mut browse, &mut focused);
|
||||
});
|
||||
ui.add_space(14.0);
|
||||
ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,32 @@
|
|||
//! A form keeps its fields and its buttons inside the dialog, whatever a choice's text is (#978).
|
||||
use effectcraft_engine::Session;
|
||||
use effectcraft_ui_egui::{Dialog, EffectcraftApp, menus};
|
||||
use egui::{Rect, pos2, vec2};
|
||||
use egui_kittest::Harness;
|
||||
use serde_json::json;
|
||||
|
||||
fn rect(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect {
|
||||
let e = h.state().auto.find(id).unwrap_or_else(|| panic!("missing {id}"));
|
||||
Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3]))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_settings_fits_the_form_width() {
|
||||
let mut session = Session::default();
|
||||
session.prefs.disk.disk_cache_enabled = false;
|
||||
let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(session));
|
||||
h.run_steps(4);
|
||||
let ctx = h.ctx.clone();
|
||||
menus::invoke(h.state_mut(), &ctx, "file.projectSettings", json!({})).unwrap();
|
||||
h.run_steps(4);
|
||||
assert_eq!(h.state().dialog, Some(Dialog::Form));
|
||||
// The renderer's choice names the GPU (or its absence): longer than the 240 px column.
|
||||
let (ok, cancel, depth, renderer) = (rect(&h, "form.ok"), rect(&h, "form.cancel"), rect(&h, "form.field.bitDepth"), rect(&h, "form.field.renderer"));
|
||||
assert!((renderer.width() - depth.width()).abs() < 1.0, "a long choice does not widen its popup: {renderer:?} vs {depth:?}");
|
||||
// The dialog is 480 px wide and centred; nothing reaches past its 18 px margin.
|
||||
let dialog = Rect::from_center_size(ctx.content_rect().center(), vec2(480.0 - 36.0, f32::INFINITY));
|
||||
for (id, r) in [("form.ok", ok), ("form.cancel", cancel), ("form.field.renderer", renderer)] {
|
||||
assert!(r.left() >= dialog.left() - 1.0 && r.right() <= dialog.right() + 1.0, "{id} inside the dialog: {r:?} in {dialog:?}");
|
||||
}
|
||||
assert!(ok.top() > renderer.bottom(), "the buttons sit under the fields");
|
||||
}
|
||||
Loading…
Reference in New Issue