From 3bcaacc272027dc7a7c6c4d7eb6beb7454d4d5d6 Mon Sep 17 00:00:00 2001 From: lordwotton29 <57874567+lordwotton29@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:19:34 +0100 Subject: [PATCH] fix(print): keep the localized reset label off the properties panel title (#5550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(print): keep the localized reset label off the properties panel title The header of Print ▸ Properties… laid the title out first and gave what was left of the row to Done and Reset. When the translated "Reset to printer defaults" needs more room than that, egui keeps placing the two buttons from the right edge of the child leftwards and paints them back over the title: in Italian the label is 51 characters against 25 in English, and both strings end up unreadable (#5545). Laid out with egui::Sides::shrink_left().truncate instead: the buttons take their room first and the title takes what is left of the row, cut with an ellipsis. Nothing moves in English, where the title still fits. * test(print): pin the header against every catalog and the title's tooltip The geometry tests behind #5545 tried English, Italian and German at four widths. They now walk every catalogued language at the row the settings column really gives the header (470 px less the frame's 10 px margins), so the longest translation of "Reset to printer defaults" is covered by the run instead of by a guess at which locale is the worst one. Two more tests: hovering the elided title opens a tooltip with the full string, while a title that fits whole opens none; and a row too narrow to give the title even an ellipsis leaves it out instead of painting it back over the buttons. That last case is why driver_options_header returns Option. Bulgarian (370 px of button), Spanish (365) and Ukrainian (361) fill the row with the two buttons alone at 450 px: the ellipsis is 12.4 px wide and the clearance was 0.1 px, so a pixel less and the label was painted over the buttons again. --------- Co-authored-by: Jarvis (Hermes) Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> --- crates/ui-egui/src/workflows/print_ui/body.rs | 161 ++++++++++++++++-- 1 file changed, 150 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/workflows/print_ui/body.rs b/crates/ui-egui/src/workflows/print_ui/body.rs index 7058233318..43e5323bd3 100644 --- a/crates/ui-egui/src/workflows/print_ui/body.rs +++ b/crates/ui-egui/src/workflows/print_ui/body.rs @@ -4,23 +4,47 @@ use super::{ Subset, Tokens, pos2, print, printer_label, spool, still_looking, theme, vec2, widgets, }; +/// The narrowest the title is still worth cutting: an ellipsis and a couple of pixels at the title's +/// font. Given less room than this, [`egui::Sides::truncate`] cannot cut any further and the label +/// is painted back over the buttons, which is the defect of #5545; the title is left out instead. +const TITLE_MIN_WIDTH: f32 = 16.0; + +/// The panel's own header: "Printer properties" on the left, Done and Reset on the right. +/// +/// The buttons take their room first, from the right (`shrink_left`), and the title gets what is +/// left of the row: [`egui::Sides::truncate`] cuts it with an ellipsis instead of letting a label wider than the row +/// spill over it. Laid out the other way round, the label is placed first at its natural width and +/// the buttons are painted back over it, which is what a translation of "Reset to printer +/// defaults" long enough to fill the row did in Italian (#5545). +/// +/// The title comes back as `None` when the row is too narrow for even an ellipsis, which a catalog +/// whose two buttons nearly fill the row reaches (Bulgarian, Ukrainian, Spanish at about 450 px). +fn driver_options_header(ui: &mut egui::Ui, show: &mut bool, can_reset: bool) -> (Option, egui::Response) { + egui::Sides::new().shrink_left().truncate().show( + ui, + |ui| { + let room = ui.available_width(); + (room >= TITLE_MIN_WIDTH).then(|| ui.add(egui::Label::new(egui::RichText::new(tl!("Printer properties")).font(theme::semibold(13.0))))) + }, + |ui| { + if ui.button(tl!("Done")).clicked() { + *show = false; + } + ui.add_enabled(can_reset, egui::Button::new(tl!("Reset to printer defaults"))) + }, + ) +} + /// Properties… on CUPS: the driver's options for this print, grouped as the driver groups them. /// Only choices away from the printer's defaults are kept, and sent with the job. fn driver_options_panel(ui: &mut egui::Ui, d: &mut PrintDraft, t: &Tokens) { let PrintDraft { driver_options: Some((_, options)), driver_choices: choices, show_driver_options: show, .. } = d else { return }; egui::Frame::new().fill(t.hover).corner_radius(egui::CornerRadius::same(6)).inner_margin(egui::Margin::same(10)).show(ui, |ui| { ui.set_width(ui.available_width()); - ui.horizontal(|ui| { - ui.label(egui::RichText::new(tl!("Printer properties")).font(theme::semibold(13.0))); - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - if ui.button(tl!("Done")).clicked() { - *show = false; - } - if ui.add_enabled(!choices.is_empty(), egui::Button::new(tl!("Reset to printer defaults"))).clicked() { - choices.clear(); - } - }); - }); + let (_, reset) = driver_options_header(ui, show, !choices.is_empty()); + if reset.clicked() { + choices.clear(); + } if options.is_empty() { ui.label(egui::RichText::new(tl!("This printer offers no settings of its own.")).small().color(t.text_muted)); return; @@ -670,8 +694,123 @@ mod tests { use egui_kittest::Harness; use egui_kittest::kittest::Queryable; + use super::*; + use crate::i18n::Lang; use crate::{Dialog, PdfCraftApp}; + /// The row the settings column really gives the header: `body` sets it to 470 px, less the + /// frame's 10 px margins. + const PANEL_ROW: f32 = 450.0; + + /// One frame of the header at `width`, drawn in the app's own fonts; with `hover`, the pointer + /// is parked on the title and the frames after the first are run so that a tooltip can come up. + /// Returns the title's rectangle (none when the title was left out), the reset button's, and + /// whether a tooltip layer is open. + fn header_frame(lang: &str, width: f32, hover: bool) -> (Option, egui::Rect, bool) { + crate::i18n::set_current(Lang::from_code(lang).expect("a catalogued language")); + let ctx = egui::Context::default(); + ctx.set_fonts(theme::font_definitions()); + // Without this the tooltip would need half a second of frames to show. + ctx.all_styles_mut(|style| style.interaction.tooltip_delay = 0.0); + let row = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(width, 400.0)); + let pointer = std::cell::Cell::new(None); + let seen = std::cell::Cell::new((None, egui::Rect::NOTHING)); + for _ in 0..3 { + let mut input = egui::RawInput { screen_rect: Some(row), ..Default::default() }; + if let Some(at) = hover.then(|| pointer.get()).flatten() { + input.events.push(egui::Event::PointerMoved(at)); + } + let mut output = ctx.run_ui(input, |ui| { + let mut show = true; + let (title, reset) = driver_options_header(ui, &mut show, true); + let title = title.map(|title| title.rect); + pointer.set(title.map(|title| title.center())); + seen.set((title, reset.rect)); + }); + // Nothing draws the frame, but epaint's deltas must be taken anyway. + output.textures_delta.clear(); + } + let tooltip = ctx.memory(|memory| memory.areas().visible_layer_ids().iter().any(|layer| layer.order == egui::Order::Tooltip)); + crate::i18n::set_current(Lang::EN); + let (title, reset) = seen.get(); + (title, reset, tooltip) + } + + fn header_at(lang: &str, width: f32) -> (Option, egui::Rect) { + let (title, reset, _) = header_frame(lang, width, false); + (title, reset) + } + + /// #5545: in a long locale the reset label is wider than the row, and it used to be painted + /// over the title. Every catalogued language is checked at the row the panel really gives the + /// header, so the longest translation of "Reset to printer defaults" is covered by the run + /// rather than by a guess at which locale is the worst one. + #[test] + fn the_reset_button_never_covers_the_title_in_any_language() { + if !crate::i18n::needs_catalogs() { + return; + } + for lang in Lang::all() { + let (title, reset) = header_at(lang.code(), PANEL_ROW); + assert!( + title.is_none_or(|title| !title.intersects(reset)), + "{} at {PANEL_ROW}px: the title {title:?} is drawn on top of the reset button {reset:?}", + lang.code() + ); + assert!(reset.right() <= PANEL_ROW, "{}: the reset button {reset:?} runs out of the row", lang.code()); + } + } + + /// A row narrower than the buttons plus one ellipsis cannot be cut any further: the title is + /// left out instead of being painted back over the buttons. Bulgarian, Ukrainian and Spanish + /// are the catalogs that get there first, at the row's own width. + #[test] + fn a_row_without_room_for_a_title_leaves_it_out() { + if !crate::i18n::needs_catalogs() { + return; + } + for width in [449.0, 440.0, 400.0, 300.0] { + for lang in ["bg", "uk", "es", "it", "en"] { + let (title, reset) = header_at(lang, width); + assert!( + title.is_none_or(|title| !title.intersects(reset)), + "{lang} at {width}px: the title {title:?} is drawn on top of the reset button {reset:?}" + ); + } + } + let (title, reset) = header_at("bg", 440.0); + assert!(title.is_none(), "the title is dropped, not squeezed to a sliver: {:?} next to {reset:?}", title); + } + + /// The title is what gives way: it truncates in a narrow row while the buttons keep their + /// size, and a wide row keeps it whole instead of cutting it for nothing. + #[test] + fn the_title_truncates_and_leaves_the_buttons_whole() { + let (wide_title, wide_reset) = header_at("en", 620.0); + let (narrow_title, narrow_reset) = header_at("en", 300.0); + let (wide_title, narrow_title) = (wide_title.expect("the wide row keeps the title"), narrow_title.expect("the narrow row keeps a title")); + assert!(narrow_title.width() < wide_title.width(), "the title shrinks in a narrow row"); + assert_eq!(narrow_reset.width(), wide_reset.width(), "the buttons keep their size"); + assert!(narrow_reset.right() <= 301.0, "the reset button stays inside the row"); + } + + /// The cut title stays reachable: hovering it shows the full string in a tooltip (egui's + /// `Label` does that by itself when it elides), and a title that fits whole shows none. + #[test] + fn the_truncated_title_is_reachable_through_a_tooltip() { + if !crate::i18n::needs_catalogs() { + return; + } + let (whole, _, _) = header_frame("it", 620.0, false); + let whole = whole.expect("a wide row keeps the title"); + let (cut, _, tooltip) = header_frame("it", PANEL_ROW, true); + let cut = cut.expect("the Italian title is elided, not dropped"); + assert!(cut.width() < whole.width(), "the Italian title is cut at {PANEL_ROW}px: {} of {}", cut.width(), whole.width()); + assert!(tooltip, "hovering the elided title opens a tooltip with the full string"); + let (_, _, tooltip) = header_frame("en", 620.0, true); + assert!(!tooltip, "a title that fits whole needs no tooltip"); + } + /// #4252, #5317: on a short screen the settings+preview area used to grow past /// both the top and bottom of the screen (centered modals overflow both edges equally), /// taking the Cancel button with it. It must now stay reachable, scrolling instead.