fix(ui): allow selecting a field before applying unique symbology

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),

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.

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<f64>, Vec<f64>) = 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<f64> = kept.iter().map(|k| k.0).collect();

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;
         }

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;

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<Value> = 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));
     }

crates/engine/src/model.rs

@@ -223,7 +223,7 @@ pub fn validate(s: &Session, g: &ModelGraph) -> Vec<Problem> {
                 };
                 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)));
                     }

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 =

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<String> {
-    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);

crates/ui-egui/src/lib.rs

@@ -440,6 +440,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<rustgis_engine::map::Renderer>,
+    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,
@@ -612,6 +641,8 @@ pub struct UiState {
     pub copy_image: Option<egui::ColorImage>,
     /// The scale being typed into the status bar, while its box has focus.
     pub scale_text: Option<String>,
+    /// A field chosen in the Symbology pane before applying Unique Values.
+    pub symbology_pending: Option<PendingUniqueSymbology>,
 }
 
 impl UiState {
@@ -731,6 +762,7 @@ impl Default for UiState {
             layers_view: LayersView::DrawingOrder,
             dynamic_text: None,
             scale_text: None,
+            symbology_pending: None,
         }
     }
 }
@@ -1492,6 +1524,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") {
@@ -1578,7 +1615,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();

crates/ui-egui/src/panes.rs

@@ -3401,6 +3401,9 @@ fn pending_symbology(app: &RustGisApp, ctx: &egui::Context) -> Option<PendingSym
 
 /// Shows the active layer's renderer controls, including No Fill for polygon layers.
 fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) {
+    if app.ui.symbology_pending.as_ref().is_some_and(|pending| !pending.is_current(&app.session)) {
+        app.ui.symbology_pending = None;
+    }
     let Some(l) = app.session.active_layer.and_then(|id| app.session.map().layer(id)).cloned() else {
         ui.label(egui::RichText::new(tl!("Select a layer in the Layers pane.")).color(t.text_dim));
         return;
@@ -3410,6 +3413,8 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) {
         return;
     }
     let g = l.geometry_type();
+    let numeric: Vec<String> = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect();
+    let all: Vec<String> = 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));
@@ -3432,7 +3437,7 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) {
                 "graduated" => "graduated",
                 _ => "proportional",
             };
-            let sel = cur_kind == *label;
+            let sel = if app.ui.symbology_pending.is_some() { *code == "unique" } else { cur_kind == *label };
             let (r, resp) = ui.allocate_exact_size(vec2(68.0, 56.0), Sense::click());
             resp.widget_info(|| egui::WidgetInfo::selected(egui::Role::Button, true, sel, tl!(*label)));
             ui.painter().rect_filled(
@@ -3462,6 +3467,14 @@ 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 {
+                if *code == "unique" && !all.is_empty() {
+                    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(crate::PendingUniqueSymbology::new(&app.session, &l, field.name.clone()));
+                        clear_pending_symbology(ui.ctx());
+                    }
+                    continue;
+                }
                 match app.run("layer.symbology", json!({"type": code})) {
                     Err(error) => {
                         let pending = PendingSymbology {
@@ -3474,20 +3487,53 @@ fn symbology(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) {
                         };
                         ui.data_mut(|data| data.insert_temp(egui::Id::unique(PENDING_SYMBOLOGY), pending));
                     }
-                    Ok(_) => clear_pending_symbology(ui.ctx()),
+                    Ok(_) => {
+                        clear_pending_symbology(ui.ctx());
+                        app.ui.symbology_pending = None;
+                    }
                 }
             }
         }
     });
     ui.add_space(8.0);
+    if let Some(mut pending) = app.ui.symbology_pending.clone() {
+        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| {
+            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,
+                    Err(error) => {
+                        app.session.notify(error, true);
+                        false
+                    }
+                };
+            if applied {
+                app.ui.symbology_pending = None;
+            } else if selected != pending_field {
+                pending.field = selected;
+                app.ui.symbology_pending = Some(pending);
+            }
+        });
+        ui.add_space(6.0);
+    }
     if g == GeometryType::Polygon && !l.graphics {
         if ui.button(tl!("No Fill")).on_hover_text(tl!("Draw only polygon outlines, keeping their colors and widths.")).clicked() {
             let _ = app.run("layer.noFill", Value::Null);
         }
         ui.add_space(6.0);
     }
-    let numeric: Vec<String> = l.dataset.fields.iter().filter(|f| f.ty.is_numeric()).map(|f| f.name.clone()).collect();
-    let all: Vec<String> = l.dataset.fields.iter().map(|f| f.name.clone()).collect();
     if let Some(pending) = pending_symbology(app, ui.ctx()) {
         ui.label(egui::RichText::new(pending.error.as_ref()).color(ui.visuals().error_fg_color));
         let fields = if pending.kind == "unique" { &all } else { &numeric };

crates/ui-egui/tests/smoke.rs

@@ -3059,6 +3059,86 @@ 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("id", FieldType::Integer), Field::new("category", FieldType::Text), Field::new("other", FieldType::Text)],
+    );
+    ds.push(Some(Geometry::Point(Coord::new(117.0, 3.0))), vec![Value::Int(1), Value::Text("village".into()), Value::Text("town".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.step();
+    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.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();
+    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 == "other"));
+    assert_eq!(h.state().ui.symbology_pending, None);
+}
+
+#[test]
+fn unique_symbology_pending_choice_follows_its_context() {
+    for change in ["map", "project", "renderer", "undo", "external renderer", "layer"] {
+        let mut h = harness();
+        h.state_mut().run("layer.select", json!({"layer": "Cities"})).unwrap();
+        h.state_mut().run("layer.symbology", json!({"type": "unique", "field": "name"})).unwrap();
+        h.state_mut().run("layer.symbology", json!({"type": "single"})).unwrap();
+        // A second map deliberately has the same layer IDs and renderer as the first.
+        let mut other = h.state().session.map().clone();
+        other.name = "Other map".into();
+        h.state_mut().session.project.maps.push(other);
+        h.state_mut().run("ui.pane.symbology", json!(null)).unwrap();
+        h.step();
+        h.get_by_role_and_label(egui::accesskit::Role::Button, "Unique Values").click();
+        h.step();
+        assert!(h.state().ui.symbology_pending.is_some());
+        // Failed commands leave the valid choice available for retry.
+        assert!(h.state_mut().run("layer.symbology", json!({"type": "unique", "field": "missing field"})).is_err());
+        assert!(h.state().ui.symbology_pending.is_some());
+        if change != "external renderer" {
+            h.state_mut().ui.close_right(rustgis_ui_egui::RightPane::Symbology);
+        }
+        match change {
+            "map" => {
+                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();