Accessibility: name ribbon drop-downs by purpose and expose the shown text as their value (#1956) (#1963)

crates/ui-egui/src/ribbon/animations.rs

@@ -164,7 +164,7 @@ pub(super) fn animations(app: &mut SlideApp, ui: &mut Ui) {
                     deckcraft_model::AnimStart::WithPrevious => "With Previous",
                     deckcraft_model::AnimStart::AfterPrevious => "After Previous",
                 });
-                dropdown(ui, "astart", s.unwrap_or(""), 112.0, cur.is_some(), |ui| {
+                dropdown(ui, "Start", s.unwrap_or(""), 112.0, cur.is_some(), |ui| {
                     for (l, v) in [("On Click", "onClick"), ("With Previous", "withPrevious"), ("After Previous", "afterPrevious")] {
                         if ui.button(l).clicked() {
                             run(app, "animation.timing", json!({"start": v}));

crates/ui-egui/src/ribbon/home.rs

@@ -171,7 +171,7 @@ fn font_group(app: &mut SlideApp, ui: &mut Ui, st: &Value) {
                 let size = st.get("size").and_then(Value::as_f64);
                 let fams = font_families(app);
                 let theme_fonts: Vec<String> = fams.iter().take(2).cloned().collect();
-                dropdown(ui, "font", &font, 150.0, en, |ui| {
+                dropdown(ui, "Font", &font, 150.0, en, |ui| {
                     for (k, f) in fams.iter().enumerate() {
                         let label = if k == 0 && theme_fonts.len() == 2 {
                             format!("{f} (Headings)")
@@ -196,7 +196,7 @@ fn font_group(app: &mut SlideApp, ui: &mut Ui, st: &Value) {
                     }
                 });
                 let size_text = size.map(|s| widgets::fmt_num(s, "")).unwrap_or_default();
-                dropdown(ui, "size", &size_text, 52.0, en, |ui| {
+                dropdown(ui, "Font size", &size_text, 52.0, en, |ui| {
                     for s in deckcraft_engine::cmd::format::SIZES.iter().chain([72.0, 80.0, 88.0, 96.0].iter()) {
                         if ui.button(widgets::fmt_num(*s, "")).clicked() {
                             run(app, "format.size", json!({"size": s}));

crates/ui-egui/src/tests.rs

@@ -1223,6 +1223,31 @@ fn step_app(services: Services) -> egui_kittest::Harness<'static, SlideApp> {
     h
 }
 
+/// #1956: the ribbon's drop-downs are announced by their purpose, not by their value or an
+/// internal key. Animations › Start with a shape that has no animation was "astart"; Home's Font
+/// and Font size boxes were "font" and "size" while nothing told them a font or size.
+#[test]
+fn ribbon_dropdowns_are_named_by_purpose_even_when_empty() {
+    use egui::accesskit::Role;
+    let mut h = step_app(Services::default());
+    // A fresh shape, selected but not edited and without an animation.
+    h.state_mut().run("shape.insert", json!({"preset": "rect", "rect": [100.0, 100.0, 200.0, 100.0]})).unwrap();
+    h.state_mut().run("text.exit", json!({})).unwrap();
+    h.state_mut().ui.tab = "Animations".into();
+    h.run_steps(2);
+    let start = h.get_by_role_and_label(Role::ComboBox, "Start");
+    assert_eq!(start.value(), None, "no animation, no start value");
+    assert!(h.query_by_label("astart").is_none(), "the internal key is not a name");
+
+    h.state_mut().ui.tab = "Home".into();
+    h.state_mut().run("edit.select", json!({"ids": []})).unwrap();
+    h.run_steps(2);
+    for (name, key) in [("Font", "font"), ("Font size", "size")] {
+        assert!(h.query_by_role_and_label(Role::ComboBox, name).is_some(), "{name} is named while empty");
+        assert!(h.query_by_role_and_label(Role::ComboBox, key).is_none(), "{key} is not announced");
+    }
+}
+
 /// Copy of a bold selection offers HTML; paste of Pages-like HTML keeps the bold.
 #[test]
 fn copy_offers_html_and_paste_keeps_bold() {

crates/ui-egui/src/widgets.rs

@@ -330,7 +330,9 @@ pub fn rows<R>(ui: &mut Ui, add: impl FnOnce(&mut Ui) -> R) -> R {
 }
 
 /// A text dropdown field (font name, size): shows `current`, opens a popup with `add_items`.
-pub fn dropdown(ui: &mut Ui, id: &str, current: &str, width: f32, enabled: bool, add_items: impl FnOnce(&mut Ui)) -> Response {
+/// `label` is what assistive technology calls it ("Font", "Start"); `current` is exposed as its
+/// value, so an empty box is still announced by its purpose and never by an internal key (#1956).
+pub fn dropdown(ui: &mut Ui, label: &str, current: &str, width: f32, enabled: bool, add_items: impl FnOnce(&mut Ui)) -> Response {
     let t = Tokens::get(ui.ctx());
     let (rect, resp) = ui.allocate_exact_size(vec2(width, 22.0), if enabled { Sense::click() } else { Sense::hover() });
     let painter = ui.painter_at(rect);
@@ -350,8 +352,10 @@ pub fn dropdown(ui: &mut Ui, id: &str, current: &str, width: f32, enabled: bool,
         if enabled { t.text } else { t.text_faint },
     );
     chevron(&painter, pos2(rect.max.x - 9.0, rect.center().y), 3.0, t.text_dim);
-    resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::ComboBox, enabled, if current.is_empty() { id } else { current }));
-    let _ = id;
+    resp.widget_info(|| egui::WidgetInfo {
+        current_text_value: (!current.is_empty()).then(|| current.to_string()),
+        ..egui::WidgetInfo::labeled(egui::Role::ComboBox, enabled, label)
+    });
     egui::Popup::menu(&resp).width(width.max(140.0)).show(|ui| {
         egui::ScrollArea::vertical().max_height(360.0).show(ui, add_items);
     });
@@ -742,19 +746,26 @@ mod tests {
     }
 
     /// The custom ribbon dropdown is a real combo box in the accessibility tree, named by its
-    /// current value (0.37's strict unnamed-input check would otherwise reject it).
+    /// purpose with the shown text as its value. An empty one (no selection to read a font or
+    /// size from) keeps its name rather than falling back to an internal key (#1956).
     #[test]
-    fn the_dropdown_has_a_combo_box_role_and_a_name() {
+    fn the_dropdown_is_named_by_its_purpose_and_carries_its_value() {
         let mut harness = egui_kittest::Harness::builder().with_size(vec2(400.0, 200.0)).build_ui(|ui| {
             theme::install_fonts(ui.ctx());
-            dropdown(ui, "font-size", "18 pt", 120.0, true, |ui| {
+            dropdown(ui, "Font size", "18 pt", 120.0, true, |ui| {
                 ui.label("12 pt");
                 ui.label("18 pt");
             });
+            dropdown(ui, "Start", "", 112.0, false, |_| {});
         });
         harness.run();
-        let combo = harness.get_by_role_and_label(Role::ComboBox, "18 pt");
+        let combo = harness.get_by_role_and_label(Role::ComboBox, "Font size");
+        assert_eq!(combo.value().as_deref(), Some("18 pt"));
         assert!(combo.rect().width() > 100.0);
+        assert!(harness.query_by_role_and_label(Role::ComboBox, "18 pt").is_none(), "the value is not its name");
+        let empty = harness.get_by_role_and_label(Role::ComboBox, "Start");
+        assert_eq!(empty.value(), None, "nothing shown, no value");
+        assert!(harness.query_by_label("astart").is_none() && harness.query_by_label("start").is_none(), "no internal key is announced");
     }
 
     /// #32: clicking the "More Colors" hex field closed the colour menu before anything could be