Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Fixed duplication of points when map bounds are crossed #575 #581

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

pandafy
Copy link
Member

@pandafy pandafy commented May 23, 2024

Comment on lines 239 to 242
// Exclude the features that may be added for the East world map
westWorldFeatures.features = westWorldFeatures.features.filter(
element => element.geometry.coordinates[0] <= 180
)
Copy link
Member Author

Choose a reason for hiding this comment

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

The issue occurs in the following scenario:

  1. The user pans the map to the East, resulting in the map data containing points for the East World map
  2. Then, the user pans the map to the West. When replicating points for the West World map, the process should not include the points for the East World map. Subtracting 360 from the East World map points will incorrectly map them to the central region. Thus, duplicating the points on the center map.

westWorldFeatures.features.forEach(element => {
if (element.geometry) {
element.geometry.coordinates[0] -= 360;
}
});
// netjsonGraph.utils.appendData call the render method which
// super imposes the data on the existing points on the map.
netjsonGraph.leaflet.geoJSON.removeFrom(netjsonGraph.leaflet);
Copy link
Member Author

Choose a reason for hiding this comment

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

The points were getting duplicated by just using appendData. I reviewed the code of netjsongraph.js and appendData do not call netjsonGraph.leaflet.geoJSON.removeFrom(netjsonGraph.leaflet); like overrideData does.

https://github.com/openwisp/netjsongraph.js/blob/90799710ba2eec7d440a67e58d98452ce3f28aec/src/js/netjsongraph.update.js#L176-L184

I may be wrong, but we may need to replicate this in appendData as well.

Copy link
Member

Choose a reason for hiding this comment

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

I did not understand what you mean exactly with:

I may be wrong, but we may need to replicate this in appendData as well.

You mean that we should patch that method in netjsongraph.js, or what?

@pandafy pandafy force-pushed the issues/575-map-duplicate branch from 42698dc to 9b95704 Compare May 23, 2024 10:25
westWorldFeatures.features.forEach(element => {
if (element.geometry) {
element.geometry.coordinates[0] -= 360;
}
});
// netjsonGraph.utils.appendData call the render method which
// super imposes the data on the existing points on the map.
netjsonGraph.leaflet.geoJSON.removeFrom(netjsonGraph.leaflet);
Copy link
Member

Choose a reason for hiding this comment

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

I did not understand what you mean exactly with:

I may be wrong, but we may need to replicate this in appendData as well.

You mean that we should patch that method in netjsongraph.js, or what?

eastWorldFeatures.features = eastWorldFeatures.features.filter(
element => element.geometry.coordinates[0] >= -180
);
window.console.log(eastWorldFeatures.features);
Copy link
Member

Choose a reason for hiding this comment

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

👀?

@nemesifier nemesifier merged commit 3572e5f into master Jul 5, 2024
21 checks passed
@nemesifier nemesifier deleted the issues/575-map-duplicate branch July 5, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[bug] General map: points in the current view get duplicated when map bounds are crossed
2 participants