Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions public/example_templates/netjsonmap-indoormap-overlay.html
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@
enabled: true,
id: "indoorMap",
zoomOnRestore: false,
preserveFragment: true,
},
prepareData: (data) => {
data.nodes.forEach((n) => {
Expand Down
1 change: 1 addition & 0 deletions src/js/netjsongraph.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ const NetJSONGraphDefaultConfig = {
id: null,
zoomOnRestore: true,
zoomLevel: null,
preserveFragment: false,
},

/**
Expand Down
12 changes: 8 additions & 4 deletions src/js/netjsongraph.gui.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ class NetJSONGraphGUI {
console.error("Node location not available. Cannot load popup.");
return;
}
const bookmarkableActionId =
self.config.bookmarkableActions && self.config.bookmarkableActions.id;
const {bookmarkableActions: {id: bookmarkableActionId, preserveFragment} = {}} =
self.config;
const popupRequest = {};
self.leaflet.currentPopupRequest = popupRequest;
// Track whether tooltip/labels were already hidden by an open popup, so
Expand Down Expand Up @@ -354,7 +354,7 @@ class NetJSONGraphGUI {
if (self.leaflet.currentPopupRequest !== popupRequest) {
return;
}
self.utils.removeUrlFragment(bookmarkableActionId, "nodeId");
self.utils.removeUrlFragment(bookmarkableActionId, "nodeId", preserveFragment);
self.leaflet.currentPopupRequest = null;
// If we tore down a previous popup before content generation failed,
// its remove handler was bypassed — restore tooltip/labels manually so
Expand Down Expand Up @@ -397,7 +397,11 @@ class NetJSONGraphGUI {
const fragments = self.utils.parseUrlFragments();
const currentFragment = fragments[bookmarkableActionId];
if (currentFragment && currentFragment.get("nodeId") === popupNodeId) {
self.utils.removeUrlFragment(bookmarkableActionId, "nodeId");
self.utils.removeUrlFragment(
bookmarkableActionId,
"nodeId",
preserveFragment,
);
}
}
self.leaflet.currentPopup = null;
Expand Down
21 changes: 16 additions & 5 deletions src/js/netjsongraph.util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,16 @@ class NetJSONGraphUtil {
.map((urlParams) => urlParams.toString())
.join(";");

// Remove dangling "#" when no fragments remain.
if (!newHash) {
window.history.pushState(
state,
"",
window.location.pathname + window.location.search,
);
return;
}

// We store the selected node's data to the browser's history state.
// This allows the node's information to be retrieved instantly on a back/forward
// button click without needing to re-parse the entire nodes list.
Expand Down Expand Up @@ -1336,18 +1346,19 @@ class NetJSONGraphUtil {
* @param {string} [paramName] If provided, only this query-param is removed
* from the fragment. If omitted, the whole fragment for the id is dropped.
*/
removeUrlFragment(id, paramName = null) {
removeUrlFragment(id, paramName = null, preserveFragment = false) {
const fragments = this.parseUrlFragments();
if (!fragments[id]) {
return;
}
if (paramName) {
fragments[id].delete(paramName);
// Drop the whole entry if only the bare action id is left — a fragment
// like "#id=geoMap" with no other params is a useless stub that
// parseUrlFragments would still pick up on subsequent visits.
// Remove the entire fragment if only the bare action id remains,
// unless preserveFragment is enabled. Some consumers (e.g. indoor
// overlays) use the fragment itself as meaningful UI state even
// without additional params like nodeId.
const remainingKeys = Array.from(fragments[id].keys()).filter((k) => k !== "id");
if (remainingKeys.length === 0) {
if (!preserveFragment && remainingKeys.length === 0) {
delete fragments[id];
}
} else {
Expand Down
30 changes: 25 additions & 5 deletions test/netjsongraph.dom.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,11 @@ describe("Test GUI loadNodePopup with async and tooltip handling", () => {
testGraph.utils.removeUrlFragment = jest.fn();
const node = {id: "node-1", location: {lat: 10, lng: 20}};
await testGraph.gui.loadNodePopup(node);
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith("id", "nodeId");
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith(
"id",
"nodeId",
false,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(testGraph.echarts.setOption).not.toHaveBeenCalled();
expect(testGraph.utils.updateLabelVisibility).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -778,7 +782,11 @@ describe("Test GUI loadNodePopup with async and tooltip handling", () => {
await testGraph.gui.loadNodePopup(node);
expect(testGraph.utils.setTooltipVisibility).toHaveBeenCalledWith(testGraph, true);
expect(testGraph.utils.updateLabelVisibility).toHaveBeenCalledWith(testGraph, true);
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith("id", "nodeId");
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith(
"id",
"nodeId",
false,
);
});

test("loadNodePopup catches synchronous custom content errors", async () => {
Expand Down Expand Up @@ -807,7 +815,11 @@ describe("Test GUI loadNodePopup with async and tooltip handling", () => {
testGraph.utils.removeUrlFragment = jest.fn();
const node = {id: "node-1", location: {lat: 10, lng: 20}};
await testGraph.gui.loadNodePopup(node);
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith("id", "nodeId");
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith(
"id",
"nodeId",
false,
);
expect(testGraph.echarts.setOption).not.toHaveBeenCalled();
});

Expand All @@ -832,7 +844,11 @@ describe("Test GUI loadNodePopup with async and tooltip handling", () => {
mockPopup.handlers.remove();
expect(testGraph.utils.setTooltipVisibility).toHaveBeenCalledWith(testGraph, true);
expect(testGraph.utils.updateLabelVisibility).toHaveBeenCalledWith(testGraph, true);
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith("id", "nodeId");
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith(
"id",
"nodeId",
false,
);
});

test("loadNodePopup ignores stale async content without clearing newer URL fragment", async () => {
Expand Down Expand Up @@ -1032,7 +1048,11 @@ describe("Test GUI loadNodePopup with async and tooltip handling", () => {
const node = {id: "node-1", location: {lat: 10, lng: 20}};
await testGraph.gui.loadNodePopup(node);
expect(mockPopup.remove).toHaveBeenCalled();
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith("id", "nodeId");
expect(testGraph.utils.removeUrlFragment).toHaveBeenCalledWith(
"id",
"nodeId",
false,
);
expect(consoleErrorSpy).toHaveBeenCalledWith(
"Failed to run popup onOpen callback:",
onOpenError,
Expand Down
Loading