diff --git a/crates/ui-egui/src/panels/shortcut_editor.rs b/crates/ui-egui/src/panels/shortcut_editor.rs index c8948ab8..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]; @@ -237,7 +240,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 +442,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 +615,48 @@ 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 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()); 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));