Skip to content

Commit 4ad8280

Browse files
authored
Support inline GeoJSON styles in notebook bridge (#1619)
* fix: support inline GeoJSON styles in notebooks * fix: skip empty notebook style updates * Address review feedback - Extract the empty-style guard in `addGeoJsonLayer` into a documented `styleParamPatch` helper (`scripting/style-params.ts`), so the rule that a `{}` payload writes nothing is testable on its own. The scripting module itself pulls in the plugin/map stack (and its CSS) and cannot be imported under `node --test`. - Add `tests/scripting-style-params.test.ts`: covers the patch helper and replays what the handler does against the real store — the style merges onto the new layer, and a style-less call still costs exactly one undo step. - docs/notebook.md: flag that the fire-and-forget client returns no layer id, so the `layer_id`-taking calls in the snippet need one from the blocking widget client. * Address Claude review feedback - Give the in-app Pyodide console's `add_geojson` the same `**style` kwargs as the notebook client, so one-call styling behaves identically across both Python entry points (flagged in the review summary as an inconsistency). * Document the console's inline add_geojson styles - Update the Python console user guide for the `**style` kwargs added to `console_api.add_geojson` in c3c5b22.
1 parent c93327f commit 4ad8280

8 files changed

Lines changed: 142 additions & 10 deletions

File tree

apps/geolibre-desktop/src/lib/pyodide/console_api.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,15 @@ def get_layer(self, layer_id):
187187
return layer
188188
raise ValueError(f"No layer with id {layer_id!r}")
189189

190-
def add_geojson(self, data, name="GeoJSON"):
190+
def add_geojson(self, data, name="GeoJSON", **style):
191191
"""Add a GeoJSON layer from a dict / geometry / ``__geo_interface__``.
192192
193+
Style overrides (e.g. ``fillColor="#facc15"``) are applied to the new
194+
layer in the same call, matching the notebook client's ``add_geojson``.
193195
Returns the new layer id. For a remote URL use :meth:`load_geojson`.
194196
"""
195197
fc = _coerce_featurecollection(data)
196-
return _js.addGeoJsonLayer(_to_js({"name": name, "geojson": fc}))
198+
return _js.addGeoJsonLayer(_to_js({"name": name, "geojson": fc, "style": style}))
197199

198200
async def load_geojson(self, url, name="GeoJSON"):
199201
"""Fetch a GeoJSON URL and add it as a layer (async). Returns the id."""

apps/geolibre-desktop/src/lib/scripting/scriptingApi.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { Feature, FeatureCollection } from "geojson";
1212
import type { MapController } from "@geolibre/map";
1313
import { beginProcessingRun } from "../processing-history";
1414
import { captureMapImage } from "../print-layout-export";
15+
import { styleParamPatch } from "./style-params";
1516

1617
// The scripting command surface, shared by every programmatic entry point: the
1718
// Jupyter widget's postMessage bridge (useCommandBridge) and the in-app Python
@@ -129,7 +130,12 @@ export function createScriptingHandlers(deps: ScriptingDeps): ScriptingHandlers
129130
addGeoJsonLayer: (params) => {
130131
const name = String(params.name ?? "GeoJSON");
131132
const geojson = params.geojson as FeatureCollection;
132-
return useAppStore.getState().addGeoJsonLayer(name, geojson);
133+
const layerId = useAppStore.getState().addGeoJsonLayer(name, geojson);
134+
const style = styleParamPatch(params.style);
135+
if (style) {
136+
useAppStore.getState().setLayerStyle(layerId, style);
137+
}
138+
return layerId;
133139
},
134140
removeLayer: (params) => {
135141
useAppStore.getState().removeLayer(requireLayerId(params));
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Normalize a `style` command param into a layer-style patch.
3+
*
4+
* The Python notebook client collects style kwargs with `**style`, so it always
5+
* sends a `style` object — `{}` when the caller passed none. Applying an empty
6+
* patch would still call `setLayerStyle`, which rebuilds the layer object and
7+
* pushes a no-op undo entry, so a plain `add_geojson(gdf, name=…)` would cost
8+
* two undo steps instead of one. Returning `null` for anything that isn't a
9+
* non-empty plain object lets callers skip the store write entirely.
10+
*
11+
* @param value - The raw `style` param, from an untrusted command payload.
12+
* @returns The patch to merge, or `null` when there is nothing to apply.
13+
*/
14+
export function styleParamPatch(value: unknown): Record<string, unknown> | null {
15+
if (!value || typeof value !== "object" || Array.isArray(value)) return null;
16+
const style = value as Record<string, unknown>;
17+
return Object.keys(style).length > 0 ? style : null;
18+
}

backend/geolibre_server/notebook_client.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,17 +361,23 @@ def set_basemap(self, url: str) -> None:
361361

362362
# -- layers ---------------------------------------------------------------
363363

364-
def add_geojson(self, data: Any, name: str = "GeoJSON") -> None:
364+
def add_geojson(self, data: Any, name: str = "GeoJSON", **style: Any) -> None:
365365
"""Add a GeoJSON layer.
366366
367367
Args:
368368
data: A FeatureCollection/Feature/geometry dict, a JSON string, or
369369
any object with ``__geo_interface__`` (e.g. a GeoDataFrame).
370370
name: Layer display name.
371+
**style: Style overrides applied when the layer is created (e.g.
372+
``fillColor="#facc15"`` or ``strokeColor="#d97706"``).
371373
"""
372374
_send(
373375
"addGeoJsonLayer",
374-
{"name": name, "geojson": _to_featurecollection(data)},
376+
{
377+
"name": name,
378+
"geojson": _to_featurecollection(data),
379+
"style": dict(style),
380+
},
375381
)
376382

377383
def add_marker(
@@ -391,8 +397,8 @@ def add_markers(self, points: Iterable[Any], *, name: str = "Markers") -> None:
391397
{"name": name, "geojson": _points_to_featurecollection(points)},
392398
)
393399

394-
# add_circle_markers is an alias today (styling is applied via set_style or
395-
# the Style panel); kept for parity with the geolibre package's vocabulary.
400+
# add_circle_markers is an alias today; kept for parity with the geolibre
401+
# package's vocabulary.
396402
add_circle_markers = add_markers
397403

398404
def remove_layer(self, layer_id: str) -> None:

backend/geolibre_server/tests/test_notebook_client.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,22 @@ def test_add_geojson_wraps_a_bare_geometry(relay, displays):
178178
params = json.loads(relay.calls[0].data)["params"]
179179
assert params["name"] == "Pin"
180180
assert params["geojson"]["features"][0]["geometry"]["coordinates"] == [1, 2]
181+
assert params["style"] == {}
182+
183+
184+
def test_add_geojson_sends_inline_style_overrides(relay, displays):
185+
notebook_client.HostMap().add_geojson(
186+
{"type": "FeatureCollection", "features": []},
187+
name="Major Cities",
188+
fillColor="#facc15",
189+
strokeColor="#d97706",
190+
)
191+
192+
params = json.loads(relay.calls[0].data)["params"]
193+
assert params["style"] == {
194+
"fillColor": "#facc15",
195+
"strokeColor": "#d97706",
196+
}
181197

182198

183199
def test_add_markers_builds_a_point_collection(relay, displays):

docs/notebook.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,17 @@ import geolibre
2828

2929
m = geolibre.connect() # or geolibre.Map()
3030
m.fly_to(-122.4, 37.8, zoom=11) # animate the live map in the left pane
31-
m.add_geojson(gdf, name="My layer") # GeoDataFrame, dict, or JSON string
31+
m.add_geojson(
32+
gdf,
33+
name="My layer",
34+
fillColor="#facc15",
35+
strokeColor="#d97706",
36+
) # GeoDataFrame, dict, or JSON string
3237
m.fit_bounds([-123, 37, -122, 38])
3338
m.set_basemap("https://…/style.json")
39+
40+
# Layer-targeted calls take a layer id. This client is fire-and-forget, so
41+
# `add_geojson` does not hand one back — see the note below.
3442
m.set_visibility(layer_id, False)
3543
m.remove_layer(layer_id)
3644
```
@@ -43,6 +51,9 @@ client source: `backend/geolibre_server/notebook_client.py`.
4351

4452
> Read-back queries (e.g. `get_center`) are not exposed by this fire-and-forget
4553
> client; they need the blocking request/reply path the `geolibre` widget uses.
54+
> Layer ids come from the same path: this client's `add_geojson` returns
55+
> `None`, while the widget's ([`geolibre` package](python.md)) returns the new
56+
> layer's id — which is what the id-taking calls above expect.
4657
4758
## Driving the map from an external client (VS Code, …)
4859

docs/user-guide/python-console.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ define in a script is immediately usable in the console, and vice-versa.
6363
```python
6464
# Add data (a GeoJSON dict, a geometry, or anything with __geo_interface__)
6565
layer_id = geolibre.add_geojson(
66-
{"type": "Point", "coordinates": [-122.4, 37.8]}, name="Pin"
66+
{"type": "Point", "coordinates": [-122.4, 37.8]},
67+
name="Pin",
68+
fillColor="#facc15", # style overrides are applied in the same call
6769
)
6870

6971
# Style, toggle, and inspect layers
@@ -121,7 +123,7 @@ is available this way.
121123
| `fit_bounds([w, s, e, n])` | Fit the camera to a bounding box. |
122124
| `set_basemap(url)` | Set the basemap style (an http(s) or root-relative URL). |
123125
| `identify(lng, lat, layer_id=None)` | Query rendered features at a point (like a click). |
124-
| `add_geojson(data, name=)` | Add a layer from a GeoJSON dict / geometry / `__geo_interface__`; returns the layer id. |
126+
| `add_geojson(data, name=, **style)` | Add a layer from a GeoJSON dict / geometry / `__geo_interface__`, with optional inline style overrides; returns the layer id. |
125127
| `await load_geojson(url, name=)` | Fetch a GeoJSON URL and add it; returns the layer id. |
126128
| `layers` | List of [`Layer`](#layer) objects, in draw order. |
127129
| `get_layer(layer_id)` | The `Layer` with that id (raises if absent). |
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import assert from "node:assert/strict";
2+
import { beforeEach, describe, it } from "node:test";
3+
import { useAppStore } from "@geolibre/core";
4+
import { styleParamPatch } from "../apps/geolibre-desktop/src/lib/scripting/style-params";
5+
6+
// The notebook client's `add_geojson(gdf, **style)` always sends a `style`
7+
// object, so the scripting handler's `addGeoJsonLayer` decides — via
8+
// `styleParamPatch` — whether that payload is worth a store write at all. The
9+
// handler module itself pulls in the whole plugin/map stack (and its CSS), so
10+
// these tests exercise the decision helper directly and then replay what the
11+
// handler does with it against the real store.
12+
13+
const FC = {
14+
type: "FeatureCollection" as const,
15+
features: [
16+
{
17+
type: "Feature" as const,
18+
properties: {},
19+
geometry: { type: "Point" as const, coordinates: [0, 0] },
20+
},
21+
],
22+
};
23+
24+
describe("styleParamPatch", () => {
25+
it("keeps a non-empty style object", () => {
26+
const style = { fillColor: "#facc15", strokeWidth: 2 };
27+
assert.deepEqual(styleParamPatch(style), style);
28+
});
29+
30+
it("rejects an empty object, so a style-less call writes nothing", () => {
31+
assert.equal(styleParamPatch({}), null);
32+
});
33+
34+
it("rejects missing and non-object payloads", () => {
35+
assert.equal(styleParamPatch(undefined), null);
36+
assert.equal(styleParamPatch(null), null);
37+
assert.equal(styleParamPatch("fillColor"), null);
38+
assert.equal(styleParamPatch(["fillColor"]), null);
39+
});
40+
});
41+
42+
describe("addGeoJsonLayer command styling", () => {
43+
beforeEach(() => {
44+
useAppStore.getState().newProject({ name: "Scripting" });
45+
useAppStore.temporal.getState().clear();
46+
});
47+
48+
it("merges an inline style into the new layer", () => {
49+
const layerId = useAppStore.getState().addGeoJsonLayer("Styled", FC);
50+
const style = styleParamPatch({ fillColor: "#facc15", strokeColor: "#d97706" });
51+
assert.ok(style);
52+
useAppStore.getState().setLayerStyle(layerId, style);
53+
54+
const layer = useAppStore.getState().layers.find((item) => item.id === layerId);
55+
assert.equal(layer?.style?.fillColor, "#facc15");
56+
assert.equal(layer?.style?.strokeColor, "#d97706");
57+
});
58+
59+
it("costs a single undo step when no style kwargs were passed", () => {
60+
const layerId = useAppStore.getState().addGeoJsonLayer("Plain", FC);
61+
const style = styleParamPatch({});
62+
if (style) useAppStore.getState().setLayerStyle(layerId, style);
63+
64+
assert.equal(useAppStore.temporal.getState().pastStates.length, 1);
65+
useAppStore.temporal.getState().undo();
66+
assert.equal(
67+
useAppStore.getState().layers.find((item) => item.id === layerId),
68+
undefined,
69+
);
70+
});
71+
});

0 commit comments

Comments
 (0)