[examples] Added UI guidance to JSONDataUpdate override example by Adityashandilya555 · Pull Request #494 · openwisp/netjsongraph.js

Adityashandilya555 · 2026-01-23T17:31:38Z

Summary

Added an info banner to the override data example explaining its purpose
Renamed netjsonmap-nodeTiles.html to netjsonmap-overrideData.html for clarity (file name now matches its purpose)
Updated index.html link to point to the renamed file
Added dedicated map container to prevent CSS conflicts with banner

Related Issue

#492

Test plan

Open the JSONDataUpdate override example
Verify the info banner is displayed at the top of the page
Verify the banner text clearly explains the example's purpose
Verify the map loads correctly
Zoom in to levels 4 and 6 to confirm the override behavior works

coderabbitai · 2026-01-23T17:32:06Z

📝 Walkthrough

Walkthrough

The changes include formatting refinements to index.html such as quote style normalization, CSS transition multiline reformatting, and localStorage/theme toggle string literal updates. A link href is modified from netjsonmap-nodeTiles.html to netjsonmap-overrideData.html. A new example template file netjsonmap-overrideData.html is introduced with CSS for full-viewport layout, an informational banner, and a map container configured via NetJSONGraph's el option to specify the rendering target.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

javascript

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: adding UI guidance (info banner) to the JSONDataUpdate override example file, which is the primary objective of this PR.
Description check	✅ Passed	The description covers key changes, references an existing issue (`#492`), and includes a test plan, but is missing the contributor checklist and doesn't explicitly state testing completion.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

🧹 Recent nitpick comments

public/example_templates/netjsonmap-overrideData.html (1)
86-93: Use .forEach() instead of .map() for side effects.

Array.prototype.map() is semantically for transformations that return a new array. Here, you're mutating nodes in place without using the return value—.forEach() better expresses this intent.
♻️ Suggested fix
          prepareData: (data) => {
-            data.nodes.map((node) => {
+            data.nodes.forEach((node) => {
              node.label = node.name;
              node.properties = Object.assign(node.properties || {}, {
                location: node.location,
              });
            });
          },

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b09363c and f502b36.

📒 Files selected for processing (2)

index.html
public/example_templates/netjsonmap-overrideData.html

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2026-01-04T08:00:40.936Z

Learnt from: dee077
Repo: openwisp/netjsongraph.js PR: 417
File: src/js/netjsongraph.util.js:1302-1341
Timestamp: 2026-01-04T08:00:40.936Z
Learning: In the netjsongraph.js codebase, the `== null` pattern is intentionally used to check for both null and undefined values in a single comparison, which is a well-established JavaScript idiom and should not be flagged for replacement with strict equality checks.

Applied to files:

index.html

📚 Learning: 2026-01-22T22:37:11.271Z

Learnt from: CR
Repo: openwisp/netjsongraph.js PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-22T22:37:11.271Z
Learning: Applies to test/netjsongraph.browser.test.js : Write browser tests in test/netjsongraph.browser.test.js using Chrome/ChromeDriver and run with `yarn test test/netjsongraph.browser.test.js`

Applied to files:

index.html

🔇 Additional comments (5)

public/example_templates/netjsonmap-overrideData.html (3)

1-15: LGTM!

The HTML structure, charset, viewport meta, and external CSS references are correctly set up for the example template.

16-31: LGTM!

The CSS styles correctly set up a full-viewport layout for the map container.

71-72: LGTM!

The el: "#map-container" option correctly targets the new container, and the zoom-based data update logic with caching is well-implemented.

Also applies to: 95-137

index.html (2)

36-38: LGTM!

The multiline formatting for the transition property improves readability.

254-276: LGTM!

The JavaScript formatting changes maintain the same functionality while improving code style consistency.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…isp#492 Added an info banner to the netjsonmap-nodeTiles.html example explaining that it demonstrates replacing existing map data with new data based on zoom level, and instructing users to zoom in to levels 4 and 6 to see the behavior. Related to openwisp#492

nemesifier

Thanks for the prompt response on this issue, I have one question below:

nemesifier · 2026-01-23T17:49:19Z

public/example_templates/netjsonmap-nodeTiles.html

+      color: white;
+      padding: 12px 20px;
+      border-radius: 8px;
+      font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif;


Why are you changing the font?

oh that might be the issue why qa checks are filing sorry for the oversight . thanks @nemesifier .i will configure it properly

nemesifier

I am not seeing any change, weird:

Screencast.from.2026-01-23.15-25-00.mp4

What about the name of the example file? I don't understand why it contains "nodeTiles", it sounds misleading to me.

Are you moving forward also with the test for this?

nemesifier · 2026-01-23T18:22:30Z

public/example_templates/netjsonmap-overrideData.html

+        z-index: 1000;
+        max-width: 90%;
+        text-align: center;
+        box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2);


I think it would be good to move all the CSS logic to one file so it can be shared between different examples which need further explanation.

Adityashandilya555 · 2026-01-24T03:15:43Z

Netjsongraph.js_.Examples.mp4

nemesifier · 2026-01-24T13:18:52Z

public/example_templates/netjsonmap-overrideData.html

+        z-index: 1000;
+        max-width: 90%;
+        text-align: center;
+        box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2);


