From 3e82fbba215ceedccb8774ddc40af07e6b19468e Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 26 Oct 2026 16:29:18 +0100 Subject: [PATCH 1/2] fix(ui): reserve popup header space for controls --- crates/ui-egui/src/mapview.rs | 62 +++++++++++++++++------------------ crates/ui-egui/tests/smoke.rs | 46 ++++++++++++++++++++++++++ docs/user-guide/editing.md | 3 +- 3 files changed, 79 insertions(+), 32 deletions(-) diff --git a/crates/ui-egui/src/mapview.rs b/crates/ui-egui/src/mapview.rs index 89de244ce..86477738b 100644 --- a/crates/ui-egui/src/mapview.rs +++ b/crates/ui-egui/src/mapview.rs @@ -2844,40 +2844,40 @@ fn popup(app: &mut RustGisApp, ui: &mut Ui, rect: Rect) { .corner_radius(CornerRadius { nw: 10, ne: 10, sw: 0, se: 0 }) .inner_margin(egui::Margin::symmetric(12, 8)) .show(ui, |ui| { - ui.horizontal(|ui| { - ui.vertical(|ui| { - ui.label(egui::RichText::new(&title).font(semibold(14.0))); - ui.label(egui::RichText::new(&layer).size(11.0).color(t.text_dim)); - }); - ui.with_layout(egui::Layout::right_to_left(egui::Align::Min), |ui| { - if ui.add(egui::Button::new(tl!("×")).frame(false)).clicked() { - close = true; + // Lay out controls first; the title and layer share only the width left. + ui.with_layout(egui::Layout::right_to_left(egui::Align::Min), |ui| { + if ui.add(egui::Button::new(tl!("×")).frame(false)).clicked() { + close = true; + } + // A feature (not a raster pixel): zoom to it. + if shown.is_some() { + let (r, resp) = ui.allocate_exact_size(vec2(18.0, 18.0), Sense::click()); + let ink = if resp.hovered() { t.text } else { t.icon }; + crate::icons::paint(ui.painter(), r, "zoom_sel", ink, t.accent); + if resp.on_hover_text(tl!("Zoom To")).clicked() { + zoom = true; } - // A feature (not a raster pixel): zoom to it. - if shown.is_some() { - let (r, resp) = ui.allocate_exact_size(vec2(18.0, 18.0), Sense::click()); - let ink = if resp.hovered() { t.text } else { t.icon }; - crate::icons::paint(ui.painter(), r, "zoom_sel", ink, t.accent); - if resp.on_hover_text(tl!("Zoom To")).clicked() { - zoom = true; - } + } + // Several features identified: step through them. + if results > 1 { + if ui.add_enabled(index + 1 < results, egui::Button::new("›").frame(false)).clicked() { + step = Some(index + 1); } - // Several features identified: step through them. - if results > 1 { - if ui.add_enabled(index + 1 < results, egui::Button::new("›").frame(false)).clicked() { - step = Some(index + 1); - } - let n = (index + 1).to_string(); - let total = results.to_string(); - ui.label( - egui::RichText::new(crate::i18n::fmt("{n} of {total}", &[("n", &n), ("total", &total)])) - .size(11.0) - .color(t.text_dim), - ); - if ui.add_enabled(index > 0, egui::Button::new("‹").frame(false)).clicked() { - step = Some(index.saturating_sub(1)); - } + let n = (index + 1).to_string(); + let total = results.to_string(); + ui.label( + egui::RichText::new(crate::i18n::fmt("{n} of {total}", &[("n", &n), ("total", &total)])) + .size(11.0) + .color(t.text_dim), + ); + if ui.add_enabled(index > 0, egui::Button::new("‹").frame(false)).clicked() { + step = Some(index.saturating_sub(1)); } + } + ui.with_layout(egui::Layout::top_down(egui::Align::Min), |ui| { + ui.add(egui::Label::new(egui::RichText::new(&title).font(semibold(14.0))).truncate()).on_hover_text(&title); + ui.add(egui::Label::new(egui::RichText::new(&layer).size(11.0).color(t.text_dim)).truncate()) + .on_hover_text(&layer); }); }); }); diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index c4ab4fa43..5a02edf58 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -2917,6 +2917,52 @@ fn dynamic_text_is_a_menu_of_values_that_starts_the_text() { assert_eq!(h.state().ui.dynamic_text, None); } +#[test] +fn popup_titles_leave_room_for_controls() { + use egui::accesskit::Role; + + for title in ["Oak".to_string(), "HIDDEN INTERSECTION Tab Sign (Oversized) ".repeat(4), "長い地物名".repeat(30)] { + for count in [1, 2] { + let mut h = harness(); + let features: Vec<_> = (0..count) + .map(|_| json!({"type": "Feature", "properties": {"name": title}, "geometry": {"type": "Point", "coordinates": [0, 0]}})) + .collect(); + h.state_mut() + .run( + "layer.addDataText", + json!({ + "fileName": "popup.geojson", + "name": "A layer with a name much longer than the popup header can hold", + "text": json!({"type": "FeatureCollection", "features": features}).to_string() + }), + ) + .unwrap(); + h.state_mut().run("map.zoomToExtent", json!({"extent": [-5, -4, 5, 4]})).unwrap(); + h.ctx.data_mut(|d| d.insert_temp(egui::Id::unique("identify_popup_limit"), egui::vec2(240.0, 360.0))); + h.state_mut().run("ui.identify", json!({"lon": 0, "lat": 0})).unwrap(); + h.run_steps(6); + + let title_rect = h.query_all_by_label(&title).map(|n| n.rect()).min_by(|a, b| a.min.y.total_cmp(&b.min.y)).unwrap(); + let close = h.get_by_role_and_label(Role::Button, "×").rect(); + assert!(close.max.x <= title_rect.min.x - 12.0 + 240.0, "close must stay inside the popup"); + assert!(title_rect.max.x < close.min.x, "title must leave room for close"); + if count > 1 { + let previous = h.get_by_role_and_label(Role::Button, "‹").rect(); + assert!(title_rect.max.x < previous.min.x, "title must leave room for all navigation controls"); + h.get_by_role_and_label(Role::Button, "›").click(); + h.run_steps(3); + assert_eq!(h.state().map.popup.as_ref().unwrap().index, 1); + h.get_by_role_and_label(Role::Button, "‹").click(); + h.run_steps(3); + assert_eq!(h.state().map.popup.as_ref().unwrap().index, 0); + } + h.get_by_role_and_label(Role::Button, "×").click(); + h.run_steps(3); + assert!(h.state().map.popup.is_none(), "close must remain clickable"); + } + } +} + #[test] fn the_popup_closes_when_its_feature_is_deleted() { let mut h = harness(); diff --git a/docs/user-guide/editing.md b/docs/user-guide/editing.md index 14541f87e..198909ab7 100644 --- a/docs/user-guide/editing.md +++ b/docs/user-guide/editing.md @@ -141,7 +141,8 @@ Pop-ups (Home › Navigate) choose which layers a click with Explore identifies: under the click, every visible layer, only selectable layers, only the layer selected in Layers, or nothing. The pop-up lists every feature under the click in those layers, overlapping ones included (up to 100): step through them with ‹ ›. The feature shown is the one selected. The magnifier in -the pop-up's header zooms to it. +the pop-up's header zooms to it. Long feature titles and layer names are shortened with an +ellipsis to leave room for the header controls; hover over either name to read it in full. The topmost and visible modes also identify the features of vector tile layers under the click: PMTiles and MBTiles archives and vector tiles added as data (not catalog basemaps or MapLibre From 5e660e8b5f712b1d8f47915365c278faf70ab559 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 26 Oct 2026 17:10:30 +0100 Subject: [PATCH 2/2] test(ui): allow missing glyphs in popup layout regression --- crates/ui-egui/tests/smoke.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index 5a02edf58..e389b5850 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -2924,6 +2924,7 @@ fn popup_titles_leave_room_for_controls() { for title in ["Oak".to_string(), "HIDDEN INTERSECTION Tab Sign (Oversized) ".repeat(4), "長い地物名".repeat(30)] { for count in [1, 2] { let mut h = harness(); + h.ctx.set_missing_glyph_policy(egui::MissingGlyphPolicy::Tofu); let features: Vec<_> = (0..count) .map(|_| json!({"type": "Feature", "properties": {"name": title}, "geometry": {"type": "Point", "coordinates": [0, 0]}})) .collect();