M5.7: the Fill & Sign text controls use a colour dot and a size flyout

The quick bar is too narrow for the "Font size:" label, number box and
"Colour" text, which wrapped mid-word. They now work like Draw's controls:
a divider, a colour dot that opens the swatches, and an icon that opens a
Font size flyout with a slider. Picking a colour also recolours the selected
Fill & Sign text, as Draw's colour does for its own marks.

crates/ui-egui/src/hotbar.rs

@@ -287,6 +287,30 @@ fn core_tool_buttons(ui: &mut egui::Ui, app: &mut PdfCraftApp, cell: f32, popup_
     });
 }
 
+fn controls_divider(ui: &mut egui::Ui, t: &Tokens, vertical: bool) {
+    let (r, _) = if vertical {
+        ui.allocate_exact_size(vec2(crate::canvas::bars::QUICK_BAR_WIDTH, 9.0), Sense::hover())
+    } else {
+        ui.allocate_exact_size(vec2(9.0, crate::canvas::bars::QUICK_BAR_WIDTH), Sense::hover())
+    };
+    if vertical {
+        ui.painter().hline(r.x_range().shrink(6.0), r.center().y, Stroke::new(1.0, t.divider));
+    } else {
+        ui.painter().vline(r.center().x, r.y_range().shrink(6.0), Stroke::new(1.0, t.divider));
+    }
+}
+
+fn selected_fill_text(app: &PdfCraftApp) -> Option<(usize, usize)> {
+    let (index, id) = app.active_ids()?;
+    let (page, annotation) = app.views.get(index)?.comments.selected?;
+    let doc = app.session.get(id).filter(|doc| doc.allows_annotation())?;
+    doc.info
+        .annotations
+        .iter()
+        .any(|a| a.page == page && a.index == annotation && a.fill_sign && a.subtype == "FreeText" && !a.locked)
+        .then_some((page, annotation))
+}
+
 /// Extra controls rendered below/beside the buttons when a comment or fill tool is active.
 fn active_tool_controls(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, vertical: bool) {
     let current_fill = match app.quick_tool {
@@ -294,47 +318,37 @@ fn active_tool_controls(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, ve
         _ => None,
     };
     if matches!(current_fill, Some(crate::fill_sign::FillTool::Text | crate::fill_sign::FillTool::Date)) {
-        if vertical {
-            ui.label(tl!("Font size:"));
-        }
-        let size = ui.add(
-            egui::DragValue::new(&mut app.fill_text_size)
-                .range(crate::fill_sign::MIN_TEXT_SIZE..=crate::fill_sign::MAX_TEXT_SIZE)
-                .speed(0.25)
-                .suffix(" pt"),
-        );
-        if ((size.changed() && !size.dragged()) || size.drag_stopped())
-            && let Some((index, id)) = app.active_ids()
-            && let Some((page, annotation)) = app.views.get(index).and_then(|view| view.comments.selected)
-            && app.session.get(id).is_some_and(|doc| {
-                doc.allows_annotation()
-                    && doc
-                        .info
-                        .annotations
-                        .iter()
-                        .any(|a| a.page == page && a.index == annotation && a.fill_sign && a.subtype == "FreeText" && !a.locked)
-            })
-        {
-            app.apply_edit(Edit::FontSizeAnnotation { page, index: annotation, size: app.fill_text_size });
-        }
-        ui.menu_button(tl!("Colour"), |ui| {
-            if let Some(color) = comments::swatch_grid(ui, Some(app.fill_text_color)) {
-                app.fill_text_color = color;
-                ui.close();
+        controls_divider(ui, t, vertical);
+        if let Some(color) = comments::quick_bar_colour(ui, app.fill_text_color) {
+            app.fill_text_color = color;
+            if let Some((page, index)) = selected_fill_text(app) {
+                app.apply_edit(Edit::StyleAnnotation { page, index, color: Some(color), opacity: None, width: None, endings: None });
             }
-        });
+        }
+        let resp = icons::button(ui, "sliders-horizontal", 32.0, false, tl!("Font size"));
+        let resized = egui::Popup::menu(&resp)
+            .align(egui::RectAlign::RIGHT_START)
+            .close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside)
+            .show(|ui| {
+                ui.set_min_width(180.0);
+                ui.label(egui::RichText::new(tl!("Font size")).font(theme::semibold(12.0)));
+                let size = ui.add(
+                    egui::Slider::new(&mut app.fill_text_size, crate::fill_sign::MIN_TEXT_SIZE..=crate::fill_sign::MAX_TEXT_SIZE)
+                        .logarithmic(true)
+                        .suffix(" pt"),
+                );
+                // #1274: the size also applies to the Fill & Sign text that is selected,
+                // once per change (a typed value, or the end of a drag), so a drag adds
+                // one undo step and not one per frame.
+                (size.changed() && !size.dragged()) || size.drag_stopped()
+            })
+            .is_some_and(|popup| popup.inner);
+        if resized && let Some((page, index)) = selected_fill_text(app) {
+            app.apply_edit(Edit::FontSizeAnnotation { page, index, size: app.fill_text_size });
+        }
     }
     if let QuickTool::Comment(tool) = app.quick_tool {
-        let (r, _) = if vertical {
-            ui.allocate_exact_size(vec2(crate::canvas::bars::QUICK_BAR_WIDTH, 9.0), Sense::hover())
-        } else {
-            ui.allocate_exact_size(vec2(9.0, crate::canvas::bars::QUICK_BAR_WIDTH), Sense::hover())
-        };
-        if vertical {
-            ui.painter().hline(r.x_range().shrink(6.0), r.center().y, Stroke::new(1.0, t.divider));
-        } else {
-            ui.painter().vline(r.center().x, r.y_range().shrink(6.0), Stroke::new(1.0, t.divider));
-        }
+        controls_divider(ui, t, vertical);
         if let Some(color) = comments::quick_bar_controls(ui, tool, &mut app.comment_prefs)
             && let Some((index, id)) = app.active_ids()
             && let Some((page, annotation)) = app.views.get(index).and_then(|view| view.comments.selected)

crates/ui-egui/src/tools/comments.rs

@@ -41,7 +41,7 @@ pub(crate) use input::{describe, fill_grabs, hit, keys, nudge_keys, page_after_t
 pub use menu::{CanvasAction, fill_picker, recent_custom_colours, remember_custom_colour, status_badge, swatch_grid};
 #[cfg(test)]
 use menu::{RECENT_CUSTOM, same_colour};
-pub(crate) use menu::{attachment_items, context_menu, quick_bar_controls};
+pub(crate) use menu::{attachment_items, context_menu, quick_bar_colour, quick_bar_controls};
 pub(crate) use paint::paint_page;
 #[cfg(test)]
 use paint::{changing_points, drawn_shape, edited_in_view};

crates/ui-egui/src/tools/comments/menu.rs

@@ -286,25 +286,32 @@ pub fn fill_picker(ui: &mut egui::Ui, current: Option<Rgb>) -> Option<Option<Rgb
     picked
 }
 
-/// The comment tools' extra quick-bar controls: pin, colour, fill, opacity and thickness.
-pub(crate) fn quick_bar_controls(ui: &mut egui::Ui, tool: CommentTool, prefs: &mut CommentPrefs) -> Option<Rgb> {
-    let style = prefs.style(tool);
-    let mut selected_color = None;
-    if icons::button(ui, "pin", 32.0, prefs.pinned, if prefs.pinned { tl!("Keep tool selected: on") } else { tl!("Keep tool selected") }).clicked() {
-        prefs.pinned = !prefs.pinned;
-    }
+pub(crate) fn quick_bar_colour(ui: &mut egui::Ui, current: Rgb) -> Option<Rgb> {
     let (r, resp) = ui.allocate_exact_size(vec2(32.0, 32.0), Sense::click());
-    ui.painter().circle_filled(r.center(), 9.0, color32(style.color));
+    ui.painter().circle_filled(r.center(), 9.0, color32(current));
     ui.painter().circle_stroke(r.center(), 9.0, Stroke::new(1.0, Color32::from_black_alpha(50)));
     resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, tl!("Colour")));
     let resp = resp.on_hover_text(tl!("Colour"));
+    let mut picked = None;
     egui::Popup::menu(&resp).align(egui::RectAlign::RIGHT_START).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| {
-        if let Some(c) = swatch_grid(ui, Some(style.color)) {
-            prefs.set_color(tool, c);
-            selected_color = Some(c);
+        if let Some(c) = swatch_grid(ui, Some(current)) {
+            picked = Some(c);
             ui.close();
         }
     });
+    picked
+}
+
+/// The comment tools' extra quick-bar controls: pin, colour, fill, opacity and thickness.
+pub(crate) fn quick_bar_controls(ui: &mut egui::Ui, tool: CommentTool, prefs: &mut CommentPrefs) -> Option<Rgb> {
+    let style = prefs.style(tool);
+    if icons::button(ui, "pin", 32.0, prefs.pinned, if prefs.pinned { tl!("Keep tool selected: on") } else { tl!("Keep tool selected") }).clicked() {
+        prefs.pinned = !prefs.pinned;
+    }
+    let selected_color = quick_bar_colour(ui, style.color);
+    if let Some(c) = selected_color {
+        prefs.set_color(tool, c);
+    }
     if tool.has_fill() {
         let resp = fill_button(ui, style.fill);
         egui::Popup::menu(&resp).align(egui::RectAlign::RIGHT_START).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| {

crates/ui-egui/tests/it/fill_sign.rs

@@ -164,7 +164,7 @@ fn fill_text_size_controls_new_text_and_survives_settings_restore() {
     let mut h = harness();
     h.state_mut().execute("sign.fill.text");
     h.run_steps(2);
-    h.get_by_label("Font size:");
+    h.get_by_label("Font size");
     h.state_mut().fill_text_size = 24.0;
     click(&mut h, 50.0, 250.0);
     assert_eq!(h.state().views[0].fill_text.as_ref().map(|t| t.size), Some(24.0));
@@ -201,6 +201,8 @@ fn fill_text_size_resizes_the_selected_text() {
     };
     let before = height(&h);
     let set_size = |h: &mut Harness<'static, PdfCraftApp>, value: &str| {
+        h.get_by_role_and_label(egui::accesskit::Role::Button, "Font size").click();
+        h.run_steps(2);
         h.query_all_by_role(egui::accesskit::Role::SpinButton)
             .find(|n| n.value().is_some_and(|v| v.ends_with(" pt")))
             .expect("the Font size control")
@@ -211,6 +213,8 @@ fn fill_text_size_resizes_the_selected_text() {
         h.run_steps(1);
         h.key_press(egui::Key::Enter);
         h.run_steps(4);
+        h.get_by_role_and_label(egui::accesskit::Role::Button, "Font size").click();
+        h.run_steps(2);
     };
     // Nothing selected: only the size for new text changes.
     h.state_mut().views[0].comments.selected = None;
@@ -1812,3 +1816,100 @@ fn mode_tabs_keep_the_hand_over_a_type_box_scrolled_under_them() {
     h.run_steps(3);
     assert_eq!(h.state().mode, pdfcraft_ui_egui::Mode::Convert, "the tab takes the click, not the text box");
 }
+
+fn place_selected_text(h: &mut Harness<'static, PdfCraftApp>, text: &str) {
+    h.state_mut().execute("sign.fill.text");
+    click(h, 50.0, 250.0);
+    h.event(egui::Event::Text(text.into()));
+    h.run_steps(1);
+    h.key_press(egui::Key::Enter);
+    h.run_steps(3);
+    let selected = {
+        let s = h.state();
+        s.session
+            .get(s.views[0].id)
+            .unwrap()
+            .info
+            .annotations
+            .iter()
+            .find(|a| a.contents.as_deref() == Some(text))
+            .map(|a| (a.page, a.index))
+            .unwrap()
+    };
+    h.state_mut().views[0].comments.selected = Some(selected);
+    h.run_steps(2);
+}
+
+fn placed_height(h: &Harness<'static, PdfCraftApp>, text: &str) -> f32 {
+    let s = h.state();
+    let a = s.session.get(s.views[0].id).unwrap().info.annotations.iter().find(|a| a.contents.as_deref() == Some(text)).unwrap();
+    a.rect[3] - a.rect[1]
+}
+
+fn placed_color(h: &Harness<'static, PdfCraftApp>, text: &str) -> Option<[f32; 3]> {
+    let s = h.state();
+    s.session.get(s.views[0].id).unwrap().info.annotations.iter().find(|a| a.contents.as_deref() == Some(text)).unwrap().color
+}
+
+#[test]
+fn the_fill_text_size_flyout_opens_from_an_icon_and_leaves_a_stored_size_alone() {
+    let mut h = harness();
+    h.state_mut().fill_text_size = 9.1;
+    place_selected_text(&mut h, "Keep me");
+    let height = placed_height(&h, "Keep me");
+    assert_eq!(h.query_all_by_role(egui::accesskit::Role::SpinButton).filter(|n| n.value().is_some_and(|v| v.ends_with(" pt"))).count(), 0);
+    h.get_by_label("Font size").click();
+    h.run_steps(4);
+    assert!(
+        h.query_all_by_role(egui::accesskit::Role::SpinButton).any(|n| n.value().is_some_and(|v| v.ends_with(" pt"))),
+        "the flyout holds the size"
+    );
+    assert_eq!(h.state().fill_text_size, 9.1, "opening the flyout does not round the size");
+    assert_eq!(placed_height(&h, "Keep me"), height, "opening the flyout does not resize the selected text");
+}
+
+#[test]
+fn dragging_the_fill_text_size_slider_is_one_undo_step() {
+    let mut h = harness();
+    place_selected_text(&mut h, "Drag me");
+    let height = placed_height(&h, "Drag me");
+    h.get_by_label("Font size").click();
+    h.run_steps(2);
+    let slider = h.get_by_role(egui::accesskit::Role::Slider).rect();
+    let y = (slider.min.y + slider.max.y) / 2.0;
+    let from = pos2(slider.min.x + 10.0, y);
+    let to = pos2(slider.max.x - 10.0, y);
+    h.hover_at(from);
+    h.run_steps(1);
+    h.drag_at(from);
+    h.run_steps(1);
+    for k in 1..=4 {
+        h.hover_at(from + (to - from) * (k as f32 / 4.0));
+        h.run_steps(1);
+    }
+    assert_eq!(placed_height(&h, "Drag me"), height, "nothing is applied before the release");
+    h.drop_at(to);
+    h.run_steps(3);
+    assert!(placed_height(&h, "Drag me") > height, "the release resizes the selected text");
+    h.state_mut().execute("edit.undo");
+    h.run_steps(3);
+    assert_eq!(placed_height(&h, "Drag me"), height, "one undo takes the whole drag back");
+}
+
+#[test]
+fn the_fill_colour_dot_recolours_the_selected_text() {
+    let mut h = harness();
+    place_selected_text(&mut h, "Paint me");
+    let before = placed_color(&h, "Paint me");
+    h.get_by_label("Colour").click();
+    h.run_steps(2);
+    h.get_by_label("Blue").click();
+    h.run_steps(3);
+    let color = h.state().fill_text_color.map(|v| v as f32);
+    assert!(color[2] > color[0], "blue was selected: {color:?}");
+    assert_ne!(before, Some(color));
+    assert_eq!(placed_color(&h, "Paint me"), Some(color));
+    h.state_mut().execute("edit.undo");
+    h.run_steps(3);
+    assert_eq!(placed_color(&h, "Paint me"), before, "one undo takes the colour back");
+}