From 29bec7f52888c6acb305478e67c31be37a5d3edd Mon Sep 17 00:00:00 2001 From: clover caruso Date: Fri, 2 Oct 2026 18:58:23 -0700 Subject: [PATCH] fix: menus scroll only once they outgrow the window A menu capped its list at twelve rows, so the notebook menu scrolled with half the window free. Menus now run to the window less its popup margin, after flipping or shifting to fit; the palette and dialog lists keep the cap. Assisted-by: claude-opus-5.5 --- crates/ui/src/popup.rs | 20 +++++++++++++++----- crates/ui/src/tests.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/ui/src/popup.rs b/crates/ui/src/popup.rs index a5c1a8ff27a33701889926bd8738aee0bda7df61..c68715a0753fbe01ef54978e9ec07e0f32455954 100644 --- a/crates/ui/src/popup.rs +++ b/crates/ui/src/popup.rs @@ -20,7 +20,7 @@ pub(crate) const PAD: f32 = 4.0; /// A palette's rows and filter field, and Snowbound's more compact menu rows. const ROW: f32 = 26.0; pub(crate) const MENU_ROW: f32 = 22.0; -/// Rows a list shows before it scrolls. +/// Rows a palette or a dialog's list shows before it scrolls; a menu runs to the window's edge. const ROWS: f32 = 12.0; /// How long the pointer rests on a row before its submenu opens, as Windows waits by default. const SUBMENU_DELAY: Duration = Duration::from_millis(200); @@ -115,6 +115,12 @@ pub fn menu( .map_or(0.0, |badge| crate::badge_width(ui, badge) + ICON_GAP) }) .collect(); + let window = ui.rect(Id::ROOT).map_or(0.0, |window| window[3]); + let field = if filter.is_some() { + ROW + style.pad + } else { + 0.0 + }; let mut measure = |text| ui.texts.label(text, style.font_size, ui.frame).size[0]; let [text, shortcut] = items @@ -140,7 +146,7 @@ pub fn menu( } else { 0.0 } + if icons(items) { ICON + ICON_GAP } else { 0.0 } - + if items.len() as f32 > ROWS { + + if items.len() as f32 * style.row > window - 4.0 * PAD - field { GUTTER } else { 0.0 @@ -510,9 +516,13 @@ fn choose( 0.0 }; let content = matches.count() as f32 * row + matches.space_before(matches.count()); - let view = content - .max(row) - .min((ROWS * row).min(window - 4.0 * PAD - field).max(row)); + let room = window - 4.0 * PAD - field; + let most = if role == Role::Menu { + room + } else { + (ROWS * row).min(room) + }; + let view = content.max(row).min(most.max(row)); let chosen = if matches.count() == 0 { ui.leaf( "empty", diff --git a/crates/ui/src/tests.rs b/crates/ui/src/tests.rs index d2748f686f5f10a09b4dc2b5b0443cf640e41577..cf47717c99d6d3d91f4bddaca44d2cec11dd3f93 100644 --- a/crates/ui/src/tests.rs +++ b/crates/ui/src/tests.rs @@ -2024,6 +2024,36 @@ fn keys_move_the_selection_and_the_view_eases_after_it() { assert_eq!((selected, row_top(&ui, 0)), (Some(0), Some(0.0))); } +#[test] +fn a_menu_runs_to_the_window_edge_before_it_scrolls() { + let names: Vec = (0..20).map(|index| format!("Item {index}")).collect(); + let items: Vec<_> = names + .iter() + .map(|text| popup::Item { + text, + ..popup::Item::default() + }) + .collect(); + let build = |ui: &mut Ui, height: f32| { + sized_frame(ui, [400.0, height], |ui| { + popup::menu(ui, menu_id(), BELOW, &items, None); + }) + }; + let mut ui = Ui::new(Theme::dark(), DOUBLE_CLICK); + build(&mut ui, 800.0); + ui.open_popup(menu_id()); + for _ in 0..3 { + build(&mut ui, 800.0); + } + let bar = |ui: &Ui| ui.rect(menu_id().child("rows").child("bar")); + assert!(bar(&ui).is_none(), "twenty rows fit a tall window whole"); + assert!(ui.rect(menu_id().child("rows").child(19u64)).is_some()); + for _ in 0..3 { + build(&mut ui, 300.0); + } + assert!(bar(&ui).is_some(), "a short window scrolls them"); +} + #[test] fn a_long_menu_scrolls_by_dragging_its_thumb() { let names: Vec = (0..40).map(|index| format!("Item {index}")).collect(); -- 2.54.0