Can we extract this to a dedicated CSS so we can reuse it across different examples.

Can you also add a close button?

nemesifier · 2026-01-24T13:31:47Z

index.html


-      themeToggle.addEventListener('click', () => {
-        htmlElement.classList.toggle('dark-mode');
+      themeToggle.addEventListener("click", () => {


These changes are being handled in #497.

nemesifier · 2026-01-24T13:32:37Z

@Adityashandilya555

Test plan

Open the JSONDataUpdate override example

Verify the info banner is displayed at the top of the page

Verify the banner text clearly explains the example's purpose

Verify the map loads correctly

Zoom in to levels 4 and 6 to confirm the override behavior works

Are you moving forward with this?

Adityashandilya555 · 2026-01-26T07:04:52Z

closed this pr in support of this ->#498

coderabbitai bot previously approved these changes Jan 23, 2026

View reviewed changes

Adityashandilya555 force-pushed the add-ui-guidance-override-example branch from b09363c to d09f7ee Compare January 23, 2026 17:42

Adityashandilya555 force-pushed the add-ui-guidance-override-example branch from d09f7ee to df25ab3 Compare January 23, 2026 17:46

nemesifier reviewed Jan 23, 2026

View reviewed changes

Merge branch 'master' into add-ui-guidance-override-example

9319f23

Adityashandilya555 dismissed coderabbitai[bot]’s stale review via 9319f23 January 23, 2026 17:59

Adityashandilya555 force-pushed the add-ui-guidance-override-example branch from 7ba3571 to 9319f23 Compare January 23, 2026 17:59

nemesifier requested changes Jan 23, 2026

View reviewed changes

Merge branch 'master' into add-ui-guidance-override-example

f502b36

Adityashandilya555 force-pushed the add-ui-guidance-override-example branch from e000813 to f502b36 Compare January 24, 2026 02:29

coderabbitai bot added the javascript Pull requests that update Javascript code label Jan 24, 2026

coderabbitai bot approved these changes Jan 24, 2026

View reviewed changes

nemesifier requested changes Jan 24, 2026

View reviewed changes

Adityashandilya555 closed this Jan 26, 2026

Uh oh!

Conversation

Adityashandilya555 commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Test plan

Uh oh!

coderabbitai bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Suggested labels

Uh oh!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

nemesifier Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Adityashandilya555 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

nemesifier Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Adityashandilya555 commented Jan 24, 2026

Uh oh!

nemesifier Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

nemesifier Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

nemesifier commented Jan 24, 2026

Test plan

Uh oh!

Adityashandilya555 commented Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Adityashandilya555 commented Jan 23, 2026 •

edited

Loading

coderabbitai bot commented Jan 23, 2026 •

edited

Loading

nemesifier Jan 23, 2026 •

edited

Loading