Skip to content

[examples] Added UI guidance to append data arrays example#496

Closed
Adityashandilya555 wants to merge 3 commits intoopenwisp:masterfrom
Adityashandilya555:add-ui-guidance-arrays-example
Closed

[examples] Added UI guidance to append data arrays example#496
Adityashandilya555 wants to merge 3 commits intoopenwisp:masterfrom
Adityashandilya555:add-ui-guidance-arrays-example

Conversation

@Adityashandilya555
Copy link
Contributor

Summary

  • Added an info banner to the netjsonmap-appendData2.html example explaining its purpose
  • The banner describes that this example demonstrates loading multiple JSON data files at once using an array
  • Users are informed that all three data files are loaded and combined automatically when the map initializes

Related Issue

#492

Test plan

  • Open the append data arrays 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 with data from all three JSON files combined

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

Added a full-page map layout and a dedicated map container element (<div id="map-container"></div>), plus a fixed information banner (#info-banner) with instructions about appending data using arrays. The NetJSONGraph initialization now accepts an el: string option and the example passes el: "#map-container" so the map renders into the new container. The example continues to load and combine multiple JSON data files at initialization; script control flow for data loading remains unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes a summary of changes and a test plan, but is missing key sections from the template including the checklist, reference to the existing issue in the proper format, and screenshot section. Add the required checklist items, format the issue reference as 'Closes #492', and include a screenshot section or note if no screenshot is applicable.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding UI guidance (an info banner) to the append data arrays example file.

✏️ Tip: You can configure your own custom pre-merge checks in the 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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 23, 2026
@Adityashandilya555 Adityashandilya555 force-pushed the add-ui-guidance-arrays-example branch from daea2e0 to f389e15 Compare January 23, 2026 17:42
Added an info banner to the netjsonmap-appendData2.html example
explaining that it demonstrates loading multiple JSON data files
at once using an array, and that all three data files are loaded
and combined automatically when the map initializes.

Related to openwisp#492
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 23, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@public/example_templates/netjsonmap-appendData2.html`:
- Around line 61-63: Fix the typo in the block comment that reads "Similiar to
the first method, but easier." — update the word "Similiar" to "Similar" in the
comment near the array-files append-data section in
public/example_templates/netjsonmap-appendData2.html so the sentence reads
"Similar to the first method, but easier."
🧹 Nitpick comments (1)
public/example_templates/netjsonmap-appendData2.html (1)

16-58: Move info-banner styling into the stylesheet for easier theming.

Inline styles make future tweaks and overrides harder. Consider a CSS rule and keep the markup clean (optionally add a semantic role).

♻️ Proposed refactor
@@
-    <div
-      id="info-banner"
-      style="
-        position: fixed;
-        top: 10px;
-        left: 50%;
-        transform: translateX(-50%);
-        background: rgba(52, 73, 94, 0.95);
-        color: white;
-        padding: 12px 20px;
-        border-radius: 8px;
-        font-size: 14px;
-        z-index: 1000;
-        max-width: 90%;
-        text-align: center;
-        box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2);
-      "
-    >
+    <div id="info-banner" role="note">
@@
       `#map-container` {
         position: absolute;
         top: 0;
         left: 0;
         width: 100%;
         height: 100%;
       }
+      `#info-banner` {
+        position: fixed;
+        top: 10px;
+        left: 50%;
+        transform: translateX(-50%);
+        background: rgba(52, 73, 94, 0.95);
+        color: white;
+        padding: 12px 20px;
+        border-radius: 8px;
+        font-size: 14px;
+        z-index: 1000;
+        max-width: 90%;
+        text-align: center;
+        box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2);
+      }
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a23ddb1 and 6b63ef8.

📒 Files selected for processing (1)
  • public/example_templates/netjsonmap-appendData2.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Tests and Coverage
🔇 Additional comments (1)
public/example_templates/netjsonmap-appendData2.html (1)

71-72: The el option is fully supported and will work in any NetJSONGraph bundle.

The el option is a standard configuration parameter handled directly in NetJSONGraphCore.setConfig(), which processes it as either a DOM element or CSS selector string. This is core functionality included in all builds and not subject to tree-shaking. The example will render correctly into #map-container.

Likely an incorrect or invalid review comment.

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

Comment on lines +61 to 63
Using array files to append data step by step at start.
Similiar to the first method, but easier.
*/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo in comment (Line 62).

“Similiar” → “Similar”.

✏️ Suggested fix
-          Similiar to the first method, but easier.
+          Similar to the first method, but easier.
🤖 Prompt for AI Agents
In `@public/example_templates/netjsonmap-appendData2.html` around lines 61 - 63,
Fix the typo in the block comment that reads "Similiar to the first method, but
easier." — update the word "Similiar" to "Similar" in the comment near the
array-files append-data section in
public/example_templates/netjsonmap-appendData2.html so the sentence reads
"Similar to the first method, but easier."

@Adityashandilya555
Copy link
Contributor Author

Netjsongraph.js_.Examples.1.mp4

@Adityashandilya555
Copy link
Contributor Author

closed this pr in support of this ->#498

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant