Merge pull request #1116 from Ni-zav/fix/font-menu-highlight-rebuild-s20261011T0359Z-c8a1

fix(fonts): preserve highlighted font across menu list refreshes

crates/ui-egui/src/font_menu.rs

@@ -131,6 +131,13 @@ impl Row {
     }
 }
 
+/// Keep keyboard/pointer focus on the same font after filtering, loading new fonts or
+/// expanding a family's styles. A row index on its own can now name a different font.
+fn remap_highlight(before: &[Row], selected: Option<usize>, after: &[Row]) -> Option<usize> {
+    let row = before.get(selected?)?;
+    after.iter().position(|candidate| candidate == row)
+}
+
 /// What the rows were listed for: the query, the script, kind and favourites filters, Show Font
 /// Names in English, the expanded families, the favourites and the font list's generation.
 type RowsKey = (String, usize, Option<FontClass>, bool, bool, Vec<String>, Vec<String>, u64);
@@ -285,6 +292,10 @@ fn list(ui: &mut Ui, state_id: egui::Id, st: &mut MenuState, current: &str, samp
                 rows.extend(db.styles(f).into_iter().map(|s| Row::Style(f.clone(), s)));
             }
         }
+        // Filter changes (including un-starring a favourite) may remove rows; expanding
+        // another family or a background font scan can insert rows before the highlight.
+        // Never preview an unrelated family because the old numeric index now points to it.
+        st.highlight = remap_highlight(&st.rows, st.highlight, &rows);
         st.rows = rows.into();
         st.rows_key = Some(key);
     }
@@ -320,7 +331,9 @@ fn list(ui: &mut Ui, state_id: egui::Id, st: &mut MenuState, current: &str, samp
     }
     if rows.is_empty() {
         widgets::dim_label(ui, tl!("No matching fonts"));
-        return None;
+        // A filter or a font rescan can remove the row whose font was previewed.
+        // Do not leave that font applied while the list has no candidate to highlight.
+        return st.previewed.take().map(|_| FontPick::EndPreview);
     }
     // The rows: only those in view are laid out (there are thousands of families). The list is
     // the menu's one scrolling part, under the search field and filters (#555): it fills the
@@ -438,6 +451,7 @@ fn list(ui: &mut Ui, state_id: egui::Id, st: &mut MenuState, current: &str, samp
             st.previewed = Some(pick.clone());
             Some(FontPick::Preview(pick.0, pick.1))
         }
+        None => st.previewed.take().map(|_| FontPick::EndPreview),
         _ => None,
     }
 }
@@ -731,3 +745,35 @@ pub(crate) fn sample_text(app: &VectorcraftApp) -> Option<String> {
     let line = text.lines().map(str::trim).find(|l| !l.is_empty())?;
     Some(line.chars().take(12).collect())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{Row, remap_highlight};
+
+    fn family(name: &str) -> Row {
+        Row::Family(name.to_string())
+    }
+
+    #[test]
+    fn keeps_the_same_family_when_other_rows_are_inserted_or_removed() {
+        let before = [family("Alpha"), family("Bravo"), family("Charlie")];
+        assert_eq!(remap_highlight(&before, Some(2), &[family("Bravo"), family("Charlie")]), Some(1));
+        assert_eq!(remap_highlight(&before, Some(2), &[family("Alpha"), family("New"), family("Bravo"), family("Charlie")]), Some(3));
+    }
+
+    #[test]
+    fn clears_the_highlight_when_its_family_is_filtered_out() {
+        let before = [family("Alpha"), family("Bravo"), family("Charlie")];
+        assert_eq!(remap_highlight(&before, Some(1), &[family("Alpha"), family("Charlie")]), None);
+        assert_eq!(remap_highlight(&before, Some(10), &before), None);
+        assert_eq!(remap_highlight(&before, None, &before), None);
+    }
+
+    #[test]
+    fn expanded_style_rows_do_not_retarget_the_highlight() {
+        let before = [family("Alpha"), family("Bravo")];
+        let after = [family("Alpha"), Row::Style("Alpha".into(), "Bold".into()), family("Bravo")];
+        assert_eq!(remap_highlight(&before, Some(1), &after), Some(2));
+        assert_eq!(remap_highlight(&after, Some(1), &before), None);
+    }
+}

crates/ui-egui/src/tests_font_menu.rs

@@ -224,6 +224,57 @@ fn right_arrow_lists_a_familys_styles_and_they_preview() {
     assert_eq!(picked, Some(FontPick::Chosen("Source Sans 3".into(), Some(styles[1].clone()))));
 }
 
+/// #1116: styles listed above the highlight (a click on another family's triangle) leave it on its
+/// family; it doesn't land on one of those styles and preview it.
+#[test]
+fn styles_listed_above_the_highlight_leave_it_on_its_family() {
+    let ctx = egui::Context::default();
+    crate::theme::install_fonts(&ctx);
+    let frame = |events: Vec<egui::Event>| {
+        let input =
+            egui::RawInput { events, screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(600.0, 800.0))), ..Default::default() };
+        let (mut at, mut picked) = (egui::Pos2::ZERO, None);
+        let mut out = ctx.run_ui(input, |ui| {
+            at = ui.next_widget_position();
+            picked = font_menu(ui, "test-rebuild", "Source Sans 3", 260.0, None, MenuLook::default());
+        });
+        out.textures_delta.clear();
+        // The family's row: the lowest text naming it (the menu's button above names it too).
+        let row = out
+            .shapes
+            .iter()
+            .filter_map(|s| match &s.shape {
+                egui::Shape::Text(t) if t.galley.text() == "Source Sans 3" => Some(t.pos + egui::vec2(0.0, t.galley.size().y * 0.5)),
+                _ => None,
+            })
+            .max_by(|a, b| a.y.total_cmp(&b.y));
+        (at, picked, row)
+    };
+    let (at, ..) = frame(vec![]);
+    let click = at + egui::vec2(40.0, 10.0);
+    let button = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
+    frame(vec![egui::Event::PointerMoved(click), button(click, true)]);
+    frame(vec![button(click, false)]);
+    let mut row = None;
+    for _ in 0..5 {
+        row = frame(vec![]).2;
+    }
+    // The family's disclosure triangle, left of its name: hovered, it highlights the family.
+    let triangle = row.expect("the family is listed") - egui::vec2(7.0, 0.0);
+    frame(vec![egui::Event::PointerMoved(triangle)]);
+    let (_, picked, _) = frame(key(egui::Key::ArrowDown));
+    let Some(FontPick::Preview(next, None)) = picked else { panic!("↓ previews the next family: {picked:?}") };
+    // A click on the triangle, the pointer still: the styles are listed between the two families.
+    frame(vec![button(triangle, true)]);
+    frame(vec![button(triangle, false)]);
+    for _ in 0..3 {
+        let (_, picked, _) = frame(vec![]);
+        assert_eq!(picked, None, "{next} stays highlighted and previewed");
+    }
+    let (_, picked, _) = frame(key(egui::Key::Enter));
+    assert_eq!(picked, Some(FontPick::Chosen(next, None)));
+}
+
 #[test]
 fn stars_mark_favourites_and_the_preferences_size_the_rows() {
     let mut app = VectorcraftApp::new(Session::new(), crate::Services::default());