From fa213cfad0eefce340e2ba7717ad27a41bf29f8f Mon Sep 17 00:00:00 2001 From: Combiler01 Date: Sun, 25 Oct 2026 16:16:59 +0530 Subject: [PATCH] fix(ui): show selection font/size in format panel and add missing accessibility labels --- crates/ui-egui/src/dialogs.rs | 6 +- crates/ui-egui/src/edit_text_ui.rs | 127 ++++++++++++++++++++++++++- crates/ui-egui/src/format_text_ui.rs | 26 +++--- crates/ui-egui/src/panels.rs | 11 ++- crates/ui-egui/tests/it/control.rs | 41 +++++++++ crates/ui-egui/tests/it/editing.rs | 83 +++++++++++++++++ 6 files changed, 279 insertions(+), 15 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 0021129e71..cd980cc4c0 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1221,11 +1221,12 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { ui.horizontal(|ui| { let (icon_rect, _) = ui.allocate_exact_size(egui::vec2(18.0, 18.0), egui::Sense::hover()); crate::icons::paint(ui, icon_rect, "search", 16.0, t.text_muted); - ui.add( + let search_resp = ui.add( egui::TextEdit::singleline(&mut app.pin_tool_search) .hint_text(tl!("Search tools…")) .desired_width(ui.available_width() - 32.0), ); + search_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::TextEdit, ui.is_enabled(), tl!("Search tools…"))); if !app.pin_tool_search.is_empty() && widgets::ghost_button(ui, "x", tl!("Clear search")).clicked() { app.pin_tool_search.clear(); } @@ -1254,6 +1255,7 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { let pinned = app.is_tool_pinned(g.id); let (rect, resp) = ui.allocate_exact_size(egui::vec2(ui.available_width(), 44.0), egui::Sense::click()); + resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, label)); let press = theme::Press::track(ui, &resp); press.wash(ui, rect, 6, false); let body = rect.translate(press.offset()); @@ -1280,6 +1282,8 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { } else { (egui::Color32::TRANSPARENT, egui::Stroke::new(1.0, t.border), t.text_muted, tl!("Pin")) }; + let tip = if pinned { tl!("Unpin from Home") } else { tl!("Pin to Home") }; + btn_resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::Button, true, pinned, format!("{tip}: {label}"))); ui.painter().rect(btn_rect, egui::CornerRadius::same(6), btn_fill, btn_stroke, egui::StrokeKind::Inside); ui.painter().text(btn_rect.center(), egui::Align2::CENTER_CENTER, btn_label, theme::medium(12.0), text_color); diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index e2c72ea241..0b0cc0bc1e 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -726,7 +726,6 @@ impl LineEditor { /// After applying formatting: the paragraph now has it. pub fn applied(&mut self) { - self.selection = None; self.font_resource = None; self.family_chosen = false; self.look0 = self.look.clone(); @@ -743,6 +742,132 @@ impl LineEditor { self.text = block.editor_text.clone(); self.original = self.text.clone(); } + + pub fn selection_formatting(&self, sources: &[(String, String)]) -> SelectionFormatting { + let Some((block, lines)) = &self.source else { + return SelectionFormatting { + font_shown: self.font_shown(sources), + size: Some(self.look.size), + family: Some(self.look.family), + bold: Some(self.look.bold), + italic: Some(self.look.italic), + color: Some(self.look.color), + }; + }; + + let spans = block.edited_spans(&self.text); + let matching_lines: Vec<&pdfcraft_engine::TextLine> = match &self.selection { + Some(range) if range.start < range.end => spans + .iter() + .filter(|(span, _)| span.start < range.end && span.end > range.start) + .filter_map(|(_, idx)| lines.get(*idx)) + .collect(), + _ => spans.iter().filter_map(|(_, idx)| lines.get(*idx)).collect(), + }; + + if matching_lines.is_empty() { + return SelectionFormatting { + font_shown: self.font_shown(sources), + size: Some(self.look.size), + family: Some(self.look.family), + bold: Some(self.look.bold), + italic: Some(self.look.italic), + color: Some(self.look.color), + }; + } + + let line_font = |l: &pdfcraft_engine::TextLine| -> String { + if let Some((_, face)) = sources.iter().find(|(res, _)| res == &l.font) { + return face.clone(); + } + let base = l.base_font.as_str(); + let name = match base.split_once('+') { + Some((tag, rest)) if tag.len() == 6 && tag.bytes().all(|b| b.is_ascii_uppercase()) => rest, + _ => base, + }; + if let Some((_, face)) = sources.iter().find(|(_, face)| face.as_str() == name) { + return face.clone(); + } + let look = look_of_line(l); + let lower = name.to_ascii_lowercase(); + if ["times", "serif", "roman", "courier", "helvetica", "arial"].iter().any(|s| lower.contains(s)) { + return look.family.label().to_string(); + } + if name.is_empty() { + tl!("Keep source fonts").to_string() + } else { + name.to_string() + } + }; + + let first_font = line_font(matching_lines[0]); + let same_font = matching_lines.iter().skip(1).all(|l| line_font(l) == first_font); + let font_shown = if same_font { + if self.font_resource.is_some() || self.look.family != self.look0.family || self.family_chosen { + self.font_shown(sources) + } else { + first_font + } + } else { + tl!("Mixed").to_string() + }; + + let first_size = (matching_lines[0].size * 10.0).round() / 10.0; + let same_size = matching_lines.iter().skip(1).all(|l| ((l.size * 10.0).round() / 10.0 - first_size).abs() < 0.05); + let size = if same_size { Some(first_size) } else { None }; + + let first_look = look_of_line(matching_lines[0]); + let same_family = matching_lines.iter().skip(1).all(|l| look_of_line(l).family == first_look.family); + let family = if same_family { Some(first_look.family) } else { None }; + let same_bold = matching_lines.iter().skip(1).all(|l| l.bold == matching_lines[0].bold); + let bold = if same_bold { Some(matching_lines[0].bold) } else { None }; + let same_italic = matching_lines.iter().skip(1).all(|l| l.italic == matching_lines[0].italic); + let italic = if same_italic { Some(matching_lines[0].italic) } else { None }; + let same_color = matching_lines.iter().skip(1).all(|l| l.color == matching_lines[0].color); + let color = if same_color { Some(matching_lines[0].color) } else { None }; + + SelectionFormatting { font_shown, size, family, bold, italic, color } + } + + pub(crate) fn sync_selection_look(&mut self, fmt: &SelectionFormatting) { + if let Some(sz) = fmt.size { + self.look.size = sz; + self.look0.size = sz; + } else { + self.look.size = 0.0; + self.look0.size = 0.0; + } + if let Some(f) = fmt.family { + self.look.family = f; + self.look0.family = f; + } + if let Some(b) = fmt.bold { + self.look.bold = b; + self.look0.bold = b; + } + if let Some(it) = fmt.italic { + self.look.italic = it; + self.look0.italic = it; + } + if let Some(c) = fmt.color { + self.look.color = c; + self.look0.color = c; + } + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct SelectionFormatting { + pub font_shown: String, + pub size: Option, + pub family: Option, + pub bold: Option, + pub italic: Option, + pub color: Option<[f64; 3]>, +} + +fn look_of_line(l: &pdfcraft_engine::TextLine) -> pdfcraft_engine::AddedText { + source_look(&l.base_font, l.size, l.color, l.bold, l.italic) } /// The look shown for a paragraph: the family and weight guessed from its PDF font name. diff --git a/crates/ui-egui/src/format_text_ui.rs b/crates/ui-egui/src/format_text_ui.rs index 749f09dcc2..cc137bafa2 100644 --- a/crates/ui-egui/src/format_text_ui.rs +++ b/crates/ui-egui/src/format_text_ui.rs @@ -37,6 +37,7 @@ pub(crate) struct Fonts<'a> { pub sources: Option<&'a [(String, String)]>, /// The name the list shows for the current choice. pub shown: String, + pub mixed_size: bool, } /// What the panel changed: the style (in place) and a font picked from the list. @@ -77,7 +78,7 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras: // The font, across the panel. let width = ui.available_width(); - egui::ComboBox::from_id_salt("format-font") + let font_resp = egui::ComboBox::from_id_salt("format-font") .selected_text(fonts.shown.as_str()) .width(width) .show_ui(ui, |ui| { @@ -121,22 +122,25 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras: }); } }) - .response - .on_hover_text(format!("{}\n{}", tl!("Font"), tl!("Thai and Arabic use regular fallback fonts when available."))); + .response; + font_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::ComboBox, ui.is_enabled(), tl!("Font"))); + font_resp.on_hover_text(format!("{}\n{}", tl!("Font"), tl!("Thai and Arabic use regular fallback fonts when available."))); ui.add_space(4.0); // Size and colour | bullets, numbering and alignment. row(ui, |ui| { - let _ = egui::ComboBox::from_id_salt("format-size") - .selected_text(number(look.size)) + let size_str = if fonts.mixed_size || look.size <= 0.0 { tl!("Mixed").to_string() } else { number(look.size) }; + let size_resp = egui::ComboBox::from_id_salt("format-size") + .selected_text(size_str) .width(64.0) .show_ui(ui, |ui| { for size in SIZES { ui.selectable_value(&mut look.size, size, number(size)); } }) - .response - .on_hover_text(tl!("Font size")); + .response; + size_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::ComboBox, ui.is_enabled(), tl!("Font size"))); + size_resp.on_hover_text(tl!("Font size")); let swatch = colour_button(ui, t, look.color); egui::Popup::menu(&swatch).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| { if let Some(picked) = crate::comments::swatch_grid(ui, Some(look.color)) { @@ -204,12 +208,13 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras: labelled_icon(ui, t, "arrow-up-down", tl!("Line spacing")); let own = tl!("Auto").to_string(); let shown = if e.line_spacing == 0.0 { own.clone() } else { number(e.line_spacing) }; - egui::ComboBox::from_id_salt("format-line-spacing").selected_text(shown).width(half).show_ui(ui, |ui| { + let spacing_resp = egui::ComboBox::from_id_salt("format-line-spacing").selected_text(shown).width(half).show_ui(ui, |ui| { ui.selectable_value(&mut e.line_spacing, 0.0, own.as_str()); for v in LINE_SPACINGS { ui.selectable_value(&mut e.line_spacing, v, number(v)); } - }); + }).response; + spacing_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::ComboBox, ui.is_enabled(), tl!("Line spacing"))); ui.add_space(8.0); ui.add_enabled_ui(false, |ui| { labelled_icon(ui, t, "pilcrow", tl!("Paragraph spacing")); @@ -295,7 +300,8 @@ fn number_with_presets(ui: &mut egui::Ui, n: Number<'_>, presets: &[f64], width: ui.spacing_mut().item_spacing.x = 2.0; let field = egui::DragValue::new(&mut *n.value).range(n.range).speed(n.speed).max_decimals(2).suffix(n.suffix); let height = ui.spacing().interact_size.y; - ui.add_sized([(width - PRESET_LIST - 2.0).max(30.0), height], field).labelled_by(label.0); + let drag_resp = ui.add_sized([(width - PRESET_LIST - 2.0).max(30.0), height], field).labelled_by(label.0); + drag_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::DragValue, ui.is_enabled(), label.1)); let list = icons::button(ui, "chevron-down", PRESET_LIST, false, label.1); egui::Popup::menu(&list).show(|ui| { for &v in presets { diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index a50e2710e9..38cace3211 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -116,6 +116,8 @@ fn tool_row(ui: &mut egui::Ui, t: &Tokens, g: &ToolGroup, is_pinned: bool) -> (e let pin_rect = Rect::from_center_size(body.right_center() - vec2(14.0, 0.0), vec2(22.0, 22.0)); let pin_resp = ui.interact(pin_rect, ui.id().with(("pin_toggle_row", g.id)), Sense::click()); + let tip = if is_pinned { tl!("Unpin from Home") } else { tl!("Pin to Home") }; + pin_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, tip)); let pin_hovered = pin_resp.hovered(); let mut pin_clicked = false; @@ -566,8 +568,11 @@ fn format_section(app: &mut PdfCraftApp, ui: &mut egui::Ui, t: &Tokens) { .into_iter() .map(|f| (f.resource, f.face)) .collect(); - let shown = ed.font_shown(&sources); - let out = crate::format_text_ui::panel(ui, t, &mut ed.look, Some(&mut ed.extras), Fonts { sources: Some(&sources), shown }); + let fmt = ed.selection_formatting(&sources); + ed.sync_selection_look(&fmt); + let shown = fmt.font_shown; + let mixed_size = fmt.size.is_none(); + let out = crate::format_text_ui::panel(ui, t, &mut ed.look, Some(&mut ed.extras), Fonts { sources: Some(&sources), shown, mixed_size }); let mut changed = out.changed; match out.font { Some(FontChoice::KeepSource) => { @@ -647,7 +652,7 @@ fn format_section(app: &mut PdfCraftApp, ui: &mut egui::Ui, t: &Tokens) { /// Format text for added text: the standard families, size, colour, style and alignment. fn added_text_format(ui: &mut egui::Ui, t: &Tokens, style: &mut pdfcraft_engine::AddedText) { let shown = style.family.label().to_string(); - let out = crate::format_text_ui::panel(ui, t, style, None, crate::format_text_ui::Fonts { sources: None, shown }); + let out = crate::format_text_ui::panel(ui, t, style, None, crate::format_text_ui::Fonts { sources: None, shown, mixed_size: false }); if let Some(crate::format_text_ui::FontChoice::Family(family)) = out.font { style.family = family; } diff --git a/crates/ui-egui/tests/it/control.rs b/crates/ui-egui/tests/it/control.rs index d921b31d6d..745a024ff4 100644 --- a/crates/ui-egui/tests/it/control.rs +++ b/crates/ui-egui/tests/it/control.rs @@ -1891,3 +1891,44 @@ fn routine_continuous_scrolling_does_not_add_navigation_history() { assert!(h.state().views[0].back.is_empty()); assert!(h.state().views[0].forward.is_empty()); } + +#[test] +fn accessible_labels_on_format_text_and_pin_dialog() { + let (mut h, c) = harness(); + + // 1. Check Dialog::PinTool accessibility labels + h.state_mut().dialog = Some(pdfcraft_ui_egui::Dialog::PinTool); + h.run_steps(3); + + let search = ok(&mut h, &c, "ui.inspect", json!({"role": "TextInput", "query": "Search tools…"})); + assert!(search["count"].as_u64().unwrap() >= 1, "search box has accessible label: {search}"); + + let result_rows = ok(&mut h, &c, "ui.inspect", json!({"query": "Organize Pages"})); + assert!(result_rows["count"].as_u64().unwrap() >= 1, "result row has accessible label: {result_rows}"); + + let pin_toggles = ok(&mut h, &c, "ui.inspect", json!({"query": "Pin"})); + assert!(pin_toggles["count"].as_u64().unwrap() >= 1, "pin toggle has accessible label: {pin_toggles}"); + + h.state_mut().dialog = None; + h.run_steps(2); + + // 2. Open Add Text so Format text panel is visible + ok(&mut h, &c, "ui.command", json!({"id": "edit.text"})); + h.run_steps(3); + + let font_combo = ok(&mut h, &c, "ui.inspect", json!({"query": "Font", "role": "ComboBox"})); + assert!(font_combo["count"].as_u64().unwrap() >= 1, "format-font has accessible label: {font_combo}"); + + let size_combo = ok(&mut h, &c, "ui.inspect", json!({"query": "Font size", "role": "ComboBox"})); + assert!(size_combo["count"].as_u64().unwrap() >= 1, "format-size has accessible label: {size_combo}"); + + let line_spacing_combo = ok(&mut h, &c, "ui.inspect", json!({"query": "Line spacing", "role": "ComboBox"})); + assert!(line_spacing_combo["count"].as_u64().unwrap() >= 1, "format-line-spacing has accessible label: {line_spacing_combo}"); + + let scale_spin = ok(&mut h, &c, "ui.inspect", json!({"query": "Horizontal scale", "role": "SpinButton"})); + assert!(scale_spin["count"].as_u64().unwrap() >= 1, "horizontal scale spin button has accessible label: {scale_spin}"); + + let char_spin = ok(&mut h, &c, "ui.inspect", json!({"query": "Character spacing", "role": "SpinButton"})); + assert!(char_spin["count"].as_u64().unwrap() >= 1, "character spacing spin button has accessible label: {char_spin}"); +} + diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 1a618fb8a9..4cedcf33b3 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -7268,3 +7268,86 @@ fn clicking_after_image_context_menu_action_resets_menu_state_and_updates_select h.run_steps(2); assert!(h.state().views[0].image_selection.is_none(), "clicking empty page clears selection after context menu action"); } + +#[test] +fn format_panel_shows_selection_size_and_updates_after_font_applied() { + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [4 0 R] /Count 1 /MediaBox [0 0 200 300] >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(), + "<< /Type /Page /Parent 2 0 R /Contents 5 0 R /Resources << /Font << /F1 3 0 R >> >> >>".into(), + ]; + let body = "BT /F1 24 Tf 20 200 Td (Heading) Tj /F1 12 Tf 0 -25 Td (Paragraph) Tj ET"; + objs.push(format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len())); + let mut pdf = Vec::from(b"%PDF-1.7\n"); + let mut xref = Vec::new(); + for (i, o) in objs.iter().enumerate() { + xref.push(pdf.len()); + pdf.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref_pos = pdf.len(); + pdf.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in xref { + pdf.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + pdf.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref_pos}\n%%EOF\n", objs.len() + 1).as_bytes()); + + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("test.pdf", None, pdf).expect("fixture opens"); + app + }); + h.run_steps(4); + + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + let at = egui::pos2(r.left() + 40.0 / 200.0 * r.width(), r.top() + (300.0 - 180.0) / 300.0 * r.height()); + h.hover_at(at); + h.run_steps(1); + h.drag_at(at); + h.run_steps(1); + h.drop_at(at); + h.run_steps(3); + + assert!(h.state().views[0].line_editor.is_some(), "editor opened"); + let text = h.state().views[0].line_editor.as_ref().unwrap().text.clone(); + assert!(text.contains("Paragraph"), "text is {text}"); + + // Select the 12 pt paragraph text "Paragraph" via keyboard selection in the TextEdit + h.key_press_modifiers(Modifiers::COMMAND, Key::ArrowRight); + h.run_steps(1); + for _ in 0.."Paragraph".len() { + h.key_press_modifiers(Modifiers::SHIFT, Key::ArrowLeft); + h.run_steps(1); + } + use egui::accesskit::Role; + let size_node = h.get_by_role_and_label(Role::ComboBox, "Font size"); + assert_eq!(size_node.value().as_deref(), Some("12"), "size combo box value is 12"); + + let font_node = h.get_by_role_and_label(Role::ComboBox, "Font"); + assert_eq!(font_node.value().as_deref(), Some("Helvetica"), "initial font is Helvetica"); + + // Apply Times to the selection + font_node.click(); + h.run_steps(2); + h.get_by_label("Times").click(); + h.run_steps(4); + + // Font combo box for the whole block is now Mixed because Heading is Helvetica and Paragraph is Times + let font_node = h.get_by_role_and_label(Role::ComboBox, "Font"); + assert_eq!(font_node.value().as_deref(), Some("Mixed"), "whole block has mixed fonts"); + + // Apply Times to the whole block + font_node.click(); + h.run_steps(2); + h.get_by_label("Times").click(); + h.run_steps(4); + + // Now all text is Times, so font combo box updates to Times + let font_node = h.get_by_role_and_label(Role::ComboBox, "Font"); + assert_eq!(font_node.value().as_deref(), Some("Times"), "font combo box updates to Times"); +} +