Skip to content

[fix] Remove custom world-wrap code in favour of worldCopyJump: true#739

Open
pushpitkamboj wants to merge 2 commits intoopenwisp:masterfrom
pushpitkamboj:fix/remove_custom_leaflet
Open

[fix] Remove custom world-wrap code in favour of worldCopyJump: true#739
pushpitkamboj wants to merge 2 commits intoopenwisp:masterfrom
pushpitkamboj:fix/remove_custom_leaflet

Conversation

@pushpitkamboj
Copy link
Copy Markdown

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #719

Description of Changes

Removed the custom moveend listener and setMaxBounds call from
device-map.js that were manually cloning and shifting map features
by ±360° when panning past the international date line.

This is no longer needed since openwisp/netjsongraph.js#466 has been
merged, which sets worldCopyJump: true by default in Leaflet,
handling world-wrap behaviour natively.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 17, 2026

No actionable comments were generated in the recent review. 🎉

📜 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 331fca2 and c4a8b5a.

📒 Files selected for processing (1)
  • openwisp_monitoring/device/static/monitoring/js/device-map.js
💤 Files with no reviewable changes (1)
  • openwisp_monitoring/device/static/monitoring/js/device-map.js
⏰ 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). (9)
  • GitHub Check: Python==3.12 | django~=5.1.0
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.10 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=5.2.0
  • GitHub Check: Python==3.13 | django~=5.2.0
  • GitHub Check: Python==3.13 | django~=5.1.0
  • GitHub Check: Python==3.11 | django~=5.1.0
  • GitHub Check: Python==3.10 | django~=4.2.0

Walkthrough

The change removes 45 lines of custom horizontal wrap/dateline wrapping logic from the device map's JavaScript file. This includes the deletion of the setMaxBounds call that restricted map panning to three wrapped worlds, the moveend event handler that cloned and appended map features at world boundaries, and related state tracking flags (westWorldFeaturesAppended, eastWorldFeaturesAppended). The removal delegates this functionality to Leaflet's built-in worldCopyJump option.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main change: removing custom world-wrap code in favor of Leaflet's native worldCopyJump option.
Description check ✅ Passed The description addresses most template sections but is missing manual testing and test case updates despite being required checklist items.
Linked Issues check ✅ Passed The PR successfully removes the custom world-wrap code (setMaxBounds and moveend listener) from device-map.js as required by issue #719, relying on worldCopyJump: true from netjsongraph.js.
Out of Scope Changes check ✅ Passed All changes are in-scope: the PR removes only the custom world-wrap logic from device-map.js as specified in issue #719, with no additional modifications.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Copy link
Copy Markdown
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

How can this possibly work? Is the new version of netjsongraph.js already included?

@pushpitkamboj
Copy link
Copy Markdown
Author

pushpitkamboj commented Feb 17, 2026

No, it dosent. You are right
the vendored netjsongraph.min.js was last updated on Aug 14, 2025, which predates the merge of openwisp/netjsongraph.js#466 (Dec 19, 2025). So the current build does not include worldCopyJump: true.

can I update it by cloning the netjsongraph.js repo, running npm install && npm run build, and replacing the existing file with the output from dist/netjsongraph.min.js. Does that approach work? Please validate.
@nemesifier

@nemesifier
Copy link
Copy Markdown
Member

@pushpitkamboj let's wait for #738, but please STOP sending PRs with code you haven't tested manually, this is really unprofessional and generates overhead.

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.

[change] Remove custom leaflet code in favour of worldCopyJump: true

2 participants