diff --git a/crates/ui-egui/src/type_font_picker_tests.rs b/crates/ui-egui/src/type_font_picker_tests.rs index a9b1d23590..12c1dec020 100644 --- a/crates/ui-egui/src/type_font_picker_tests.rs +++ b/crates/ui-egui/src/type_font_picker_tests.rs @@ -696,7 +696,7 @@ fn font_menu_rows_search_both_names_and_sort_by_label() { let labels: HashMap = [("MS Gothic", "ＭＳ ゴシック"), ("Microsoft YaHei", "微软雅黑")].iter().map(|(a, b)| (a.to_string(), b.to_string())).collect(); let rows = |labels: Option<&HashMap>, q: &str| -> Vec<(String, String)> { - super::font_menu_rows(&families, labels, q, &[]).into_iter().map(|(f, l)| (f.clone(), l.to_string())).collect() + super::font_menu_rows(&families, labels, q, &[]).0.into_iter().map(|(f, l)| (f.clone(), l.to_string())).collect() }; let english = rows(None, ""); assert!(english.iter().all(|(f, l)| f == l)); @@ -707,26 +707,121 @@ fn font_menu_rows_search_both_names_and_sort_by_label() { assert!(rows(Some(&labels), "nothing").is_empty()); } -/// Preferences ▸ Type ▸ Recent Fonts: recently used families move to the top (most recent first, -/// once each); a query drops the hoisting so the list stays alphabetical while searching. +/// Preferences ▸ Type ▸ Recent Fonts: as in Photoshop, recently used families head the menu in +/// their own group (most recent first, once each) and keep their alphabetical place below it; a +/// query lists each match once, alphabetically. #[test] -fn recent_fonts_are_hoisted_to_the_top() { +fn recent_fonts_head_the_menu_and_keep_their_place() { let families: Vec = ["Arial", "Baskerville", "Courier", "Zeta Sans"].iter().map(|s| s.to_string()).collect(); - let recents: Vec = vec!["Zeta Sans".into(), "Courier".into()]; - let rows = |q: &str| -> Vec { super::font_menu_rows(&families, None, q, &recents).into_iter().map(|(f, _)| f.clone()).collect() }; - assert_eq!(rows(""), ["Zeta Sans", "Courier", "Arial", "Baskerville"], "recents hoist, most recent first, once each"); - assert_eq!(rows("a"), ["Arial", "Baskerville", "Zeta Sans"], "a query drops the hoisting"); + let recents: Vec = vec!["Zeta Sans".into(), "Courier".into(), "Gone".into()]; + let rows = |q: &str| -> (Vec, usize) { + let (rows, n) = super::font_menu_rows(&families, None, q, &recents); + (rows.into_iter().map(|(f, _)| f.clone()).collect(), n) + }; + assert_eq!( + rows(""), + (vec!["Zeta Sans".into(), "Courier".into(), "Arial".into(), "Baskerville".into(), "Courier".into(), "Zeta Sans".into()], 2), + "a recent group (no uninstalled family), then the whole alphabetical list" + ); + assert_eq!(rows("a"), (vec!["Arial".into(), "Baskerville".into(), "Zeta Sans".into()], 0), "a query lists each match once"); } -/// The menu itself draws the hoisted recents first. +/// The menu itself draws the recent group first, and the recent families again in their place. #[test] fn recent_fonts_appear_first_in_the_menu() { let mut h = harness_with_recents(FontPreview::Medium, vec!["Zeta Sans".into(), "Family 05".into()]); open(&mut h); - let zeta = h.get_by_label("Zeta Sans").rect().top(); - let five = h.get_by_label("Family 05").rect().top(); - let six = h.get_by_label("Family 06").rect().top(); - assert!(zeta < five && five < six, "recents come first, in order: {zeta} {five} {six}"); + let tops = |h: &Harness<'static, State>, label: &str| -> Vec { + let mut t: Vec = h.query_all_by_label(label).map(|n| n.rect().top()).collect(); + t.sort_by(f32::total_cmp); + t + }; + let (zeta, five, first, six) = (tops(&h, "Zeta Sans"), tops(&h, "Family 05"), tops(&h, "Family 00"), tops(&h, "Family 06")); + assert_eq!(five.len(), 2, "a recent family is listed twice: {five:?}"); + assert!(zeta[0] < five[0] && five[0] < first[0], "the recent group comes first, in order: {zeta:?} {five:?} {first:?}"); + assert!(first[0] < five[1] && five[1] < six[0], "and keeps its alphabetical place: {five:?} {six:?}"); + assert!(first[0] - five[0] > five[0] - zeta[0], "a divider ends the recent group: {zeta:?} {five:?} {first:?}"); +} + +/// The open menu's arrows walk on through the list although each step records its family as +/// the most recent one (CodeRabbit on #3559): the recent group keeps its order while the menu +/// is open, and a step from a recent family's lower copy goes on from there. +#[test] +fn open_menu_arrows_walk_on_through_recent_fonts() { + let mut h = harness_with_recents(FontPreview::Off, vec!["Family 03".into()]); + open(&mut h); + let mut seen = vec![h.state().current.clone()]; + for _ in 0..5 { + h.key_press(Key::ArrowDown); + h.run(); + let now = h.state().current.clone(); + // The options bar records each applied family as the most recent one. + h.state_mut().recents.retain(|f| *f != now); + h.state_mut().recents.insert(0, now.clone()); + h.run(); + seen.push(now); + } + assert_eq!(seen, ["Family 00", "Family 01", "Family 02", "Family 03", "Family 04", "Family 05"], "{seen:?}"); + h.key_press(Key::ArrowUp); + h.run(); + assert_eq!(h.state().current, "Family 04", "↑ steps back"); +} + +/// Opened on a family that is also a recent one (the usual case: every pick records it), the +/// arrows start from its alphabetical place, and only that row is marked. +#[test] +fn open_menu_on_a_recent_font_steps_through_the_alphabet() { + use egui_kittest::kittest::NodeT; + for (key, want) in [(Key::ArrowDown, "Family 06"), (Key::ArrowUp, "Family 04")] { + let mut h = harness_with_recents(FontPreview::Off, vec!["Family 05".into(), "Family 09".into()]); + h.state_mut().current = "Family 05".into(); + h.run(); + open(&mut h); + let marked = |h: &Harness<'static, State>, label: &str| { + h.query_all_by_label(label).filter(|n| n.accesskit_node().toggled() == Some(egui::accesskit::Toggled::True)).count() + }; + assert_eq!(marked(&h, "Family 05"), 1, "one row marked"); + h.key_press(key); + h.run(); + assert_eq!(h.state().current, want); + h.key_press(Key::ArrowDown); + h.key_press(Key::ArrowUp); + h.run(); + assert_eq!(h.state().current, want, "and walks on from there"); + } +} + +/// The same through the real options bar, where the pick itself records the recent family. +#[test] +fn options_bar_menu_arrows_walk_on_while_recording_recent_fonts() { + use crate::{PhotocraftApp, Services}; + use serde_json::json; + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Services::default()); + app.run("file.new", json!({"width": 200, "height": 100})).unwrap(); + app.run("type.create", json!({"text": "Sample", "font": "Inter", "size": 18})).unwrap(); + let mut h = Harness::builder().with_size(vec2(900.0, 700.0)).build_ui_state( + |ui, app| { + if ui.ctx().fonts(|f| f.families().contains(&egui::FontFamily::Name("medium".into()))) { + ui.horizontal(|ui| crate::type_tool::options_bar(app, ui)); + } + }, + app, + ); + crate::PhotocraftApp::setup_context(&h.ctx, Default::default()); + h.run_steps(4); + h.query_all_by_role(Role::ComboBox).next().unwrap().click(); + h.run(); + let mut seen = vec![h.state().ui.tool_options.type_font.clone()]; + for _ in 0..4 { + h.key_press(Key::ArrowDown); + h.run(); + seen.push(h.state().ui.tool_options.type_font.clone()); + } + let mut once = seen.clone(); + once.sort(); + once.dedup(); + assert_eq!(once.len(), seen.len(), "each ↓ reaches a new family: {seen:?}"); + assert_eq!(h.state().ui.recent_fonts.first(), seen.last(), "the picks are recorded"); } #[test] diff --git a/crates/ui-egui/src/type_tool.rs b/crates/ui-egui/src/type_tool.rs index 119310672f..0bfda45226 100644 --- a/crates/ui-egui/src/type_tool.rs +++ b/crates/ui-egui/src/type_tool.rs @@ -1468,10 +1468,16 @@ fn localized_families() -> Arc> { /// The font menu's rows: each family with the label it shows (its localized name when `labels` /// has one, else the family), matching `query` (lowercase) by either name. With localized labels -/// the rows sort by label, so the menu stays alphabetical as shown. Recently used families are -/// hoisted to the top, most recent first (Preferences ▸ Type ▸ Recent Fonts); a query hides the -/// hoisting so the list stays strictly alphabetical while searching. -fn font_menu_rows<'a>(families: &'a [String], labels: Option<&'a HashMap>, query: &str, recents: &[String]) -> Vec<(&'a String, &'a str)> { +/// the rows sort by label, so the menu stays alphabetical as shown. As in Photoshop, recently +/// used families also head the menu in their own group, most recent first (Preferences ▸ Type ▸ +/// Recent Fonts), and keep their alphabetical place below it; a query shows each match once. +/// Returns the rows and how many of them form the recent group. +fn font_menu_rows<'a>( + families: &'a [String], + labels: Option<&'a HashMap>, + query: &str, + recents: &[String], +) -> (Vec<(&'a String, &'a str)>, usize) { let label = |f: &'a String| labels.and_then(|l| l.get(f)).map_or(f.as_str(), String::as_str); let mut rows: Vec<(&String, &str)> = families .iter() @@ -1481,17 +1487,12 @@ fn font_menu_rows<'a>(families: &'a [String], labels: Option<&'a HashMap = recents.iter().filter_map(|r| rows.iter().find(|(f, _)| *f == r).copied()).collect(); + let n = recent.len(); + (recent.into_iter().chain(rows).collect(), n) } fn weight_name(w: f32) -> &'static str { @@ -1701,6 +1702,8 @@ pub(crate) fn family_picker_in( let mut picked = false; let search_id = ui.id().with(("font-search", salt)); let focus_id = search_id.with("focus"); + let recents_id = search_id.with("recents"); + let row_id = search_id.with("row"); // The list opens on the press, so a press-drag-release picks a font too (#2735). let (press, close) = crate::press_menu::PressCombo::before(ui, salt); let labels: Option>> = ui.ctx().data(|d| d.get_temp(font_labels_id())); @@ -1739,10 +1742,21 @@ pub(crate) fn family_picker_in( d.insert_temp(search_id, q.clone()); }); let ql = q.to_lowercase(); - let rows = font_menu_rows(families, labels, &ql, recents); + // The recent group stays as it was when the menu opened: each arrow step applies (and so + // records) a family, and moving it to the top would send the next step back up. + let open_recents: Vec = ui.data_mut(|d| d.get_temp_mut_or_insert_with(recents_id, || recents.to_vec()).clone()); + let (rows, n_recent) = font_menu_rows(families, labels, &ql, &open_recents); + // A recent family is listed twice. The current family's row is the one the last arrow + // selected while it still shows that family, else its alphabetical copy, so the arrows + // step through the alphabet as on the closed field. + let mut selected = ui + .data(|d| d.get_temp::(row_id)) + .filter(|&i| rows.get(i).is_some_and(|(f, _)| *f == current)) + .or_else(|| rows.iter().skip(n_recent).position(|(f, _)| *f == current).map(|i| i + n_recent)) + .or_else(|| rows.iter().position(|(f, _)| *f == current)); + let mut stepped = None; if down || up { - let index = rows.iter().position(|(f, _)| *f == current); - let next = match index { + let next = match selected { Some(i) if down => i.saturating_add(1).min(rows.len().saturating_sub(1)), Some(i) => i.saturating_sub(1), None if down => 0, @@ -1753,6 +1767,9 @@ pub(crate) fn family_picker_in( current.clone_from(f); // A served family not fetched yet: start now, as a click does. photocraft_text::served::request(f); + stepped = Some(next); + selected = Some(next); + ui.data_mut(|d| d.insert_temp(row_id, next)); } } let accept = enter && rows.iter().any(|(f, _)| *f == current); @@ -1768,18 +1785,22 @@ pub(crate) fn family_picker_in( // stays fixed at the top of the popup. let mut hovered: Option = None; egui::ScrollArea::vertical().show(ui, |ui| { - for (f, label) in rows { + for (i, (f, label)) in rows.into_iter().enumerate() { + if i == n_recent && n_recent > 0 { + ui.separator(); + } // Highlighted by its fill alone, as Photoshop's font menu is. A selectable button lays // out its frame's stroke only once hovered or selected, so with the theme's stroke the // row under the pointer (and the selected one) grew and pushed the rows below down: - // the list jumped as the pointer moved (#7406). - let mut button = egui::Button::selectable(f == current, label).truncate().stroke(egui::Stroke::NONE); + // the list jumped as the pointer moved (#7406). One row only, though a recent + // family is listed twice. + let mut button = egui::Button::selectable(selected == Some(i), label).truncate().stroke(egui::Stroke::NONE); if let Some((size, [_, h])) = preview { button = button .right_text(egui::Atom::custom(ui.id().with(("font-sample", f)), egui::vec2(crate::font_preview::slot_width(ui.ctx(), f, size), h))); } let response = ui.add_sized([330.0, row_h], button); - if (down || up) && f == current { + if stepped == Some(i) { response.scroll_to_me(Some(egui::Align::Center)); } if let Some((size, _)) = preview @@ -1816,7 +1837,11 @@ pub(crate) fn family_picker_in( }); press.after(&response.response); if !egui::ComboBox::is_open(ui.ctx(), response.response.id) { - ui.data_mut(|d| d.remove::(focus_id)); + ui.data_mut(|d| { + d.remove::(focus_id); + d.remove::>(recents_id); + d.remove::(row_id); + }); } let hovered_font = response.inner.unwrap_or_default(); @@ -1839,9 +1864,9 @@ pub(crate) fn family_picker_in( ui.memory_mut(|m| m.move_focus(egui::FocusDirection::None)); } // The wheel's step: a negative notch is the next family. Steps use a stable order (no - // recents hoisting), so recording the picked family cannot make the next step jump back. + // recent group), so recording the picked family cannot make the next step jump back. let notches = f32::from(u8::from(up)) - f32::from(u8::from(down)); - let rows = font_menu_rows(families, labels, "", &[]); + let (rows, _) = font_menu_rows(families, labels, "", &[]); if let Some(i) = rows.iter().position(|(f, _)| *f == current) && let Some(n) = wheel_family_index(i, rows.len(), notches) && let Some((f, _)) = rows.get(n) @@ -1861,10 +1886,10 @@ pub(crate) fn family_picker_in( ui.input_mut(|i| i.smooth_scroll_delta = egui::Vec2::ZERO); } let notches = crate::widgets::wheel_notches_of(ui, response.response.id.with("family-wheel"), hovered, 1.0); - // The wheel steps in a stable order: hoisting the family it just picked as recent + // The wheel steps in a stable order: a recent group holding the family it just picked // would change the order between notches and can jump back to the previous family. // The open menu still shows the recent families first. - let rows = font_menu_rows(families, labels, "", &[]); + let (rows, _) = font_menu_rows(families, labels, "", &[]); if let Some(i) = rows.iter().position(|(f, _)| *f == current) && let Some(n) = wheel_family_index(i, rows.len(), notches) && let Some((f, _)) = rows.get(n)