[feature] Support custom CRS in default config #380 #381

dee077 · 2025-06-19T18:06:17Z

Checklist

I have read the OpenWISP Contributing Guidelines.
I have manually tested the changes proposed in this pull request.
I have written new test cases for new code and/or updated existing tests for changes to existing code.
I have updated the documentation.

Reference to Existing Issue

Closes #380.

Description of Changes

Added the crs field in the default config
Used L.CRS.Simple for the indoor map example and updated the location field accordingly
Added selenium test for the indoor map as well.

Added support for custom CRS and used L.CRS.Simple for the indoormap and added selenium test for the indoor map as well. Fixes #380

Make manual enforce of leaflet disable class Fixes #188

pandafy · 2025-06-23T11:04:29Z

src/js/netjsongraph.render.js

+      const minZoom = self.leaflet.getMinZoom();
+      const maxZoom = self.leaflet.getMaxZoom();
+      const zoomIn = document.querySelector(".leaflet-control-zoom-in");
+      const zoomOut = document.querySelector(".leaflet-control-zoom-out");
+
+      if (zoomIn && zoomOut) {
+        if (Math.round(currentZoom) >= maxZoom) {
+          zoomIn.classList.add("leaflet-disabled");
+        } else {
+          zoomIn.classList.remove("leaflet-disabled");
+        }
+
+        if (Math.round(currentZoom) <= minZoom) {
+          zoomOut.classList.add("leaflet-disabled");
+        } else {
+          zoomOut.classList.remove("leaflet-disabled");
+        }
+      }


Add some comments in the code why this logic is needed.

In my testing, this scenario only occurs when the min/max zoom level is not a multiple of zoomSnap.

pandafy

Good work @dee077!

Let's add some code comments to improve maintainability.

pandafy · 2025-06-23T11:07:02Z

test/netjsongraph.render.test.js

+    document
+      .querySelectorAll(".leaflet-control-zoom-in, .leaflet-control-zoom-out")
+      .forEach((el) => el.remove());
+    zoomInBtn = document.createElement("a");
+    zoomInBtn.className = "leaflet-control-zoom-in";
+    zoomOutBtn = document.createElement("a");
+    zoomOutBtn.className = "leaflet-control-zoom-out";
+    document.body.appendChild(zoomInBtn);
+    document.body.appendChild(zoomOutBtn);


Why is it required to remove the existing buttons in DOM and create new buttons?

pandafy · 2025-06-23T11:09:49Z

test/netjsongraph.render.test.js

+        disableClusteringAtLevel: 0,
+        clusterRadius: 80,


If clustering is set to false, do these options make sense?

pandafy · 2025-06-23T11:17:07Z

test/netjsongraph.render.test.js

+    expect(document.querySelector(".leaflet-control-zoom-in")).toBe(zoomInBtn);
+    expect(document.querySelector(".leaflet-control-zoom-out")).toBe(
+      zoomOutBtn,
+    );


Why both these elements should have "leaflet-control-zoom-out" class at the beginning?

pandafy · 2025-06-23T11:18:31Z

test/netjsongraph.render.test.js

Can you add code comments to state the objective of the test (and helper functions)? It is a bit confusing right now.

pandafy · 2025-06-23T11:21:35Z

public/example_templates/netjsonmap-indoormap.html

          // set map initial state.
          mapOptions: {
            center: [48.577, 18.539],
-            zoom: 5.5,
+            zoom: 1,


Let's make it 0

pandafy · 2025-06-23T11:22:21Z

public/example_templates/netjsonmap-indoormap.html

@@ -120,37 +121,31 @@
          },

          onReady: function presentIndoormap() {
-            const netjsonmap = this.leaflet;
+            const netjsonmap = this.leaflet; 


Suggested change

const netjsonmap = this.leaflet;

const netjsonmap = this.leaflet;

There was a blank space at the end of line. You can configure your code editor to automatically remove them upon save.

pandafy · 2025-06-23T11:36:21Z

public/example_templates/netjsonmap-indoormap.html

+              const bottomRight = netjsonmap.unproject([0, h * 2], zoom);
+              const upperLeft = netjsonmap.unproject([w * 2, 0], zoom);


I plotted these points on the cartesian plane

I believe the name of the variables is misleading.

…flet map instead of mocking it

dee077 · 2025-06-23T18:59:30Z

Updates

Added comments above the zoom controls.
Simplified the unit tests using the leaflet map render instead of mocking everything.

nemesifier

Looks good to me.

One favour @dee077, there's a red label appearing when hovering the mouse over the nodes which is redundant, can you remove that please?

@pandafy @niteshsinha17 let me know once you think this is ready to merge.

dee077 added 2 commits June 19, 2025 23:29

[feature] Support custom CRS #380

2f3a420

Added support for custom CRS and used L.CRS.Simple for the indoormap and added selenium test for the indoor map as well. Fixes #380

[qa] Lint fix

8ec04ff

dee077 force-pushed the feature/380-add-support-for-custom-crs branch from bd80351 to 253ada2 Compare June 19, 2025 18:21

[fix] Zoom button not disabling on float zoom value #188

34618e5

Make manual enforce of leaflet disable class Fixes #188

dee077 force-pushed the feature/380-add-support-for-custom-crs branch from 253ada2 to 34618e5 Compare June 19, 2025 18:25

[tests] Add unit tests for zoom events

b58db81

dee077 force-pushed the feature/380-add-support-for-custom-crs branch from a87e505 to b58db81 Compare June 19, 2025 20:17

dee077 added this to [GSoC25] General Map: Indoor, Mobile, Linkable URLs Jun 19, 2025

dee077 moved this to In review in [GSoC25] General Map: Indoor, Mobile, Linkable URLs Jun 19, 2025

nemesifier assigned dee077 Jun 19, 2025

pandafy reviewed Jun 23, 2025

View reviewed changes

pandafy requested changes Jun 23, 2025

View reviewed changes

pandafy mentioned this pull request Jun 23, 2025

[feature] Add floorplan map above dashboard map #564 openwisp/openwisp-monitoring#680

Open

4 tasks

[fix] Added comment for zoom handler and updated test to use real lea…

7890341

…flet map instead of mocking it

[fix] Change misleading variable names in indoormap.html

82fc8b8

dee077 force-pushed the feature/380-add-support-for-custom-crs branch from 8fc3608 to 82fc8b8 Compare June 24, 2025 17:07

nemesifier reviewed Jun 24, 2025

View reviewed changes

nemesifier added the enhancement label Jun 24, 2025

[fix] Hide label

01ba42b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[feature] Support custom CRS in default config #380 #381

[feature] Support custom CRS in default config #380 #381

Uh oh!

dee077 commented Jun 19, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy left a comment

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

pandafy Jun 23, 2025

Uh oh!

dee077 commented Jun 23, 2025 •

edited

Loading

Uh oh!

nemesifier left a comment

Uh oh!

Uh oh!

	const netjsonmap = this.leaflet;
	const netjsonmap = this.leaflet;

		const bottomRight = netjsonmap.unproject([0, h * 2], zoom);
		const upperLeft = netjsonmap.unproject([w * 2, 0], zoom);

[feature] Support custom CRS in default config #380 #381

Are you sure you want to change the base?

[feature] Support custom CRS in default config #380 #381

Uh oh!

Conversation

dee077 commented Jun 19, 2025

Checklist

Reference to Existing Issue

Description of Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandafy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dee077 commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Updates

Uh oh!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dee077 commented Jun 23, 2025 •

edited

Loading