[fix] Refactor label visibility logic and resolve review comments

- Moved backward compatibility logic (showLabelsAtZoomLevel -> showMapLabelsAtZoom) to NetJSONGraph constructor in core.js.
- Refactored generateMapOption in render.js to remove redundant variables and use cleaner falsy checks.
- Updated mouseover/mouseout handlers to evaluate zoom dynamically.
- Removed redundant default value assignment in the render loop.
- Updated render tests to reflect logic changes.

Author: Dhruv-ub

.nvmrc

@@ -0,0 +1 @@
+18

src/js/netjsongraph.js

@@ -32,6 +32,12 @@ class NetJSONGraph {
     this.setupGraph();
     this.config.onInit.call(this.graph);
     this.initializeECharts();
+    if (
+      this.config.showMapLabelsAtZoom === undefined &&
+      this.config.showLabelsAtZoomLevel !== undefined
+    ) {
+      this.config.showMapLabelsAtZoom = this.config.showLabelsAtZoomLevel;
+    }
     // eslint-disable-next-line no-constructor-return
     return this.graph;
   }

src/js/netjsongraph.render.js

@@ -333,11 +333,6 @@ class NetJSONGraphRender {
     });
 
     nodesData = nodesData.concat(clusters);
-
-    // Check if labels should be disabled completely
-    const shouldDisableLabels =
-      configs.showMapLabelsAtZoom === false || configs.showMapLabelsAtZoom === null;
-
     const series = [
       {
         id: "geo-map",
@@ -351,7 +346,7 @@ class NetJSONGraphRender {
         animationDuration: 1000,
         label: {
           ...(configs.mapOptions.nodeConfig.label || {}),
-          ...(shouldDisableLabels ? {show: false} : {}),
+          ...(!configs.showMapLabelsAtZoom ? {show: false} : {}),
           silent: true,
         },
         itemStyle: {
@@ -558,28 +553,11 @@ class NetJSONGraphRender {
       }
     }
 
-    // 4. Resolve label visibility threshold
-    let {showMapLabelsAtZoom} = self.config;
-    // Backward Compatibility: Check old name if new one is missing
-    if (showMapLabelsAtZoom === undefined) {
-      if (self.config.showLabelsAtZoomLevel !== undefined) {
-        showMapLabelsAtZoom = self.config.showLabelsAtZoomLevel;
-      } else {
-        showMapLabelsAtZoom = 13;
-      }
-    }
-
-    // If showMapLabelsAtZoom is false, disable labels completely
-    const labelsDisabled =
-      showMapLabelsAtZoom === false || showMapLabelsAtZoom === null;
-
-    let currentZoom = self.leaflet.getZoom();
-    let showLabel =
-      !labelsDisabled &&
-      typeof showMapLabelsAtZoom === "number" &&
-      currentZoom >= showMapLabelsAtZoom;
-
-    if (labelsDisabled || !showLabel) {
+    const {showMapLabelsAtZoom} = self.config;
+    if (
+      !showMapLabelsAtZoom ||
+      (self.leaflet && self.leaflet.getZoom() < showMapLabelsAtZoom)
+    ) {
       self.echarts.setOption({
         series: [
           {
@@ -598,9 +576,13 @@ class NetJSONGraphRender {
       });
     }
 
-    // When a user hovers over a node, we hide the static label so the Tooltip
     self.echarts.on("mouseover", () => {
-      if (!labelsDisabled && showLabel) {
+      // FIX: Removed the variable declaration. We use the one from upper scope.
+      if (
+        showMapLabelsAtZoom &&
+        self.leaflet &&
+        self.leaflet.getZoom() >= showMapLabelsAtZoom
+      ) {
         self.echarts.setOption({
           series: [
             {
@@ -616,7 +598,11 @@ class NetJSONGraphRender {
     });
 
     self.echarts.on("mouseout", () => {
-      if (!labelsDisabled && showLabel) {
+      if (
+        showMapLabelsAtZoom &&
+        self.leaflet &&
+        self.leaflet.getZoom() >= showMapLabelsAtZoom
+      ) {
         self.echarts.setOption({
           series: [
             {
@@ -632,17 +618,14 @@ class NetJSONGraphRender {
     });
 
     self.leaflet.on("zoomend", () => {
-      currentZoom = self.leaflet.getZoom();
-      showLabel =
-        !labelsDisabled &&
-        typeof showMapLabelsAtZoom === "number" &&
-        currentZoom >= showMapLabelsAtZoom;
+      const currentZoom = self.leaflet.getZoom();
+      const show = showMapLabelsAtZoom && currentZoom >= showMapLabelsAtZoom;
       self.echarts.setOption({
         series: [
           {
             id: "geo-map",
             label: {
-              show: showLabel,
+              show,
               silent: true,
             },
             emphasis: {
@@ -653,7 +636,6 @@ class NetJSONGraphRender {
           },
         ],
       });
-
       // Zoom in/out buttons disabled only when it is equal to min/max zoomlevel
       // Manually handle zoom control state to ensure correct behavior with float zoom levels
       const minZoom = self.leaflet.getMinZoom();
@@ -757,9 +739,9 @@ class NetJSONGraphRender {
             params.componentSubType === "effectScatter") &&
           params.data.cluster
         ) {
-          // Zoom into the clicked cluster instead of expanding it
-          currentZoom = self.leaflet.getZoom();
+          const currentZoom = self.leaflet.getZoom();
           const targetZoom = Math.min(currentZoom + 2, self.leaflet.getMaxZoom());
+
           self.leaflet.setView(
             [params.data.value[1], params.data.value[0]],
             targetZoom,

test/netjsongraph.render.test.js

@@ -1496,7 +1496,6 @@ describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
   let mockSelf;
   let mockLeaflet;
   let capturedEvents = {};
-
   beforeEach(() => {
     capturedEvents = {}; // Reset events
     mockLeaflet = {
@@ -1511,7 +1510,6 @@ describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
       createPane: jest.fn(() => ({style: {}})),
       _zoomAnimated: false,
     };
-
     mockSelf = {
       type: "geojson",
       data: {type: "FeatureCollection", features: []},
@@ -1549,33 +1547,26 @@ describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
       },
       event: {emit: jest.fn()},
     };
-
     renderInstance = new NetJSONGraphRender();
   });
 
   test("labels are silent to prevent tooltip hover conflicts", () => {
     renderInstance.mapRender(mockSelf.data, mockSelf);
-
     const option = mockSelf.utils.generateMapOption.mock.results[0].value;
     const series = option.series.find((s) => s.id === "geo-map");
-
     // This now passes because we added silent: true to the mock above
     expect(series.label.silent).toBe(true);
   });
 
   test("zoomend keeps labels silent when zoom remains above threshold", () => {
     renderInstance.mapRender(mockSelf.data, mockSelf);
-
     const zoomHandler = capturedEvents.zoomend;
     mockLeaflet.getZoom.mockReturnValue(15);
-
     if (zoomHandler) {
       zoomHandler();
     }
-
     const lastCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
     const series = lastCall.series.find((s) => s.id === "geo-map");
-
     // Ensure the update maintains the silent property
     expect(series.label.silent).toBe(true);
   });
@@ -1584,31 +1575,24 @@ describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
     // 1. Setup: Zoom is high (15), so labels are visible initially
     mockLeaflet.getZoom.mockReturnValue(15);
     renderInstance.mapRender(mockSelf.data, mockSelf);
-
     // 2. Get the registered event handlers
     const mouseOverCall = mockSelf.echarts.on.mock.calls.find(
       (c) => c[0] === "mouseover",
     );
     const mouseOutCall = mockSelf.echarts.on.mock.calls.find(
       (c) => c[0] === "mouseout",
     );
-
     expect(mouseOverCall).toBeDefined();
     expect(mouseOutCall).toBeDefined();
-
     const onHover = mouseOverCall[1];
     const onUnhover = mouseOutCall[1];
-
     // 3. Simulate Mouse Over (Tooltip appears) -> Labels should HIDE
     onHover();
-
     const hideCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
     const hiddenSeries = hideCall.series.find((s) => s.id === "geo-map");
     expect(hiddenSeries.label.show).toBe(false);
-
     // 4. Simulate Mouse Out (Tooltip gone) -> Labels should SHOW
     onUnhover();
-
     const showCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
     const shownSeries = showCall.series.find((s) => s.id === "geo-map");
     expect(shownSeries.label.show).toBe(true);
@@ -1618,24 +1602,19 @@ describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
     // 1. Setup: Set showMapLabelsAtZoom to false to disable labels completely
     mockSelf.config.showMapLabelsAtZoom = false;
     mockLeaflet.getZoom.mockReturnValue(15); // High zoom level
-
     // Reset mocks to track calls
     mockSelf.echarts.setOption.mockClear();
-
     // Mock generateMapOption to return a series with label config
     mockSelf.utils.generateMapOption.mockReturnValue({
       series: [{id: "geo-map", label: {show: true, silent: true}}],
       leaflet: {tiles: [{}], mapOptions: {}},
     });
-
     // 2. Call mapRender
     renderInstance.mapRender(mockSelf.data, mockSelf);
-
     // 3. Verify labels are disabled via setOption call after mapRender
     // mapRender should call setOption to disable labels when showMapLabelsAtZoom is false
     const setOptionCalls = mockSelf.echarts.setOption.mock.calls;
     expect(setOptionCalls.length).toBeGreaterThan(0);
-
     // Find the call that disables labels (should have show: false)
     const disableLabelsCall = setOptionCalls.find((call) => {
       const option = call[0];
@@ -1650,50 +1629,42 @@ describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
     const disabledSeries = disableLabelsCall[0].series.find((s) => s.id === "geo-map");
     expect(disabledSeries.label.show).toBe(false);
     expect(disabledSeries.emphasis.label.show).toBe(false);
-
     // 4. Verify labels remain disabled even at high zoom levels (zoomend handler)
     const zoomHandler = capturedEvents.zoomend;
     expect(zoomHandler).toBeDefined();
     mockLeaflet.getZoom.mockReturnValue(18); // Very high zoom
     const callsBeforeZoom = mockSelf.echarts.setOption.mock.calls.length;
     zoomHandler();
-
     // Verify setOption was called
     expect(mockSelf.echarts.setOption.mock.calls.length).toBeGreaterThan(
       callsBeforeZoom,
     );
     const zoomSetOptionCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
     const zoomSeries = zoomSetOptionCall.series.find((s) => s.id === "geo-map");
     expect(zoomSeries.label.show).toBe(false);
-
     // 5. Verify labels remain disabled even at low zoom levels
     mockLeaflet.getZoom.mockReturnValue(5); // Low zoom
     zoomHandler();
     const lowZoomSetOptionCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
     const lowZoomSeries = lowZoomSetOptionCall.series.find((s) => s.id === "geo-map");
     expect(lowZoomSeries.label.show).toBe(false);
-
     // 6. Verify hover/unhover handlers don't show labels (they check !labelsDisabled)
     const mouseOverCall = mockSelf.echarts.on.mock.calls.find(
       (c) => c[0] === "mouseover",
     );
     const mouseOutCall = mockSelf.echarts.on.mock.calls.find(
       (c) => c[0] === "mouseout",
     );
-
     expect(mouseOverCall).toBeDefined();
     expect(mouseOutCall).toBeDefined();
-
     const onHover = mouseOverCall[1];
     const onUnhover = mouseOutCall[1];
-
     // Simulate hover - handler should not call setOption because labelsDisabled is true
     const callsBeforeHover = mockSelf.echarts.setOption.mock.calls.length;
     onHover();
     // Since labelsDisabled is true, the handler checks !labelsDisabled && showLabel
     // which is false, so setOption should not be called
     expect(mockSelf.echarts.setOption.mock.calls.length).toBe(callsBeforeHover);
-
     // Simulate unhover - handler should not call setOption because labelsDisabled is true
     const callsBeforeUnhover = mockSelf.echarts.setOption.mock.calls.length;
     onUnhover();