From cbb54fbb7ec22a1ec751a0d972b7b9e5d6d20335 Mon Sep 17 00:00:00 2001 From: Benoit Simard Date: Tue, 29 Oct 2024 17:15:59 +0100 Subject: [PATCH] [layer-maplibre, layer-leaflet] fix issues on clean-up Fix : - multiple calls to the clean function leads to an error - clean function doesn't reset the sigma settings to the previous one --- packages/layer-leaflet/src/index.ts | 26 ++++++++++---- packages/layer-maplibre/src/index.ts | 25 +++++++++---- packages/test/unit/layers/leaflet.ts | 52 +++++++++++++++++++++++++++ packages/test/unit/layers/maplibre.ts | 52 +++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 packages/test/unit/layers/leaflet.ts create mode 100644 packages/test/unit/layers/maplibre.ts diff --git a/packages/layer-leaflet/src/index.ts b/packages/layer-leaflet/src/index.ts index 5374cde1f..68b8a150c 100644 --- a/packages/layer-leaflet/src/index.ts +++ b/packages/layer-leaflet/src/index.ts @@ -2,7 +2,6 @@ import Graph from "graphology"; import { Attributes } from "graphology-types"; import L, { MapOptions } from "leaflet"; import { Sigma } from "sigma"; -import { DEFAULT_SETTINGS } from "sigma/settings"; import { graphToLatlng, latlngToGraph, setSigmaRatioBounds, syncMapWithSigma, syncSigmaWithMap } from "./utils"; @@ -22,6 +21,10 @@ export default function bindLeafletLayer( getNodeLatLng?: (nodeAttributes: Attributes) => { lat: number; lng: number }; }, ) { + // Keeping data for the cleanup + let isKilled = false; + const prevSigmaSettings = sigma.getSettings(); + // Creating map container const mapContainer = document.createElement("div"); const mapContainerId = `${sigma.getContainer().id}-map`; @@ -109,12 +112,21 @@ export default function bindLeafletLayer( // Clean up function to remove everything function clean() { - map.remove(); - mapContainer.remove(); - sigma.off("afterRender", fnSyncMapWithSigma); - sigma.off("resize", fnOnResize); - sigma.setSetting("stagePadding", DEFAULT_SETTINGS.stagePadding); - sigma.setSetting("enableCameraRotation", true); + if (!isKilled) { + isKilled = true; + + map.remove(); + mapContainer.remove(); + + sigma.off("afterRender", fnSyncMapWithSigma); + sigma.off("resize", fnOnResize); + + // Reset settings + sigma.setSetting("stagePadding", prevSigmaSettings.stagePadding); + sigma.setSetting("enableCameraRotation", prevSigmaSettings.enableCameraRotation); + sigma.setSetting("minCameraRatio", prevSigmaSettings.minCameraRatio); + sigma.setSetting("maxCameraRatio", prevSigmaSettings.maxCameraRatio); + } } // When the map is ready diff --git a/packages/layer-maplibre/src/index.ts b/packages/layer-maplibre/src/index.ts index 06806416b..758eae88b 100644 --- a/packages/layer-maplibre/src/index.ts +++ b/packages/layer-maplibre/src/index.ts @@ -20,6 +20,10 @@ export default function bindMaplibreLayer( getNodeLatLng?: (nodeAttributes: Attributes) => { lat: number; lng: number }; }, ) { + // Keeping data for the cleanup + let isKilled = false; + const prevSigmaSettings = sigma.getSettings(); + // Creating map container const mapContainer = document.createElement("div"); const mapContainerId = `${sigma.getContainer().id}-map`; @@ -91,13 +95,20 @@ export default function bindMaplibreLayer( // Clean up function to remove everything function clean() { - map.off("moveend", fnSyncSigmaWithMap); - map.remove(); - mapContainer.remove(); - sigma.off("afterRender", fnSyncMapWithSigma); - sigma.off("resize", fnOnResize); - sigma.setSetting("stagePadding", DEFAULT_SETTINGS.stagePadding); - sigma.setSetting("enableCameraRotation", true); + if (!isKilled) { + isKilled = true; + + map.off("moveend", fnSyncSigmaWithMap); + map.remove(); + mapContainer.remove(); + + sigma.off("afterRender", fnSyncMapWithSigma); + sigma.off("resize", fnOnResize); + + // Reset settings + sigma.setSetting("stagePadding", prevSigmaSettings.stagePadding); + sigma.setSetting("enableCameraRotation", prevSigmaSettings.enableCameraRotation); + } } // Update the sigma graph for geospatial coords diff --git a/packages/test/unit/layers/leaflet.ts b/packages/test/unit/layers/leaflet.ts new file mode 100644 index 000000000..22b9e9b72 --- /dev/null +++ b/packages/test/unit/layers/leaflet.ts @@ -0,0 +1,52 @@ +import bindMapLayer from "@sigma/layer-leaflet"; +import Graph from "graphology"; +import { SerializedGraph } from "graphology-types"; +import Sigma from "sigma"; +import { createElement } from "sigma/utils"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +const GRAPH: Pick = { + nodes: [ + { key: "nantes", attributes: { x: 0, y: 0, lat: 47.2308, lng: 1.5566, size: 10, label: "Nantes" } }, + { key: "paris", attributes: { x: 0, y: 0, lat: 48.8567, lng: 2.351, size: 10, label: "Paris" } }, + ], + edges: [{ source: "nantes", target: "paris" }], +}; + +interface SigmaTestContext { + sigma: Sigma; +} + +beforeEach(async (context) => { + const graph = new Graph(); + graph.import(GRAPH); + const container = createElement("div", { width: "100px", height: "100px" }); + document.body.append(container); + context.sigma = new Sigma(graph, container); +}); + +afterEach(async ({ sigma }) => { + sigma.kill(); + sigma.getContainer().remove(); +}); + +describe("@sigma/layer-leaflet", () => { + test("calling clean function multiple times should work", ({ sigma }) => { + const { clean } = bindMapLayer(sigma); + expect(() => { + clean(); + clean(); + }).not.to.throw(); + }); + test("clean the map should reset sigma'settings to its previous value", ({ sigma }) => { + const prevSettings = sigma.getSettings(); + + const { clean } = bindMapLayer(sigma); + const settings = sigma.getSettings(); + + clean(); + + expect(prevSettings).not.toEqual(settings); + expect(sigma.getSettings()).toEqual(prevSettings); + }); +}); diff --git a/packages/test/unit/layers/maplibre.ts b/packages/test/unit/layers/maplibre.ts new file mode 100644 index 000000000..b6cb7113b --- /dev/null +++ b/packages/test/unit/layers/maplibre.ts @@ -0,0 +1,52 @@ +import bindMapLayer from "@sigma/layer-maplibre"; +import Graph from "graphology"; +import { SerializedGraph } from "graphology-types"; +import Sigma from "sigma"; +import { createElement } from "sigma/utils"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +const GRAPH: Pick = { + nodes: [ + { key: "nantes", attributes: { x: 0, y: 0, lat: 47.2308, lng: 1.5566, size: 10, label: "Nantes" } }, + { key: "paris", attributes: { x: 0, y: 0, lat: 48.8567, lng: 2.351, size: 10, label: "Paris" } }, + ], + edges: [{ source: "nantes", target: "paris" }], +}; + +interface SigmaTestContext { + sigma: Sigma; +} + +beforeEach(async (context) => { + const graph = new Graph(); + graph.import(GRAPH); + const container = createElement("div", { width: "100px", height: "100px" }); + document.body.append(container); + context.sigma = new Sigma(graph, container); +}); + +afterEach(async ({ sigma }) => { + sigma.kill(); + sigma.getContainer().remove(); +}); + +describe("@sigma/layer-maplibre", () => { + test("calling clean function multiple times should work", ({ sigma }) => { + const { clean } = bindMapLayer(sigma); + expect(() => { + clean(); + clean(); + }).not.to.throw(); + }); + test("clean the map should reset sigma'settings to its previous value", ({ sigma }) => { + const prevSettings = sigma.getSettings(); + + const { clean } = bindMapLayer(sigma); + const settings = sigma.getSettings(); + + clean(); + + expect(prevSettings).not.toEqual(settings); + expect(sigma.getSettings()).toEqual(prevSettings); + }); +});