From 68c44229926fb15c0217a6b9359eb39eb081c28d Mon Sep 17 00:00:00 2001 From: devjuhans Date: Fri, 23 Oct 2026 21:03:31 +0700 Subject: [PATCH 1/8] Fix unique symbology field selection --- crates/ui-egui/src/lib.rs | 3 +++ crates/ui-egui/src/panes.rs | 39 ++++++++++++++++++++++++++++++++--- crates/ui-egui/tests/smoke.rs | 20 ++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 65ee072aa..efa229f47 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -588,6 +588,8 @@ pub struct UiState { pub copy_image: Option, /// The scale being typed into the status bar, while its box has focus. pub scale_text: Option, + /// A field chosen in the Symbology pane before applying Unique Values. + pub symbology_pending: Option<(u32, String)>, } impl UiState { @@ -700,6 +702,7 @@ impl Default for UiState { layers_view: LayersView::DrawingOrder, dynamic_text: None, scale_text: None, + symbology_pending: None, } } } diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index bc91294cf..6013c2e8d 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3142,7 +3142,12 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { raster_symbology(app, ui, &l, t); return; } + if app.ui.symbology_pending.as_ref().is_some_and(|(id, _)| *id != l.id) { + app.ui.symbology_pending = None; + } let g = l.geometry_type(); + let numeric: Vec = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect(); + let all: Vec = l.dataset.fields.iter().map(|f| f.name.clone()).collect(); ui.label(egui::RichText::new(&l.name).font(semibold(14.0))); ui.add_space(6.0); ui.label(egui::RichText::new(tl!("Primary symbology")).size(11.5).color(t.text_dim)); @@ -3194,13 +3199,41 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let short = tl!(label).split(' ').next().unwrap_or(label); ui.painter().text(pos2(r.center().x, r.max.y - 11.0), Align2::CENTER_CENTER, short, regular(11.0), t.text); if resp.on_hover_text(tl!(*label)).clicked() && !sel { - let _ = app.run("layer.symbology", json!({"type": code})); + if *code == "unique" { + if let Some(field) = all.first() { + app.ui.symbology_pending = Some((l.id, field.clone())); + } else { + let _ = app.run("layer.symbology", json!({"type": code})); + } + } else { + let _ = app.run("layer.symbology", json!({"type": code})); + } } } }); ui.add_space(8.0); - let numeric: Vec = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect(); - let all: Vec = l.dataset.fields.iter().map(|f| f.name.clone()).collect(); + if let Some((pending_id, pending_field)) = app.ui.symbology_pending.clone() { + if pending_id == l.id { + let mut selected = pending_field.clone(); + ui.horizontal(|ui| { + ui.label(tl!("Field")); + egui::ComboBox::from_id_salt("sym_pending_field").selected_text(&selected).width(200.0).show_ui(ui, |ui| { + for field in &all { + ui.selectable_value(&mut selected, field.clone(), field); + } + }); + if ui.button(tl!("Apply")).clicked() { + if app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok() { + app.ui.symbology_pending = None; + } + } + }); + if selected != pending_field { + app.ui.symbology_pending = Some((l.id, selected)); + } + ui.add_space(6.0); + } + } match &l.renderer { Renderer::Single(s) => symbol_editor(app, ui, *s, g, t), Renderer::Unique { field, classes, ramp, .. } => { diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index 01edf66ac..08ebe7153 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -2829,6 +2829,26 @@ fn geometry_calculator_switching_back_creates_a_unique_field() { assert_eq!(active.value(0, 0).as_f64(), Some(5.0)); } +#[test] +fn unique_symbology_lets_the_user_choose_a_text_field_before_applying() { + use rustgis_ui_egui::rustgis_engine::data::{Dataset, Field, FieldType, Geometry, GeometryType, Value}; + use rustgis_ui_egui::rustgis_engine::geom::Coord; + + let mut h = harness(); + let mut ds = Dataset::new("Text attributes", GeometryType::Point, vec![Field::new("category", FieldType::Text)]); + ds.push(Some(Geometry::Point(Coord::new(117.0, 3.0))), vec![Value::Text("village".into())]); + let id = h.state_mut().session.map_mut().add_dataset(ds); + h.state_mut().session.active_layer = Some(id); + h.state_mut().run("ui.pane.symbology", json!(null)).unwrap(); + h.state_mut().ui.symbology_pending = Some((id, "category".into())); + h.step(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Apply").click(); + h.step(); + let layer = h.state().session.map().layer(id).unwrap(); + assert!(matches!(layer.renderer, rustgis_ui_egui::rustgis_engine::map::Renderer::Unique { ref field, .. } if field == "category")); + assert_eq!(h.state().ui.symbology_pending, None); +} + #[test] fn map_and_layout_tabs_rename_from_their_menus() { let mut h = harness(); From 0024612f6504b47ced0e0ff37d71fba0c3b09d57 Mon Sep 17 00:00:00 2001 From: devjuhans Date: Fri, 23 Oct 2026 21:27:23 +0700 Subject: [PATCH 2/8] Address review feedback for unique symbology picker --- crates/ui-egui/src/panes.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 6013c2e8d..149fa1de8 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3200,13 +3200,16 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.painter().text(pos2(r.center().x, r.max.y - 11.0), Align2::CENTER_CENTER, short, regular(11.0), t.text); if resp.on_hover_text(tl!(*label)).clicked() && !sel { if *code == "unique" { - if let Some(field) = all.first() { - app.ui.symbology_pending = Some((l.id, field.clone())); + let field = l.dataset.fields.iter().find(|f| !f.ty.is_numeric()).or_else(|| l.dataset.fields.first()); + if let Some(field) = field { + app.ui.symbology_pending = Some((l.id, field.name.clone())); } else { let _ = app.run("layer.symbology", json!({"type": code})); } } else { - let _ = app.run("layer.symbology", json!({"type": code})); + if app.run("layer.symbology", json!({"type": code})).is_ok() { + app.ui.symbology_pending = None; + } } } } @@ -3222,10 +3225,10 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.selectable_value(&mut selected, field.clone(), field); } }); - if ui.button(tl!("Apply")).clicked() { - if app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok() { - app.ui.symbology_pending = None; - } + if ui.button(tl!("Apply")).clicked() + && app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok() + { + app.ui.symbology_pending = None; } }); if selected != pending_field { From 3348659a52c5862d634a746df0347dc3e1e5b0b2 Mon Sep 17 00:00:00 2001 From: devjuhans Date: Fri, 23 Oct 2026 21:45:00 +0700 Subject: [PATCH 3/8] Fix pending symbology selection state --- crates/ui-egui/src/panes.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 149fa1de8..41230d38b 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3138,13 +3138,13 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.label(egui::RichText::new(tl!("Select a layer in the Layers pane.")).color(t.text_dim)); return; }; + if app.ui.symbology_pending.as_ref().is_some_and(|(id, _)| *id != l.id) { + app.ui.symbology_pending = None; + } if l.is_raster() { raster_symbology(app, ui, &l, t); return; } - if app.ui.symbology_pending.as_ref().is_some_and(|(id, _)| *id != l.id) { - app.ui.symbology_pending = None; - } let g = l.geometry_type(); let numeric: Vec = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect(); let all: Vec = l.dataset.fields.iter().map(|f| f.name.clone()).collect(); @@ -3206,10 +3206,8 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } else { let _ = app.run("layer.symbology", json!({"type": code})); } - } else { - if app.run("layer.symbology", json!({"type": code})).is_ok() { - app.ui.symbology_pending = None; - } + } else if app.run("layer.symbology", json!({"type": code})).is_ok() { + app.ui.symbology_pending = None; } } } @@ -3225,15 +3223,14 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.selectable_value(&mut selected, field.clone(), field); } }); - if ui.button(tl!("Apply")).clicked() - && app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok() - { + let applied = ui.button(tl!("Apply")).clicked() + && app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok(); + if applied { app.ui.symbology_pending = None; + } else if selected != pending_field { + app.ui.symbology_pending = Some((l.id, selected)); } }); - if selected != pending_field { - app.ui.symbology_pending = Some((l.id, selected)); - } ui.add_space(6.0); } } From 1f45e4c778af204cad9f188c719689df56bc95b0 Mon Sep 17 00:00:00 2001 From: devjuhans Date: Fri, 23 Oct 2026 22:17:35 +0700 Subject: [PATCH 4/8] Fix clippy warning in symbology pane --- crates/ui-egui/src/panes.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 41230d38b..212267e6d 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3213,26 +3213,26 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } }); ui.add_space(8.0); - if let Some((pending_id, pending_field)) = app.ui.symbology_pending.clone() { - if pending_id == l.id { - let mut selected = pending_field.clone(); - ui.horizontal(|ui| { - ui.label(tl!("Field")); - egui::ComboBox::from_id_salt("sym_pending_field").selected_text(&selected).width(200.0).show_ui(ui, |ui| { - for field in &all { - ui.selectable_value(&mut selected, field.clone(), field); - } - }); - let applied = ui.button(tl!("Apply")).clicked() - && app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok(); - if applied { - app.ui.symbology_pending = None; - } else if selected != pending_field { - app.ui.symbology_pending = Some((l.id, selected)); + if let Some((pending_id, pending_field)) = app.ui.symbology_pending.clone() + && pending_id == l.id + { + let mut selected = pending_field.clone(); + ui.horizontal(|ui| { + ui.label(tl!("Field")); + egui::ComboBox::from_id_salt("sym_pending_field").selected_text(&selected).width(200.0).show_ui(ui, |ui| { + for field in &all { + ui.selectable_value(&mut selected, field.clone(), field); } }); - ui.add_space(6.0); - } + let applied = ui.button(tl!("Apply")).clicked() + && app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok(); + if applied { + app.ui.symbology_pending = None; + } else if selected != pending_field { + app.ui.symbology_pending = Some((l.id, selected)); + } + }); + ui.add_space(6.0); } match &l.renderer { Renderer::Single(s) => symbol_editor(app, ui, *s, g, t), From 16881eb8c5a00543cf430a36b45d23727528fc21 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 24 Oct 2026 08:44:58 -0400 Subject: [PATCH 5/8] Address review feedback - Show a visible error notice when applying pending unique symbology fails, retaining the field for retry. --- crates/ui-egui/src/panes.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index e129c6eea..3086e6e11 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3464,7 +3464,13 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { } }); let applied = ui.button(tl!("Apply")).clicked() - && app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})).is_ok(); + && match app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})) { + Ok(_) => true, + Err(error) => { + app.session.notify(error, true); + false + } + }; if applied { app.ui.symbology_pending = None; } else if selected != pending_field { From 79d6c78529cea5a1fe8f3b32fe2d417cbb186621 Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 24 Oct 2026 10:07:08 -0400 Subject: [PATCH 6/8] Address CodeRabbit review feedback - Scope pending Unique Values fields to their project, map and layer. - Drop stale choices after renderer changes, including commands made while the pane is closed, and validate external edits when drawing. - Cover context changes, undo and failed-command retries in UI tests. --- crates/ui-egui/src/lib.rs | 36 ++++++++++++++++++++++++- crates/ui-egui/src/panes.rs | 16 +++++------ crates/ui-egui/tests/smoke.rs | 51 ++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 27b60a61b..4e807393e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -439,6 +439,35 @@ pub enum RightPane { Plugin, } +/// A Unique Values field choice scoped to the layer and renderer that opened it. +#[derive(Clone, Debug, PartialEq)] +pub struct PendingUniqueSymbology { + project: u64, + map: usize, + layer: u32, + renderer: std::sync::Arc, + pub field: String, +} + +impl PendingUniqueSymbology { + fn new(session: &Session, layer: &rustgis_engine::map::Layer, field: String) -> Self { + Self { + project: session.opened, + map: session.project.active_map, + layer: layer.id, + renderer: std::sync::Arc::new(layer.renderer.clone()), + field, + } + } + + fn is_current(&self, session: &Session) -> bool { + self.project == session.opened + && self.map == session.project.active_map + && Some(self.layer) == session.active_layer + && session.map().layer(self.layer).is_some_and(|layer| layer.renderer == *self.renderer) + } +} + pub struct UiState { pub tab: String, pub backstage: bool, @@ -610,7 +639,7 @@ pub struct UiState { /// The scale being typed into the status bar, while its box has focus. pub scale_text: Option, /// A field chosen in the Symbology pane before applying Unique Values. - pub symbology_pending: Option<(u32, String)>, + pub symbology_pending: Option, } impl UiState { @@ -1491,6 +1520,11 @@ impl RustGisApp { if let Err(e) = &r { log::info!("{id}: {e}"); } + // Commands can replace the project, switch maps or change the renderer while the pane + // is closed. A field picked for the old context must not overwrite the new renderer. + if self.ui.symbology_pending.as_ref().is_some_and(|pending| !pending.is_current(&self.session)) { + self.ui.symbology_pending = None; + } // A project replacing this one hasn't been saved in the browser yet (opening a file // names it again). if r.is_ok() && matches!(id, "project.new" | "project.sample" | "project.open" | "project.openText" | "project.fromTemplate") { diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 7aac1af5a..84b6c2943 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3373,13 +3373,13 @@ fn pending_symbology(app: &RustGisApp, ctx: &egui::Context) -> Option { + h.state_mut().run("map.activate", json!({"name": "Other map"})).unwrap(); + } + "project" => { + h.state_mut().run("project.sample", json!(null)).unwrap(); + } + "renderer" => { + h.state_mut().run("layer.symbology", json!({"type": "unique", "field": "name"})).unwrap(); + } + "undo" => { + h.state_mut().run("edit.undo", json!(null)).unwrap(); + } + "external renderer" => { + // Commands from other clients can bypass RustGisApp::run. + h.state_mut().session.run("layer.symbology", &json!({"type": "unique", "field": "name"})).unwrap(); + h.step(); + } + "layer" => { + h.state_mut().run("layer.select", json!({"layer": "Countries"})).unwrap(); + } + _ => unreachable!(), + } + assert!(h.state().ui.symbology_pending.is_none(), "pending choice survived {change}"); + } +} + #[test] fn map_and_layout_tabs_rename_from_their_menus() { let mut h = harness(); From 0a760fc1f580fd75b6840aab50242f77223d0a2c Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 24 Oct 2026 14:13:40 -0400 Subject: [PATCH 7/8] Fix workspace Clippy validation warnings Simplify boolean guards and the navigation benchmark range check without changing validation behavior. --- apps/rustgis/examples/navigate_bench.rs | 2 +- apps/rustgis/src/main.rs | 2 +- crates/analysis/src/trend.rs | 4 ++-- crates/assistant/src/agent.rs | 2 +- crates/data/src/raster/mod.rs | 2 +- crates/engine/src/cmd/select.rs | 2 +- crates/engine/src/model.rs | 2 +- crates/map/src/globe.rs | 2 +- crates/tiles/src/embeddings.rs | 2 +- crates/ui-egui/src/lib.rs | 2 +- 10 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/rustgis/examples/navigate_bench.rs b/apps/rustgis/examples/navigate_bench.rs index 2d519766f..e0c5fc560 100644 --- a/apps/rustgis/examples/navigate_bench.rs +++ b/apps/rustgis/examples/navigate_bench.rs @@ -78,7 +78,7 @@ impl eframe::App for Bench { // From the zoom's start, all of it, however long a frame took. let dt = (t - self.last_wheel.max(LOAD + PAN)).max(0.0); self.last_wheel = t; - let dir = if z < ZOOM / 3.0 || z >= 2.0 * ZOOM / 3.0 { -1.0 } else { 1.0 }; + let dir = if !(ZOOM / 3.0..2.0 * ZOOM / 3.0).contains(&z) { -1.0 } else { 1.0 }; input.events.push(egui::Event::MouseWheel { unit: egui::MouseWheelUnit::Point, delta: egui::vec2(0.0, (dir * 600.0 * dt) as f32), diff --git a/apps/rustgis/src/main.rs b/apps/rustgis/src/main.rs index 2377d413e..d57c2cd65 100644 --- a/apps/rustgis/src/main.rs +++ b/apps/rustgis/src/main.rs @@ -309,7 +309,7 @@ fn main() -> eframe::Result { let files: Vec<&String> = args .iter() .enumerate() - .filter(|(i, a)| !a.starts_with("--") && !(*i > 0 && args.get(i - 1).is_some_and(|p| p == "--backend"))) + .filter(|(i, a)| !(a.starts_with("--") || *i > 0 && args.get(i - 1).is_some_and(|p| p == "--backend"))) .map(|(_, a)| a) .collect(); // Installed plugins, before the first session lists commands and tools. diff --git a/crates/analysis/src/trend.rs b/crates/analysis/src/trend.rs index 73eb4873d..4db29388c 100644 --- a/crates/analysis/src/trend.rs +++ b/crates/analysis/src/trend.rs @@ -111,7 +111,7 @@ pub fn trend_with(t: &[f64], y: &[f64], test: Test, alpha: f64, slopes: &mut Vec kept.sort_by(|a, b| a.0.total_cmp(&b.0)); let (t, y): (Vec, Vec) = kept.into_iter().unzip(); let n = y.len(); - if !(MIN_VALUES..=MAX_VALUES).contains(&n) || !(alpha > 0.0 && alpha < 1.0) { + if !((MIN_VALUES..=MAX_VALUES).contains(&n) && alpha > 0.0 && alpha < 1.0) { return None; } if let Test::Seasonal(m) = test { @@ -222,7 +222,7 @@ pub fn trend_seasonal_with(t: &[f64], y: &[f64], season: &[usize], alpha: f64, s let mut kept: Vec<(f64, f64, usize)> = t.iter().zip(y).zip(season).filter(|((t, y), _)| t.is_finite() && y.is_finite()).map(|((t, y), s)| (*t, *y, *s)).collect(); kept.sort_by(|a, b| a.0.total_cmp(&b.0)); - if !(MIN_VALUES..=MAX_VALUES).contains(&kept.len()) || !(alpha > 0.0 && alpha < 1.0) { + if !((MIN_VALUES..=MAX_VALUES).contains(&kept.len()) && alpha > 0.0 && alpha < 1.0) { return None; } let t: Vec = kept.iter().map(|k| k.0).collect(); diff --git a/crates/assistant/src/agent.rs b/crates/assistant/src/agent.rs index 91a06c97f..c793244ec 100644 --- a/crates/assistant/src/agent.rs +++ b/crates/assistant/src/agent.rs @@ -209,7 +209,7 @@ impl Agent { fn local_delegation(&self, call: &ToolCall) -> bool { if call.name != "delegate" || self.depth != 0 - || !call.args.get("task").and_then(serde_json::Value::as_str).is_some_and(|s| !s.trim().is_empty()) + || call.args.get("task").and_then(serde_json::Value::as_str).is_none_or(|s| s.trim().is_empty()) { return false; } diff --git a/crates/data/src/raster/mod.rs b/crates/data/src/raster/mod.rs index 24206647f..5335a5158 100644 --- a/crates/data/src/raster/mod.rs +++ b/crates/data/src/raster/mod.rs @@ -400,7 +400,7 @@ impl Raster { if self.georeferenced { return Err(DataError::Format("only a raster with no coordinates can be placed".into())); } - if !bbox.iter().all(|v| v.is_finite()) || !(w < e && s < n) { + if !(bbox.iter().all(|v| v.is_finite()) && w < e && s < n) { return Err(DataError::Format("the box is [west, south, east, north]".into())); } let old = self.transform; diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 8f5fb2544..a14b49e10 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -218,7 +218,7 @@ fn identify(s: &mut Session, p: &Value) -> CmdResult { let map = s.map(); let mut results: Vec = hits.iter().filter_map(|(id, row)| feature_json(map, *id, *row)).collect(); // Then the features of vector tile layers, beneath the layers; tiles are never selectable. - if matches!(mode, PopupMode::Topmost | PopupMode::Visible) && !(first_layer && !results.is_empty()) { + if matches!(mode, PopupMode::Topmost | PopupMode::Visible) && (!first_layer || results.is_empty()) { let room = MAX_IDENTIFIED.saturating_sub(results.len()); results.extend(tile_features(s, lon, lat, tol_px, first_layer, room)); } diff --git a/crates/engine/src/model.rs b/crates/engine/src/model.rs index 9593a843d..977669d17 100644 --- a/crates/engine/src/model.rs +++ b/crates/engine/src/model.rs @@ -223,7 +223,7 @@ pub fn validate(s: &Session, g: &ModelGraph) -> Vec { }; for p in t.params.iter().filter(|p| p.required) { let port = port_kind(p.kind).is_some(); - if !(port && wired(p.name)) && !is_set(n.params.get(p.name)) && p.default.is_empty() { + if !(port && wired(p.name) || is_set(n.params.get(p.name))) && p.default.is_empty() { let msg = if port { "\"{p}\" needs a connection or a layer." } else { "\"{p}\" needs a value." }; out.push(problem("missing-input", Some(&n.id), msg.replace("{p}", p.label))); } diff --git a/crates/map/src/globe.rs b/crates/map/src/globe.rs index c9ae4a565..ed0699e56 100644 --- a/crates/map/src/globe.rs +++ b/crates/map/src/globe.rs @@ -560,7 +560,7 @@ impl Globe { pub fn drape_mesh(&self, merc: &Extent) -> DrapeMesh { use rustgis_geom::merc_to_lonlat; let mut out = DrapeMesh::default(); - if merc.is_empty() || !(merc.width() > 0.0 && merc.height() > 0.0) || !merc.width().is_finite() || !merc.height().is_finite() { + if !(!merc.is_empty() && merc.width() > 0.0 && merc.height() > 0.0 && merc.width().is_finite() && merc.height().is_finite()) { return out; } let ll = diff --git a/crates/tiles/src/embeddings.rs b/crates/tiles/src/embeddings.rs index a83af5400..dda9b6ee9 100644 --- a/crates/tiles/src/embeddings.rs +++ b/crates/tiles/src/embeddings.rs @@ -403,7 +403,7 @@ fn utm_zone(lon: f64, lat: f64) -> u32 { /// The Sentinel-2 (MGRS 100 km) tile holding a place, such as `17SKV`; `None` outside MGRS's /// UTM latitudes (80° S to 84° N). pub fn mgrs_tile(lon: f64, lat: f64) -> Option { - if !(lon.is_finite() && lat.is_finite()) || !(-80.0..84.0).contains(&lat) { + if !(lon.is_finite() && lat.is_finite() && (-80.0..84.0).contains(&lat)) { return None; } let zone = utm_zone(lon, lat); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index a48d3b376..118d5ca0e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1614,7 +1614,7 @@ impl RustGisApp { let active = self.session.active_layer.take(); let needs = (spec.enabled)(&self.session).is_some(); self.session.active_layer = active; - if !defaults && !(needs && (spec.enabled)(&self.session).is_none()) { + if !(defaults || needs && (spec.enabled)(&self.session).is_none()) { return None; } let map = self.session.map(); From bb59e6d7f5994a21481ada5ddd235902fcf4043f Mon Sep 17 00:00:00 2001 From: giswqs Date: Sat, 24 Oct 2026 14:13:40 -0400 Subject: [PATCH 8/8] Address CodeRabbit review feedback Associate the pending Unique Values field selector with its visible label. Exercise the accessible label in the existing field-selection UI test. --- crates/ui-egui/src/panes.rs | 17 +++++++++++------ crates/ui-egui/tests/smoke.rs | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 8a94cabff..9897e54ba 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -3500,12 +3500,17 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { let pending_field = pending.field.clone(); let mut selected = if all.contains(&pending_field) { pending_field.clone() } else { all.first().cloned().unwrap_or_default() }; ui.horizontal(|ui| { - ui.label(tl!("Field")); - egui::ComboBox::from_id_salt("sym_pending_field").selected_text(&selected).width(200.0).show_ui(ui, |ui| { - for field in &all { - ui.selectable_value(&mut selected, field.clone(), field); - } - }); + let input_label = ui.label(tl!("Field")); + egui::ComboBox::from_id_salt("sym_pending_field") + .selected_text(&selected) + .width(200.0) + .show_ui(ui, |ui| { + for field in &all { + ui.selectable_value(&mut selected, field.clone(), field); + } + }) + .response + .labelled_by(input_label.id); let applied = ui.button(tl!("Apply")).clicked() && match app.run("layer.symbology", json!({"type": "unique", "field": selected.clone()})) { Ok(_) => true, diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index 3ded7597c..61bb5c73a 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -3078,7 +3078,7 @@ fn unique_symbology_lets_the_user_choose_a_text_field_before_applying() { h.get_by_role_and_label(egui::accesskit::Role::Button, "Unique Values").click(); h.step(); assert_eq!(h.state().ui.symbology_pending.as_ref().map(|pending| pending.field.as_str()), Some("category")); - h.query_all_by_role(egui::accesskit::Role::ComboBox).find(|n| n.value().as_deref() == Some("category")).unwrap().click(); + h.get_by_role_and_label(egui::accesskit::Role::ComboBox, "Field").click(); h.step(); h.step(); // Combo popups enter the accessibility tree on the following frame. h.get_by_label("other").click();