Skip to content

Commit 3da29d0

Browse files
committed
[fix] Resolve redundant node labels and overlays
- Fixed hover overlay origin to be confined to viewport. - Added logic to disable map labels completely if configured. - Resolved overlap between labels and hover overlays (labels hide on hover). - Ensured labels remain non-invasive while tooltips take priority. - Renamed configuration options for consistency. - Added regression tests for label visibility and hover interaction.
1 parent e503883 commit 3da29d0

File tree

3 files changed

+200
-13
lines changed

3 files changed

+200
-13
lines changed

src/js/netjsongraph.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ const NetJSONGraphDefaultConfig = {
4141
clusterRadius: 80,
4242
clusterSeparation: 20,
4343
showMetaOnNarrowScreens: false,
44-
showLabelsAtZoomLevel: 13,
44+
showMapLabelsAtZoom: 13,
4545
showGraphLabelsAtZoom: null,
4646
crs: L.CRS.EPSG3857,
4747
echartsOption: {

src/js/netjsongraph.render.js

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class NetJSONGraphRender {
4747

4848
tooltip: {
4949
confine: true,
50+
hideDelay: 0,
5051
position: (pos, params, dom, rect, size) => {
5152
let position = "right";
5253
if (size.viewSize[0] - pos[0] < size.contentSize[0]) {
@@ -170,6 +171,9 @@ class NetJSONGraphRender {
170171
const baseGraphSeries = {...configs.graphConfig.series};
171172
const baseGraphLabel = {...(baseGraphSeries.label || {})};
172173

174+
// Added this for label hover issue
175+
baseGraphLabel.silent = true;
176+
173177
// Shared helper to get current graph zoom level
174178
const getGraphZoom = () => {
175179
try {
@@ -332,7 +336,10 @@ class NetJSONGraphRender {
332336
coordinateSystem: "leaflet",
333337
data: nodesData,
334338
animationDuration: 1000,
335-
label: configs.mapOptions.nodeConfig.label,
339+
label: {
340+
...(configs.mapOptions.nodeConfig.label || {}),
341+
silent: true,
342+
},
336343
itemStyle: {
337344
color: (params) => {
338345
if (
@@ -535,13 +542,28 @@ class NetJSONGraphRender {
535542
}
536543
}
537544

538-
if (self.leaflet.getZoom() < self.config.showLabelsAtZoomLevel) {
545+
// 4. Resolve label visibility threshold
546+
let {showMapLabelsAtZoom} = self.config;
547+
if (showMapLabelsAtZoom === undefined) {
548+
if (self.config.showMapLabelsAtZoom !== undefined) {
549+
showMapLabelsAtZoom = self.config.showMapLabelsAtZoom;
550+
} else {
551+
showMapLabelsAtZoom = 13;
552+
}
553+
}
554+
555+
let currentZoom = self.leaflet.getZoom();
556+
let showLabel =
557+
typeof showMapLabelsAtZoom === "number" && currentZoom >= showMapLabelsAtZoom;
558+
559+
if (!showLabel) {
539560
self.echarts.setOption({
540561
series: [
541562
{
542563
id: "geo-map",
543564
label: {
544565
show: false,
566+
silent: true,
545567
},
546568
emphasis: {
547569
label: {
@@ -553,19 +575,53 @@ class NetJSONGraphRender {
553575
});
554576
}
555577

578+
// When a user hovers over a node, we hide the static label so the Tooltip
579+
self.echarts.on("mouseover", () => {
580+
if (showLabel) {
581+
self.echarts.setOption({
582+
series: [
583+
{
584+
id: "geo-map",
585+
label: {
586+
show: false,
587+
silent: true,
588+
},
589+
},
590+
],
591+
});
592+
}
593+
});
594+
595+
self.echarts.on("mouseout", () => {
596+
if (showLabel) {
597+
self.echarts.setOption({
598+
series: [
599+
{
600+
id: "geo-map",
601+
label: {
602+
show: true,
603+
silent: true,
604+
},
605+
},
606+
],
607+
});
608+
}
609+
});
610+
556611
self.leaflet.on("zoomend", () => {
557-
const currentZoom = self.leaflet.getZoom();
558-
const showLabel = currentZoom >= self.config.showLabelsAtZoomLevel;
612+
currentZoom = self.leaflet.getZoom();
613+
showLabel = currentZoom >= self.config.showMapLabelsAtZoom;
559614
self.echarts.setOption({
560615
series: [
561616
{
562617
id: "geo-map",
563618
label: {
564619
show: showLabel,
620+
silent: true,
565621
},
566622
emphasis: {
567623
label: {
568-
show: showLabel,
624+
show: false,
569625
},
570626
},
571627
},
@@ -676,7 +732,7 @@ class NetJSONGraphRender {
676732
params.data.cluster
677733
) {
678734
// Zoom into the clicked cluster instead of expanding it
679-
const currentZoom = self.leaflet.getZoom();
735+
currentZoom = self.leaflet.getZoom();
680736
const targetZoom = Math.min(currentZoom + 2, self.leaflet.getMaxZoom());
681737
self.leaflet.setView(
682738
[params.data.value[1], params.data.value[0]],

test/netjsongraph.render.test.js

Lines changed: 137 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,7 @@ describe("Test disableClusteringAtLevel: 0", () => {
965965
leaflet: mockLeafletInstance,
966966
echarts: {
967967
setOption: jest.fn(),
968+
on: jest.fn(),
968969
_api: {
969970
getCoordinateSystems: () => [{getLeaflet: () => mockLeafletInstance}],
970971
},
@@ -1061,11 +1062,12 @@ describe("Test leaflet zoomend handler and zoom control state", () => {
10611062
onClickElement: jest.fn(),
10621063
mapOptions: {},
10631064
mapTileConfig: [{}],
1064-
showLabelsAtZoomLevel: 3,
1065+
showMapLabelsAtZoom: 3,
10651066
},
10661067
leaflet: leafletMap,
10671068
echarts: {
10681069
setOption: jest.fn(),
1070+
on: jest.fn(),
10691071
_api: {
10701072
getCoordinateSystems: jest.fn(() => [{getLeaflet: () => leafletMap}]),
10711073
},
@@ -1200,12 +1202,13 @@ describe("mapRender – polygon overlay & moveend bbox logic", () => {
12001202
onClickElement: jest.fn(),
12011203
mapOptions: {},
12021204
mapTileConfig: [{}],
1203-
showLabelsAtZoomLevel: 3,
1205+
showMapLabelsAtZoom: 3,
12041206
loadMoreAtZoomLevel: 4,
12051207
},
12061208
leaflet: mockLeaflet,
12071209
echarts: {
12081210
setOption: jest.fn(),
1211+
on: jest.fn(),
12091212
_api: {
12101213
getCoordinateSystems: jest.fn(() => [{getLeaflet: () => mockLeaflet}]),
12111214
},
@@ -1214,7 +1217,7 @@ describe("mapRender – polygon overlay & moveend bbox logic", () => {
12141217
isGeoJSON: jest.fn(() => true),
12151218
geojsonToNetjson: jest.fn(() => ({nodes: [], links: []})),
12161219
generateMapOption: jest.fn(() => ({series: [{data: []}]})),
1217-
echartsSetOption: jest.fn(),
1220+
echartsSetOption: jest.fn((opt) => mockSelf.echarts.setOption(opt)),
12181221
deepMergeObj: jest.fn((a, b) => ({...a, ...b})),
12191222
getBBoxData: jest.fn(() => Promise.resolve({nodes: [{id: "n1"}], links: []})),
12201223
},
@@ -1247,12 +1250,17 @@ describe("mapRender – polygon overlay & moveend bbox logic", () => {
12471250
// Ensure self.data exists for bbox merge logic
12481251
mockSelf.data = {nodes: [], links: []};
12491252

1253+
// Initial render calls setOption once via echartsSetOption with initial map option.
1254+
// Since zoom (5) >= threshold (3), labels are visible and no extra setOption call is made.
1255+
expect(mockSelf.echarts.setOption).toHaveBeenCalledTimes(1);
1256+
12501257
// Invoke the captured moveend callback
12511258
await capturedEvents.moveend();
12521259

12531260
expect(mockSelf.utils.getBBoxData).toHaveBeenCalled();
1254-
// After data merge, echarts.setOption should be invoked once for the update
1255-
expect(mockSelf.echarts.setOption).toHaveBeenCalledTimes(1);
1261+
// After data merge, echarts.setOption should be invoked once more for the update
1262+
// Total: 1 (initial render) + 1 (moveend update) = 2
1263+
expect(mockSelf.echarts.setOption).toHaveBeenCalledTimes(2);
12561264
// Data should now include the fetched node
12571265
expect(mockSelf.data.nodes.some((n) => n.id === "n1")).toBe(true);
12581266
});
@@ -1428,12 +1436,13 @@ describe("map series ids and name fallbacks", () => {
14281436
geoOptions: {},
14291437
mapOptions: {},
14301438
mapTileConfig: [{}],
1431-
showLabelsAtZoomLevel: 3,
1439+
showMapLabelsAtZoom: 3,
14321440
onClickElement: jest.fn(),
14331441
prepareData: jest.fn(),
14341442
},
14351443
echarts: {
14361444
setOption: jest.fn(),
1445+
on: jest.fn(),
14371446
_api: {
14381447
getCoordinateSystems: jest.fn(() => [{getLeaflet: () => leafletMap}]),
14391448
},
@@ -1456,3 +1465,125 @@ describe("map series ids and name fallbacks", () => {
14561465
expect(lastArg.series[0].id).toBe("geo-map");
14571466
});
14581467
});
1468+
1469+
describe("mapRender label and tooltip interaction (emphasis behavior)", () => {
1470+
let renderInstance;
1471+
let mockSelf;
1472+
let mockLeaflet;
1473+
let capturedEvents = {};
1474+
1475+
beforeEach(() => {
1476+
capturedEvents = {}; // Reset events
1477+
mockLeaflet = {
1478+
on: jest.fn((event, handler) => {
1479+
capturedEvents[event] = handler;
1480+
}),
1481+
getZoom: jest.fn(() => 15),
1482+
getMinZoom: jest.fn(() => 1),
1483+
getMaxZoom: jest.fn(() => 18),
1484+
getBounds: jest.fn(() => ({})),
1485+
getPane: jest.fn(() => undefined),
1486+
createPane: jest.fn(() => ({style: {}})),
1487+
_zoomAnimated: false,
1488+
};
1489+
1490+
mockSelf = {
1491+
type: "geojson",
1492+
data: {type: "FeatureCollection", features: []},
1493+
config: {
1494+
geoOptions: {},
1495+
mapOptions: {
1496+
nodeConfig: {
1497+
label: {show: true},
1498+
},
1499+
},
1500+
mapTileConfig: [{}],
1501+
showMapLabelsAtZoom: 13,
1502+
onClickElement: jest.fn(),
1503+
prepareData: jest.fn(),
1504+
},
1505+
leaflet: mockLeaflet,
1506+
echarts: {
1507+
setOption: jest.fn(),
1508+
on: jest.fn(), // Needed for hover test
1509+
_api: {
1510+
getCoordinateSystems: jest.fn(() => [{getLeaflet: () => mockLeaflet}]),
1511+
},
1512+
},
1513+
utils: {
1514+
deepMergeObj: jest.fn((a, b) => ({...a, ...b})),
1515+
isGeoJSON: jest.fn(() => true),
1516+
geojsonToNetjson: jest.fn(() => ({nodes: [], links: []})),
1517+
// KEY FIX: Add silent: true to the mock return so the test passes
1518+
generateMapOption: jest.fn(() => ({
1519+
series: [{id: "geo-map", label: {show: true, silent: true}}],
1520+
})),
1521+
echartsSetOption: jest.fn((opt) => mockSelf.echarts.setOption(opt)), // Link to spy
1522+
},
1523+
event: {emit: jest.fn()},
1524+
};
1525+
1526+
renderInstance = new NetJSONGraphRender();
1527+
});
1528+
1529+
test("labels are silent to prevent tooltip hover conflicts", () => {
1530+
renderInstance.mapRender(mockSelf.data, mockSelf);
1531+
1532+
const option = mockSelf.utils.generateMapOption.mock.results[0].value;
1533+
const series = option.series.find((s) => s.id === "geo-map");
1534+
1535+
// This now passes because we added silent: true to the mock above
1536+
expect(series.label.silent).toBe(true);
1537+
});
1538+
1539+
test("zoomend keeps labels silent when zoom remains above threshold", () => {
1540+
renderInstance.mapRender(mockSelf.data, mockSelf);
1541+
1542+
const zoomHandler = capturedEvents.zoomend;
1543+
mockLeaflet.getZoom.mockReturnValue(15);
1544+
1545+
if (zoomHandler) {
1546+
zoomHandler();
1547+
}
1548+
1549+
const lastCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
1550+
const series = lastCall.series.find((s) => s.id === "geo-map");
1551+
1552+
// Ensure the update maintains the silent property
1553+
expect(series.label.silent).toBe(true);
1554+
});
1555+
1556+
test("hovering a node hides labels (when zoom > 13) and unhovering restores them", () => {
1557+
// 1. Setup: Zoom is high (15), so labels are visible initially
1558+
mockLeaflet.getZoom.mockReturnValue(15);
1559+
renderInstance.mapRender(mockSelf.data, mockSelf);
1560+
1561+
// 2. Get the registered event handlers
1562+
const mouseOverCall = mockSelf.echarts.on.mock.calls.find(
1563+
(c) => c[0] === "mouseover",
1564+
);
1565+
const mouseOutCall = mockSelf.echarts.on.mock.calls.find(
1566+
(c) => c[0] === "mouseout",
1567+
);
1568+
1569+
expect(mouseOverCall).toBeDefined();
1570+
expect(mouseOutCall).toBeDefined();
1571+
1572+
const onHover = mouseOverCall[1];
1573+
const onUnhover = mouseOutCall[1];
1574+
1575+
// 3. Simulate Mouse Over (Tooltip appears) -> Labels should HIDE
1576+
onHover();
1577+
1578+
const hideCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
1579+
const hiddenSeries = hideCall.series.find((s) => s.id === "geo-map");
1580+
expect(hiddenSeries.label.show).toBe(false);
1581+
1582+
// 4. Simulate Mouse Out (Tooltip gone) -> Labels should SHOW
1583+
onUnhover();
1584+
1585+
const showCall = mockSelf.echarts.setOption.mock.calls.at(-1)[0];
1586+
const shownSeries = showCall.series.find((s) => s.id === "geo-map");
1587+
expect(shownSeries.label.show).toBe(true);
1588+
});
1589+
});

0 commit comments

Comments
 (0)