From 07fe9cb1dc57efe1f5454d509a242658a8d84636 Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:34:56 +0200 Subject: [PATCH 1/3] Keyboard Shortcuts: keep the OK button in view [skip ci] The editor's content was a little taller than its dialog, so the outer scroll area clipped the OK button and its scrollbar covered the right edge. The dialog now has no outer scroll area: the command list takes whatever height is left, so the button row always fits, in small windows too. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/shortcut_editor.rs | 92 +++++++++++++------- 1 file changed, 60 insertions(+), 32 deletions(-) diff --git a/crates/ui-egui/src/panels/shortcut_editor.rs b/crates/ui-egui/src/panels/shortcut_editor.rs index c8948ab8..f12fe4ef 100644 --- a/crates/ui-egui/src/panels/shortcut_editor.rs +++ b/crates/ui-egui/src/panels/shortcut_editor.rs @@ -237,7 +237,9 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { } } - super::dialogs::modal(ctx, "Keyboard Shortcuts", vec2(1000.0, 720.0), t, |ui| { + // No outer scroll area: the command list takes the height left over, so OK stays in view. + super::dialogs::modal_fit(ctx, "Keyboard Shortcuts", vec2(1000.0, 760.0), t, |ui, height| { + let top = ui.cursor().top(); // ---- preset bar ui.horizontal(|ui| { ui.label(tr("Preset:")); @@ -437,42 +439,49 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { q.split_whitespace().all(|w| hay.contains(w)) }) .collect(); - ui.allocate_ui_with_layout(vec2(ui.available_width(), 270.0), egui::Layout::left_to_right(egui::Align::Min), |ui| { - ui.set_height(270.0); + let footer = 6.0 + ui.spacing().interact_size.y + ui.spacing().item_spacing.y * 2.0; + let list_h = (height - (ui.cursor().top() - top) - footer).max(140.0); + ui.allocate_ui_with_layout(vec2(ui.available_width(), list_h), egui::Layout::left_to_right(egui::Align::Min), |ui| { + ui.set_height(list_h); // Command list. ui.vertical(|ui| { ui.set_width(620.0); let row_h = 20.0; - egui::ScrollArea::vertical().id_salt("sc-rows").max_height(230.0).auto_shrink([false, false]).show_rows(ui, row_h, rows.len(), |ui, range| { - for i in range { - let b = rows[i]; - let keys = table.keys.get(&b.key).cloned().unwrap_or_default(); - let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), row_h), Sense::click()); - let sel = st.selected.as_deref() == Some(b.key.as_str()); - if sel { - ui.painter().rect_filled(r, 0.0, t.row_selected); - } else if i % 2 == 1 { - ui.painter().rect_filled(r, 0.0, t.row_alt); + egui::ScrollArea::vertical().id_salt("sc-rows").max_height(list_h - 40.0).auto_shrink([false, false]).show_rows( + ui, + row_h, + rows.len(), + |ui, range| { + for i in range { + let b = rows[i]; + let keys = table.keys.get(&b.key).cloned().unwrap_or_default(); + let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), row_h), Sense::click()); + let sel = st.selected.as_deref() == Some(b.key.as_str()); + if sel { + ui.painter().rect_filled(r, 0.0, t.row_selected); + } else if i % 2 == 1 { + ui.painter().rect_filled(r, 0.0, t.row_alt); + } + let pp = ui.painter(); + pp.text(pos2(r.min.x + 6.0, r.center().y), Align2::LEFT_CENTER, &b.label, Tokens::ui(11.5), t.text); + pp.text(pos2(r.min.x + 230.0, r.center().y), Align2::LEFT_CENTER, b.path.join(" > "), Tokens::ui(10.5), t.text_faint); + let ks: Vec = keys.iter().map(|k| crate::menus::shortcut_text(k)).collect(); + let col = if b.scope == APP_SCOPE { APP_COLOR } else { PANEL_COLOR }; + pp.text( + pos2(r.max.x - 8.0, r.center().y), + Align2::RIGHT_CENTER, + ks.join(", "), + Tokens::ui(11.5), + if keys.is_empty() { t.text_faint } else { col }, + ); + app.auto.add(&format!("shortcuts.row.{i}"), r, &b.label); + if resp.clicked() { + st.selected = Some(b.key.clone()); + st.pending.clear(); + } } - let pp = ui.painter(); - pp.text(pos2(r.min.x + 6.0, r.center().y), Align2::LEFT_CENTER, &b.label, Tokens::ui(11.5), t.text); - pp.text(pos2(r.min.x + 230.0, r.center().y), Align2::LEFT_CENTER, b.path.join(" > "), Tokens::ui(10.5), t.text_faint); - let ks: Vec = keys.iter().map(|k| crate::menus::shortcut_text(k)).collect(); - let col = if b.scope == APP_SCOPE { APP_COLOR } else { PANEL_COLOR }; - pp.text( - pos2(r.max.x - 8.0, r.center().y), - Align2::RIGHT_CENTER, - ks.join(", "), - Tokens::ui(11.5), - if keys.is_empty() { t.text_faint } else { col }, - ); - app.auto.add(&format!("shortcuts.row.{i}"), r, &b.label); - if resp.clicked() { - st.selected = Some(b.key.clone()); - st.pending.clear(); - } - } - }); + }, + ); ui.label(RichText::new(format!("{} of {} commands", rows.len(), table.bindables.len())).color(t.text_faint)); }); ui.separator(); @@ -603,6 +612,25 @@ mod tests { out.textures_delta.clear(); } + #[test] + fn the_ok_button_stays_inside_the_dialog_in_a_small_window() { + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + let ctx = egui::Context::default(); + crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); + open(&mut app); + for size in [vec2(1680.0, 1020.0), vec2(1280.0, 720.0)] { + for _ in 0..3 { + app.auto.begin_frame(); + let input = egui::RawInput { screen_rect: Some(Rect::from_min_size(pos2(0.0, 0.0), size)), ..Default::default() }; + let mut out = ctx.run_ui(input, |ui| crate::panels::dialogs::show(&mut app, ui.ctx())); + out.textures_delta.clear(); + } + let [_, y, _, h] = app.auto.find("shortcuts.close").expect("OK button").rect; + // The window less the modal's margin and its frame padding. + assert!(y + h <= size.y - 24.0 - 18.0 + 0.5, "OK ends at {} in a {size:?} window", y + h); + } + } + #[test] fn recording_a_shortcut_does_not_also_type_the_key_into_the_field() { let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); From 00afd27dcadb5985f1c9d8518c7c8a4e1ae9f8f9 Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:42:40 +0200 Subject: [PATCH 2/3] Keyboard Shortcuts: Enter is OK [skip ci] Enter closes the editor like its OK button, also while the search field has focus. It stays with the shortcut recorder (Enter can be recorded), the preset rename field and an open preset menu. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/shortcut_editor.rs | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/ui-egui/src/panels/shortcut_editor.rs b/crates/ui-egui/src/panels/shortcut_editor.rs index f12fe4ef..b0e3d7f2 100644 --- a/crates/ui-egui/src/panels/shortcut_editor.rs +++ b/crates/ui-egui/src/panels/shortcut_editor.rs @@ -215,6 +215,9 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { } } } + // Enter is OK, unless it belongs to the rename field or an open preset menu. A recorded Enter + // never gets here: the recorder took the frame's key events above. + close |= st.rename.is_none() && !egui::Popup::is_any_open(ctx) && ctx.input(|i| i.modifiers.is_none() && i.key_pressed(egui::Key::Enter)); // Modifiers: toggles, or the ones held on the keyboard. let held = ctx.input(|i| i.modifiers); let held = [held.ctrl && cfg!(target_os = "macos"), held.command, held.alt, held.shift]; @@ -631,6 +634,29 @@ mod tests { } } + #[test] + fn enter_is_ok_except_while_recording_or_renaming() { + let enter = || egui::Event::Key { key: egui::Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }; + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + let ctx = egui::Context::default(); + crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); + // Recording: Enter is the recorded key. + open(&mut app); + app.dialog_state.shortcuts.recording = true; + frame(&mut app, &ctx, vec![enter()]); + assert_eq!(app.dialog, Some(Dialog::Shortcuts)); + assert_eq!(app.dialog_state.shortcuts.pending, "Enter"); + // Renaming a preset: Enter belongs to the name field. + app.dialog_state.shortcuts.rename = Some("Mine".into()); + frame(&mut app, &ctx, vec![enter()]); + assert_eq!(app.dialog, Some(Dialog::Shortcuts)); + // Otherwise it closes the editor, also with the search field focused. + app.dialog_state.shortcuts.rename = None; + frame(&mut app, &ctx, vec![]); + frame(&mut app, &ctx, vec![enter()]); + assert_eq!(app.dialog, None); + } + #[test] fn recording_a_shortcut_does_not_also_type_the_key_into_the_field() { let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); From 5789c1c7d8e5b56dd2babaca9d06ca84a923cd4d Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:49:05 +0200 Subject: [PATCH 3/3] Scroll bars: a handle you can see [skip ci] The handle was filled with the colour of its own track (both the field background), so it only showed while hovered or dragged. It now takes the text colour, in every scroll area. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/theme.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index abc6f576..c8a4e361 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -507,6 +507,9 @@ pub fn apply_visuals(ctx: &egui::Context, t: &Tokens) { // Scroll bars always show (egui's float in only over the list), so long menus and lists // can be dragged without a mouse wheel or touchpad (#269). s.spacing.scroll = egui::style::ScrollStyle::solid(); + // The handle takes the text colour: the fill it would otherwise get is the colour of its + // track (both are the field background), which left it invisible until hovered. + s.spacing.scroll.foreground_color = true; s.text_styles.insert(TextStyle::Body, FontId::new(12.0, FontFamily::Proportional)); s.text_styles.insert(TextStyle::Button, FontId::new(12.0, FontFamily::Proportional)); s.text_styles.insert(TextStyle::Small, FontId::new(11.0, FontFamily::Proportional));