From 4ec6a373b015d1456855b97800cbf4d7823f3959 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 10:35:59 +0200 Subject: [PATCH 1/3] Ribbon: Font Size is a box you type into, the sizes behind its arrow (fixes #932) --- crates/ui-egui/src/ribbon.rs | 109 ++++++++++++++-------- crates/ui-egui/tests/font_size_box.rs | 125 ++++++++++++++++++++++++++ crates/ui-egui/tests/keytips.rs | 5 +- 3 files changed, 203 insertions(+), 36 deletions(-) create mode 100644 crates/ui-egui/tests/font_size_box.rs diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 9c6d89eeb..99b3b01e2 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -380,8 +380,6 @@ pub(crate) enum ComboResult { None, /// A row was applied: clicked or Enter on the highlighted row. Picked(String), - /// Enter with free text that matches no row (e.g. a custom font size). - Entered(String), } /// Excel-style dropdown: a filter field (type to narrow the list) plus arrow/Enter selection. @@ -394,6 +392,24 @@ pub(crate) fn editable_combo( current_value: &str, items: &[(&str, String)], forced: bool, +) -> ComboResult { + combo_popup(ui, anchor, salt, current_value, items, forced, true) +} + +/// The dropdown of [`editable_combo`] without the filter field: the whole list, picked by click or +/// arrows and Enter. +fn list_combo(ui: &mut Ui, anchor: &egui::Response, salt: &'static str, current_value: &str, items: &[(&str, String)]) -> ComboResult { + combo_popup(ui, anchor, salt, current_value, items, false, false) +} + +fn combo_popup( + ui: &mut Ui, + anchor: &egui::Response, + salt: &'static str, + current_value: &str, + items: &[(&str, String)], + forced: bool, + filter: bool, ) -> ComboResult { let popup_id = anchor.id.with("popup"); let state_id = anchor.id.with(salt).with("__combo_state"); @@ -424,15 +440,17 @@ pub(crate) fn editable_combo( }); }); } - let text_id = anchor.id.with(salt).with("__combo_text"); - let te = ui.add(egui::TextEdit::singleline(&mut st.text).id(text_id).desired_width(220.0).hint_text("Type to filter…")); - if open_now && !te.has_focus() { - ui.ctx().memory_mut(|m| m.request_focus(text_id)); - // Select all once focus lands so typing replaces the filter. - if let Some(mut s) = egui::text_edit::TextEditState::load(ui.ctx(), text_id) { - let len = st.text.chars().count(); - s.cursor.set_char_range(Some(egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(len)))); - s.store(ui.ctx(), text_id); + if filter { + let text_id = anchor.id.with(salt).with("__combo_text"); + let te = ui.add(egui::TextEdit::singleline(&mut st.text).id(text_id).desired_width(220.0).hint_text("Type to filter…")); + if open_now && !te.has_focus() { + ui.ctx().memory_mut(|m| m.request_focus(text_id)); + // Select all once focus lands so typing replaces the filter. + if let Some(mut s) = egui::text_edit::TextEditState::load(ui.ctx(), text_id) { + let len = st.text.chars().count(); + s.cursor.set_char_range(Some(egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(len)))); + s.store(ui.ctx(), text_id); + } } } let lower = st.text.to_lowercase(); @@ -446,18 +464,13 @@ pub(crate) fn editable_combo( st.sel = (st.sel - 1).rem_euclid(n); } } - if enter { - if filtered.is_empty() { - let t = st.text.trim().to_string(); - if !t.is_empty() { - result = ComboResult::Entered(t); - } - } else { - let idx = filtered[st.sel.clamp(0, filtered.len() as i32 - 1) as usize]; - result = ComboResult::Picked(items[idx].0.to_string()); - } + if enter && !filtered.is_empty() { + let idx = filtered[st.sel.clamp(0, filtered.len() as i32 - 1) as usize]; + result = ComboResult::Picked(items[idx].0.to_string()); + } + if filter { + ui.separator(); } - ui.separator(); egui::ScrollArea::vertical().max_height(240.0).show(ui, |ui| { for i in &filtered { let (value, disp) = &items[*i]; @@ -1092,25 +1105,51 @@ pub const FONTS: &[&str] = &[ "Verdana", ]; +/// Excel's size box: type a size and press Enter to apply exactly that number (the command refuses +/// sizes outside 1 to 409), Escape or clicking away reverts; the arrow opens the standard sizes. fn size_combo(app: &mut SheetApp, ui: &mut Ui, st: &Style) { + let t = Tokens::get(ui.ctx()); let size = st.font.size; - let label = if size.fract() == 0.0 { format!("{}", size as i32) } else { format!("{size}") }; - let resp = ui.add_sized([52.0, 24.0], egui::Button::new(egui::RichText::new(&label).font(theme::ui_font(12.5)))); + let current = if size.fract() == 0.0 { format!("{}", size as i32) } else { format!("{size}") }; + let field = egui::Id::new("gridcraft.font_size_box"); + let draft = field.with("draft"); + let mut text = ui.data_mut(|d| d.get_temp::(draft)).unwrap_or_else(|| current.clone()); + let gap = std::mem::replace(&mut ui.spacing_mut().item_spacing.x, 0.0); + let out = egui::TextEdit::singleline(&mut text).id(field).font(theme::ui_font(12.5)).desired_width(30.0).show(ui); + let resp = out.response; control_keytip(app, ui, &resp, "home.fontSize"); - let forced = invoke(app, "home.fontSize"); + if invoke(app, "home.fontSize") { + ui.memory_mut(|m| m.request_focus(field)); + } + if resp.gained_focus() { + // Typing replaces the size, as in Excel, whether the box was clicked or reached by keytip. + let mut state = out.state; + state.cursor.set_char_range(Some(egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(text.chars().count())))); + state.store(ui.ctx(), field); + } + if resp.lost_focus() { + ui.data_mut(|d| d.remove::(draft)); + // Consume the Enter so the grid behind does not also move the active cell down. + if ui.input_mut(|i| i.consume_key(Modifiers::NONE, Key::Enter)) + && let Some(v) = text.trim().parse::().ok().filter(|v| v.is_finite()) + { + act(app, "home.fontSize", json!({"size": v})); + } + } else if resp.has_focus() { + ui.data_mut(|d| d.insert_temp(draft, text)); + } else { + ui.data_mut(|d| d.remove::(draft)); + } + let arrow = icon_button(ui, Icon::Chevron, t.text_dim, "Font Size", vec2(14.0, 24.0)); + ui.spacing_mut().item_spacing.x = gap; + arrow.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, ui.is_enabled(), "Font Size")); let sizes = [8.0, 9.0, 10.0, 11.0, 12.0, 14.0, 16.0, 18.0, 20.0, 22.0, 24.0, 26.0, 28.0, 36.0, 48.0, 72.0]; let labels: Vec = sizes.iter().map(|s| format!("{}", *s as i32)).collect(); let items: Vec<(&str, String)> = labels.iter().map(|s| (s.as_str(), s.clone())).collect(); - let current = if size.fract() == 0.0 { format!("{}", size as i32) } else { format!("{size}") }; - let mut apply = |s: &str| { - if let Ok(v) = s.parse::() { - act(app, "home.fontSize", json!({"size": v})); - } - }; - match editable_combo(ui, &resp, "size", ¤t, &items, forced) { - ComboResult::Picked(value) => apply(&value), - ComboResult::Entered(text) => apply(&text), - _ => {} + if let ComboResult::Picked(value) = list_combo(ui, &arrow, "size", ¤t, &items) + && let Ok(v) = value.parse::() + { + act(app, "home.fontSize", json!({"size": v})); } } diff --git a/crates/ui-egui/tests/font_size_box.rs b/crates/ui-egui/tests/font_size_box.rs new file mode 100644 index 000000000..0337543ad --- /dev/null +++ b/crates/ui-egui/tests/font_size_box.rs @@ -0,0 +1,125 @@ +//! Home ▸ Font Size is a box you type a size into, with the standard sizes behind its arrow (#932). + +use egui::{Event, Key, Modifiers, accesskit::Role}; +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_engine::core::{CellRef, Value}; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + let mut s = Session::new(); + s.new_workbook(); + s.execute("selection.set", json!({"range": "A1:B2"})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(s, Default::default()), + ); + h.run_steps(4); + h +} + +fn key(h: &mut Harness, key: Key) { + h.input_mut().events.extend([ + Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }, + Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE }, + ]); + h.run_steps(2); +} + +/// Alt, H, F, S focuses the size box, then `text` is typed and confirmed with Enter. +fn type_size(h: &mut Harness, text: &str) { + key(h, Key::F10); + for k in [Key::H, Key::F, Key::S] { + h.input_mut().events.push(Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }); + h.input_mut().events.push(Event::Text(k.name().to_ascii_lowercase())); + h.run_steps(2); + } + h.run_steps(3); + for c in text.chars() { + h.input_mut().events.push(Event::Text(c.to_string())); + h.run_steps(1); + } + key(h, Key::Enter); + h.run_steps(3); +} + +fn size(h: &Harness, cell: &str) -> f32 { + let d = h.state().session.active().unwrap(); + let sh = d.wb.active().unwrap(); + d.wb.styles.get(sh.style_id(CellRef::parse(cell).unwrap())).font.size +} + +#[test] +fn typed_size_is_applied_exactly_to_the_selection() { + for (typed, want) in [("6", 6.0), ("2", 2.0), ("10.5", 10.5)] { + let mut h = harness(); + type_size(&mut h, typed); + assert!(h.state().message.is_none(), "typing {typed} must not raise an error"); + for cell in ["A1", "B2"] { + assert_eq!(size(&h, cell), want, "typing {typed} + Enter sets {cell} to {want}"); + } + assert_eq!( + h.state().session.active().unwrap().wb.active().unwrap().value(CellRef::parse("A1").unwrap()), + Value::Empty, + "the digits go to the box, not the cell" + ); + } +} + +#[test] +fn out_of_range_or_not_a_number_changes_nothing() { + let mut h = harness(); + type_size(&mut h, "500"); + let (_, text) = h.state().message.clone().expect("500 is refused with a message"); + assert!(text.contains("between 1 and 409"), "{text}"); + assert_eq!(size(&h, "A1"), 11.0); + + let mut h = harness(); + type_size(&mut h, "abc"); + assert!(h.state().message.is_none()); + assert_eq!(size(&h, "A1"), 11.0, "text that is not a number reverts"); +} + +#[test] +fn escape_cancels_the_typed_size() { + let mut h = harness(); + key(&mut h, Key::F10); + for k in [Key::H, Key::F, Key::S] { + h.input_mut().events.push(Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }); + h.input_mut().events.push(Event::Text(k.name().to_ascii_lowercase())); + h.run_steps(2); + } + h.run_steps(3); + h.input_mut().events.push(Event::Text("6".into())); + h.run_steps(1); + key(&mut h, Key::Escape); + h.run_steps(3); + assert_eq!(size(&h, "A1"), 11.0); + assert!(h.get_all_by_role(Role::TextInput).any(|n| n.value().as_deref() == Some("11")), "the box shows the cell's size again"); +} + +#[test] +fn the_arrow_opens_the_plain_list_of_sizes() { + let mut h = harness(); + h.get_by_label("Font Size").click(); + h.run_steps(3); + assert!(h.query_by_label("Type to filter…").is_none(), "the size list has no filter field"); + // The grid's row headers carry the same numbers as labels: pick the list's button. + h.get_by_role_and_label(Role::Button, "16").click(); + h.run_steps(3); + assert_eq!(size(&h, "B2"), 16.0, "clicking a size applies it"); + + let mut h = harness(); + h.get_by_label("Font Size").click(); + h.run_steps(3); + key(&mut h, Key::ArrowDown); + key(&mut h, Key::Enter); + h.run_steps(3); + assert_eq!(size(&h, "A1"), 12.0, "Down from 11 then Enter picks 12"); +} diff --git a/crates/ui-egui/tests/keytips.rs b/crates/ui-egui/tests/keytips.rs index 24be742a5..1d96b32df 100644 --- a/crates/ui-egui/tests/keytips.rs +++ b/crates/ui-egui/tests/keytips.rs @@ -495,7 +495,10 @@ fn editable_combos_filter_by_typing_and_move_by_arrows() { key(&mut h, Key::Enter); h.run_steps(3); assert!(h.state().message.is_none() && h.state().toast.is_none(), "custom sizes must apply"); - assert_eq!(h.query_all_by_label("13.5").count(), 1, "the button must show the custom size"); + assert!( + h.get_all_by_role(egui::accesskit::Role::TextInput).any(|n| n.value().as_deref() == Some("13.5")), + "the size box must show the custom size" + ); // Closing without applying (Escape) and reopening must reset the filter to the full list. let mut h = harness(blank()); From 5a7fbfa5b8f4a2f697d82de8df966a36e13c6f96 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 10:46:07 +0200 Subject: [PATCH 2/3] Contributors: add my real name --- contributors/people.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index d522b84e0..b92d6bb77 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -65,3 +65,7 @@ sync_github_name = true [people.Silvertree2010] display_name = "Netter" sync_github_name = false + +[people.marcoarnulfo] +real_name = "Marco Arnulfo" +sync_github_name = false From 4bcc5c87d175b449931f56c4bccea5b32665418e Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 13 Oct 2026 20:09:54 +1100 Subject: [PATCH 3/3] Ribbon: Font Size box matches the Font name box, list drops under it The size box is one frame with the Font name box's fill, border, height and corners, with the text field and the dropdown arrow inside it (focus shows the selection stroke). The size list drops directly below the box and is as wide as it, instead of opening 220 pt wide under the arrow. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ut3hr3a13szpQdQ7gbYbfc --- crates/ui-egui/src/ribbon.rs | 46 +++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 99b3b01e2..d2d5fe101 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -393,13 +393,14 @@ pub(crate) fn editable_combo( items: &[(&str, String)], forced: bool, ) -> ComboResult { - combo_popup(ui, anchor, salt, current_value, items, forced, true) + combo_popup(ui, anchor, salt, current_value, items, forced, true, None) } /// The dropdown of [`editable_combo`] without the filter field: the whole list, picked by click or -/// arrows and Enter. -fn list_combo(ui: &mut Ui, anchor: &egui::Response, salt: &'static str, current_value: &str, items: &[(&str, String)]) -> ComboResult { - combo_popup(ui, anchor, salt, current_value, items, false, false) +/// arrows and Enter. It drops directly below `under` and is as wide as it, like Excel's Font Size +/// list under its box. +fn list_combo(ui: &mut Ui, anchor: &egui::Response, under: Rect, salt: &'static str, current_value: &str, items: &[(&str, String)]) -> ComboResult { + combo_popup(ui, anchor, salt, current_value, items, false, false, Some(under)) } fn combo_popup( @@ -410,6 +411,7 @@ fn combo_popup( items: &[(&str, String)], forced: bool, filter: bool, + under: Option, ) -> ComboResult { let popup_id = anchor.id.with("popup"); let state_id = anchor.id.with(salt).with("__combo_state"); @@ -424,7 +426,12 @@ fn combo_popup( } st.was_open = open_now; let mut result = ComboResult::None; - egui::Popup::menu(anchor).open_memory(popup_command(forced, anchor.clicked())).show(|ui| { + let mut popup = egui::Popup::menu(anchor).open_memory(popup_command(forced, anchor.clicked())); + if let Some(r) = under { + popup = popup.anchor(r).align(egui::RectAlign::BOTTOM_START).width(r.width()); + } + let row_w = under.map_or(220.0, |r| r.width() - 12.0); + popup.show(|ui| { // Arrows and Enter navigate the list, not the filter field's cursor. Capture them and // strip them from the input so the TextEdit never sees them. let (mut down, mut up, mut enter) = (false, false, false); @@ -474,7 +481,7 @@ fn combo_popup( egui::ScrollArea::vertical().max_height(240.0).show(ui, |ui| { for i in &filtered { let (value, disp) = &items[*i]; - let row = ui.add(egui::Button::selectable(*i as i32 == st.sel, egui::RichText::new(disp)).min_size(vec2(220.0, 20.0))); + let row = ui.add(egui::Button::selectable(*i as i32 == st.sel, egui::RichText::new(disp)).min_size(vec2(row_w, 20.0))); if row.clicked() { result = ComboResult::Picked(value.to_string()); } @@ -1114,8 +1121,21 @@ fn size_combo(app: &mut SheetApp, ui: &mut Ui, st: &Style) { let field = egui::Id::new("gridcraft.font_size_box"); let draft = field.with("draft"); let mut text = ui.data_mut(|d| d.get_temp::(draft)).unwrap_or_else(|| current.clone()); - let gap = std::mem::replace(&mut ui.spacing_mut().item_spacing.x, 0.0); - let out = egui::TextEdit::singleline(&mut text).id(field).font(theme::ui_font(12.5)).desired_width(30.0).show(ui); + // One frame like the Font name box beside it (same fill, border, height and corners), with the + // text field and the dropdown arrow inside it, as in Excel. + let (frame, _) = ui.allocate_exact_size(vec2(52.0, 24.0), egui::Sense::hover()); + // Painted once the field's focus and hover are known, but beneath the field and the arrow. + let bg = ui.painter().add(egui::Shape::Noop); + let text_rect = Rect::from_min_max(frame.min, egui::pos2(frame.right() - 16.0, frame.bottom())); + let arrow_rect = Rect::from_min_max(egui::pos2(frame.right() - 16.0, frame.top() + 1.0), egui::pos2(frame.right() - 1.0, frame.bottom() - 1.0)); + let mut field_ui = + ui.new_child(egui::UiBuilder::new().max_rect(text_rect.shrink2(vec2(6.0, 0.0))).layout(egui::Layout::left_to_right(egui::Align::Center))); + let out = egui::TextEdit::singleline(&mut text) + .id(field) + .font(theme::ui_font(12.5)) + .frame(egui::Frame::NONE) + .desired_width(text_rect.width() - 12.0) + .show(&mut field_ui); let resp = out.response; control_keytip(app, ui, &resp, "home.fontSize"); if invoke(app, "home.fontSize") { @@ -1140,13 +1160,17 @@ fn size_combo(app: &mut SheetApp, ui: &mut Ui, st: &Style) { } else { ui.data_mut(|d| d.remove::(draft)); } - let arrow = icon_button(ui, Icon::Chevron, t.text_dim, "Font Size", vec2(14.0, 24.0)); - ui.spacing_mut().item_spacing.x = gap; + let mut arrow_ui = ui.new_child(egui::UiBuilder::new().max_rect(arrow_rect)); + let arrow = icon_button(&mut arrow_ui, Icon::Chevron, t.text_dim, "Font Size", arrow_rect.size()); + let visuals = ui.style().visuals.clone(); + let looks = if resp.has_focus() || ui.rect_contains_pointer(frame) { &visuals.widgets.hovered } else { &visuals.widgets.inactive }; + let stroke = if resp.has_focus() { visuals.selection.stroke } else { looks.bg_stroke }; + ui.painter().set(bg, egui::epaint::RectShape::new(frame, looks.corner_radius, looks.weak_bg_fill, stroke, egui::StrokeKind::Inside)); arrow.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, ui.is_enabled(), "Font Size")); let sizes = [8.0, 9.0, 10.0, 11.0, 12.0, 14.0, 16.0, 18.0, 20.0, 22.0, 24.0, 26.0, 28.0, 36.0, 48.0, 72.0]; let labels: Vec = sizes.iter().map(|s| format!("{}", *s as i32)).collect(); let items: Vec<(&str, String)> = labels.iter().map(|s| (s.as_str(), s.clone())).collect(); - if let ComboResult::Picked(value) = list_combo(ui, &arrow, "size", ¤t, &items) + if let ComboResult::Picked(value) = list_combo(ui, &arrow, frame, "size", ¤t, &items) && let Ok(v) = value.parse::() { act(app, "home.fontSize", json!({"size": v}));