fix(gui): draw popup messages one line per line break

crates/gui/src/draw/ui/text.rs

@@ -20,7 +20,7 @@ pub fn wrap(
 ) -> Vec<OwnedSection> {
     let mut sections = Vec::new();
     let mut cur_y = y;
-    for line in word_wrap(message, max_chars_per_line) {
+    for line in lines(message, max_chars_per_line) {
         sections.push(
             Section::default()
                 .add_text(Text::new(line).with_scale(font_scale).with_color(color))
@@ -32,6 +32,22 @@ pub fn wrap(
     sections
 }
 
+/// Split a message into the lines it will be drawn as: explicit line breaks are kept,
+/// and each of the resulting paragraphs is wrapped at word boundaries.
+pub fn lines(message: &str, max_chars_per_line: usize) -> Vec<&str> {
+    message
+        .split('\n')
+        .flat_map(|paragraph| {
+            let wrapped = word_wrap(paragraph.trim_end_matches('\r'), max_chars_per_line);
+            if wrapped.is_empty() {
+                vec![""]
+            } else {
+                wrapped
+            }
+        })
+        .collect()
+}
+
 pub(crate) fn word_wrap(text: &str, max_chars: usize) -> Vec<&str> {
     let mut lines = Vec::new();
     let mut start = 0;
@@ -67,7 +83,49 @@ pub(crate) fn word_wrap(text: &str, max_chars: usize) -> Vec<&str> {
 
 #[cfg(test)]
 mod tests {
-    use super::word_wrap;
+    use super::{lines, word_wrap};
+
+    #[test]
+    fn explicit_line_breaks_are_kept() {
+        assert_eq!(lines("uno\ndos\ntres", 40), vec!["uno", "dos", "tres"]);
+    }
+
+    #[test]
+    fn blank_lines_are_kept() {
+        assert_eq!(lines("uno\n\ndos", 40), vec!["uno", "", "dos"]);
+    }
+
+    #[test]
+    fn each_paragraph_is_wrapped_on_its_own() {
+        let wrapped = lines("hola mundo cruel\nadios", 10);
+
+        assert_eq!(wrapped.last(), Some(&"adios"));
+        assert!(wrapped.len() > 2, "the first paragraph should be wrapped");
+        for line in &wrapped {
+            assert!(
+                !line.contains('\n'),
+                "no line may carry a line break: {:?}",
+                line
+            );
+        }
+    }
+
+    #[test]
+    fn message_with_line_breaks_produces_one_line_per_break() {
+        // The approval popup message: every line must be its own entry, or they overlap
+        let message = "The server demo50.udsenterprise.com:5443\nmust be approved.\nOnly approve UDS servers you trust.\nDo you want to continue?";
+
+        let wrapped = lines(message, 46);
+
+        assert!(wrapped.len() >= 4);
+        assert_eq!(wrapped[0], "The server demo50.udsenterprise.com:5443");
+        assert_eq!(wrapped[1], "must be approved.");
+    }
+
+    #[test]
+    fn carriage_returns_are_not_drawn() {
+        assert_eq!(lines("uno\r\ndos", 40), vec!["uno", "dos"]);
+    }
 
     #[test]
     fn empty_string() {

crates/gui/src/windows/popup.rs

@@ -22,6 +22,46 @@ pub enum PopupKind {
     Error(String),
 }
 
+// Logical (unscaled) layout of the popup, shared by the window sizing and the painting
+const MARGIN: f32 = 20.0;
+const ICON_SIZE: f32 = 40.0;
+const MESSAGE_FONT_SIZE: f32 = 14.0;
+const MESSAGE_LINE_HEIGHT: f32 = MESSAGE_FONT_SIZE * 1.5;
+const MESSAGE_TOP_GAP: f32 = 10.0;
+const BUTTON_HEIGHT: f32 = 40.0;
+const BUTTON_TOP_GAP: f32 = 15.0;
+const WIDTH: f32 = 400.0;
+const MIN_HEIGHT: f32 = 200.0;
+
+fn message_of(kind: &PopupKind) -> &str {
+    match kind {
+        PopupKind::YesNo { message, .. } => message,
+        PopupKind::Warning(message) | PopupKind::Error(message) => message,
+    }
+}
+
+fn max_chars_per_line(width: f32, font_size: f32) -> usize {
+    ((width - 2.0 * MARGIN) / (font_size * 0.55)) as usize
+}
+
+fn message_top(margin: f32, icon_size: f32, top_gap: f32) -> f32 {
+    margin + icon_size + top_gap
+}
+
+fn scaled(value: f32) -> f32 {
+    value * *monitor::SCALE_FACTOR as f32
+}
+
+fn height_for(line_count: usize) -> f32 {
+    let height = message_top(MARGIN, ICON_SIZE, MESSAGE_TOP_GAP)
+        + line_count as f32 * MESSAGE_LINE_HEIGHT
+        + BUTTON_TOP_GAP
+        + BUTTON_HEIGHT
+        + MARGIN;
+
+    height.max(MIN_HEIGHT)
+}
+
 pub struct PopupState {
     pub window: Arc<winit::window::Window>,
     pub renderer: WgpuRenderer,
@@ -39,9 +79,11 @@ impl PopupState {
         kind: PopupKind,
     ) -> anyhow::Result<Self> {
         let (dw, dh) = crate::monitor::size(0).unwrap_or((1920, 1080));
-        let ww = 400.0;
-        let wh = 200.0;
         let sf = crate::monitor::scale(0) as f32;
+        let ww = WIDTH;
+        let line_count =
+            text::lines(message_of(&kind), max_chars_per_line(ww, MESSAGE_FONT_SIZE)).len();
+        let wh = height_for(line_count).min(dh as f32 / sf);
         let px = (dw as f32 - ww * sf) / 2.0;
         let py = (dh as f32 - wh * sf) / 2.0;
 
@@ -60,8 +102,8 @@ impl PopupState {
         let renderer = WgpuRenderer::new(window.clone(), phys.width, phys.height)?;
         let pw = phys.width as f32;
         let ph = phys.height as f32;
-        let bh = monitor::scaled_val(40) as f32;
-        let by = ph - bh - 20.0 * scale;
+        let bh = scaled(BUTTON_HEIGHT);
+        let by = ph - bh - scaled(MARGIN);
 
         let mut buttons = Vec::new();
         match &kind {
@@ -208,7 +250,7 @@ impl PopupState {
         data.push(panel_pixmap.take());
         ov_descs.push((0, pw, ph, 0.0, 0.0));
 
-        let icon_size_px = monitor::scaled_val(40) as u32;
+        let icon_size_px = scaled(ICON_SIZE) as u32;
         let mut icon_pixmap = Pixmap::new(icon_size_px, icon_size_px).unwrap();
         let icon_center = icon_size_px as f32 / 2.0;
         let icon_radius = icon_center - 2.0 * s;
@@ -275,18 +317,14 @@ impl PopupState {
                 .to_owned(),
         );
 
-        let msg_fs = monitor::scaled_val(14) as f32;
-        let msg_x = 20.0 * s;
-        let msg_y = 20.0 * s + icon_size_px as f32 + 10.0 * s;
-        let max_chars = ((pw as f32 - 40.0 * s) / (msg_fs * 0.55)) as usize;
         sections.extend(text::wrap(
             message,
-            max_chars,
-            msg_fs,
+            max_chars_per_line(WIDTH, MESSAGE_FONT_SIZE),
+            scaled(MESSAGE_FONT_SIZE),
             [0.9, 0.9, 0.9, 1.0],
-            msg_x,
-            msg_y,
-            msg_fs * 1.5,
+            scaled(MARGIN),
+            scaled(message_top(MARGIN, ICON_SIZE, MESSAGE_TOP_GAP)),
+            scaled(MESSAGE_LINE_HEIGHT),
         ));
 
         for btn in &self.buttons {
@@ -356,3 +394,54 @@ impl crate::AppHandler {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn short_messages_keep_the_default_size() {
+        assert_eq!(height_for(1), MIN_HEIGHT);
+    }
+
+    #[test]
+    fn the_window_grows_with_the_message() {
+        assert!(height_for(8) > height_for(4));
+        assert_eq!(
+            height_for(8) - height_for(4),
+            4.0 * MESSAGE_LINE_HEIGHT,
+            "each extra line adds exactly one line height"
+        );
+    }
+
+    #[test]
+    fn the_message_never_reaches_the_buttons() {
+        for line_count in 1..20 {
+            let message_bottom = message_top(MARGIN, ICON_SIZE, MESSAGE_TOP_GAP)
+                + line_count as f32 * MESSAGE_LINE_HEIGHT;
+            let buttons_top = height_for(line_count) - BUTTON_HEIGHT - MARGIN;
+
+            assert!(
+                message_bottom <= buttons_top,
+                "{} lines: message ends at {}, buttons start at {}",
+                line_count,
+                message_bottom,
+                buttons_top
+            );
+        }
+    }
+
+    #[test]
+    fn a_wrapped_message_is_taller_than_a_one_liner() {
+        let one_liner = text::lines(
+            "Access denied",
+            max_chars_per_line(WIDTH, MESSAGE_FONT_SIZE),
+        );
+        let long = text::lines(
+            "The server demo50.udsenterprise.com:5443\nmust be approved.\nOnly approve UDS servers you trust.\nDo you want to continue?",
+            max_chars_per_line(WIDTH, MESSAGE_FONT_SIZE),
+        );
+
+        assert!(height_for(long.len()) > height_for(one_liner.len()));
+    }
+}