-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
Conversation
// Exclude the features that may be added for the East world map | ||
westWorldFeatures.features = westWorldFeatures.features.filter( | ||
element => element.geometry.coordinates[0] <= 180 | ||
) |
There was a problem hiding this comment.
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:
- The user pans the map to the East, resulting in the map data containing points for the East World map
- 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); |
There was a problem hiding this comment.
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.
I may be wrong, but we may need to replicate this in appendData as well.
There was a problem hiding this comment.
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?
42698dc
to
9b95704
Compare
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); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀?
Closes #575
Blockers