mirror of
https://github.com/opengeos/GeoLibre.git
synced 2026-11-06 17:45:23 +00:00
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:
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
@@ -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, …)
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user