Address follow-up review feedback on #1619

Second round of bot review comments on #1619, which merged before they could
be addressed on that branch.

- docs/notebook.md: drop the `layer_id`-taking calls from the snippet instead
  of annotating them. The fire-and-forget client can never produce an id, so
  the example was self-contradictory; the calls are now described in prose
  below it, pointing at the widget client that does return ids.
- console_api.py: give `load_geojson` the same `**style` kwargs as
  `add_geojson`, so both console entry points style in one call.
- tests/scripting-style-params.test.ts: set the history coalesce window to 0
  so the empty-style guard is actually under test (at the app's default window
  the two writes merge and the assertion passed either way — verified by
  mutating the guard), and add a case pinning the app-window behavior: a
  styled add coalesces into one undo entry, so one Ctrl+Z removes the layer
  rather than leaving an unstyled one behind.
This commit is contained in:
giswqs committed 2026-08-01 11:36:09 -04:00
1 parent 4ad82800a2
commit 6260a02ad9
3 files changed
+52 -20

No files matched your search

@@ -197,8 +197,11 @@ class _GeoLibre:
fc = _coerce_featurecollection(data)
return _js.addGeoJsonLayer(_to_js({"name": name, "geojson": fc, "style": style}))
async def load_geojson(self, url, name="GeoJSON"):
"""Fetch a GeoJSON URL and add it as a layer (async). Returns the id."""
async def load_geojson(self, url, name="GeoJSON", **style):
"""Fetch a GeoJSON URL and add it as a layer (async). Returns the id.
Takes the same style overrides as :meth:`add_geojson`.
"""
from pyodide.http import pyfetch
response = await pyfetch(url)
@@ -207,7 +210,7 @@ class _GeoLibre:
if not response.ok:
raise RuntimeError(f"Failed to fetch {url!r}: HTTP {response.status}")
fc = _coerce_featurecollection(await response.json())
return _js.addGeoJsonLayer(_to_js({"name": name, "geojson": fc}))
return _js.addGeoJsonLayer(_to_js({"name": name, "geojson": fc, "style": style}))
def remove_layer(self, layer_id):
"""Remove a layer by id."""
+7 -8
View File
@@ -36,11 +36,6 @@ m.add_geojson(
) # GeoDataFrame, dict, or JSON string
m.fit_bounds([-123, 37, -122, 38])
m.set_basemap("https://…/style.json")
# Layer-targeted calls take a layer id. This client is fire-and-forget, so
# `add_geojson` does not hand one back — see the note below.
m.set_visibility(layer_id, False)
m.remove_layer(layer_id)
```
Calls are **fire-and-forget**: each posts a command to the host app over the
@@ -51,9 +46,13 @@ client source: `backend/geolibre_server/notebook_client.py`.
> Read-back queries (e.g. `get_center`) are not exposed by this fire-and-forget
> client; they need the blocking request/reply path the `geolibre` widget uses.
> Layer ids come from the same path: this client's `add_geojson` returns
> `None`, while the widget's ([`geolibre` package](python.md)) returns the new
> layer's id — which is what the id-taking calls above expect.
The client also has layer-targeted calls — `set_visibility(layer_id, visible)`,
`set_opacity`, `set_style`, `remove_layer`, `zoom_to_layer` — but they are left
out of the snippet above because there is no way to obtain a `layer_id` here:
being fire-and-forget, this client's `add_geojson` returns `None`. Use them with
an id from the blocking [`geolibre` widget](python.md), whose `add_geojson`
returns the new layer's id.
## Driving the map from an external client (VS Code, …)
+39 -9
View File
@@ -1,6 +1,7 @@
import assert from "node:assert/strict";
import { beforeEach, describe, it } from "node:test";
import { afterEach, beforeEach, describe, it } from "node:test";
import { useAppStore } from "@geolibre/core";
import { setHistoryCoalesceMs } from "../packages/core/src/history";
import { styleParamPatch } from "../apps/geolibre-desktop/src/lib/scripting/style-params";
// The notebook client's `add_geojson(gdf, **style)` always sends a `style`
@@ -39,27 +40,56 @@ describe("styleParamPatch", () => {
});
});
/** What the `addGeoJsonLayer` handler does with a raw `style` param. */
function addGeoJsonCommand(name: string, style: unknown): string {
const layerId = useAppStore.getState().addGeoJsonLayer(name, FC);
const patch = styleParamPatch(style);
if (patch) useAppStore.getState().setLayerStyle(layerId, patch);
return layerId;
}
describe("addGeoJsonLayer command styling", () => {
beforeEach(() => {
// 0 so each store write is its own history entry: with the app's default
// coalesce window the two writes of a styled add would merge and the guard
// below would pass whether or not it works.
setHistoryCoalesceMs(0);
useAppStore.getState().newProject({ name: "Scripting" });
useAppStore.temporal.getState().clear();
});
afterEach(() => {
setHistoryCoalesceMs(400);
});
it("merges an inline style into the new layer", () => {
const layerId = useAppStore.getState().addGeoJsonLayer("Styled", FC);
const style = styleParamPatch({ fillColor: "#facc15", strokeColor: "#d97706" });
assert.ok(style);
useAppStore.getState().setLayerStyle(layerId, style);
const layerId = addGeoJsonCommand("Styled", {
fillColor: "#facc15",
strokeColor: "#d97706",
});
const layer = useAppStore.getState().layers.find((item) => item.id === layerId);
assert.equal(layer?.style?.fillColor, "#facc15");
assert.equal(layer?.style?.strokeColor, "#d97706");
});
it("costs a single undo step when no style kwargs were passed", () => {
const layerId = useAppStore.getState().addGeoJsonLayer("Plain", FC);
const style = styleParamPatch({});
if (style) useAppStore.getState().setLayerStyle(layerId, style);
it("writes nothing extra when no style kwargs were passed", () => {
const layerId = addGeoJsonCommand("Plain", {});
assert.equal(useAppStore.temporal.getState().pastStates.length, 1);
useAppStore.temporal.getState().undo();
assert.equal(
useAppStore.getState().layers.find((item) => item.id === layerId),
undefined,
);
});
it("coalesces a styled add into one undo step at the app's window", () => {
// The two writes land in the same tick, so the leading debounce in the
// store's `handleSet` records only the first: one Ctrl+Z removes the
// styled layer outright rather than leaving an unstyled one behind.
setHistoryCoalesceMs(400);
const layerId = addGeoJsonCommand("Styled", { fillColor: "#facc15" });
assert.equal(useAppStore.temporal.getState().pastStates.length, 1);
useAppStore.temporal.getState().undo();