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

[Bug] Geojson layer is not updated when dataset updated #1533

Merged
merged 1 commit into from Jul 18, 2021

Conversation

delfrrr
Copy link
Contributor

@delfrrr delfrrr commented Jul 6, 2021

In current implementation when addDataToMap is called second time for the same dataset.info.id, geojson-layer will not update correctly. Because calculateDataAttribute in GeoJSON layer require precalculated this.dataToFeature property which calculated in updateLayerMeta. So If metadata did not change (same columns but different rows), updateLayerMeta will not be called and layer will not update correctly.

Given that updateLayerMeta takes allData as param, it will be logical to call it when data updates.

@@ -873,7 +873,7 @@ class Layer {
const dataUpdateTriggers = this.getDataUpdateTriggers(layerDataset);
const triggerChanged = this.getChangedTriggers(dataUpdateTriggers);

if (triggerChanged.getMeta) {
if (triggerChanged.getMeta || triggerChanged.getData) {
Copy link
Contributor

Choose a reason for hiding this comment

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

for this case, you should change getDataUpdateTriggers in base-layer, and add allData to the return,

 return {
    getData: {datasetId: id, allData, columns, filteredIndex},
    getMeta: {datasetId: id, allData, columns},
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@heshan0131 updated as requested

Signed-off-by: Vladi Bilonenko <bilonenko.v@gmail.com>
@delfrrr
Copy link
Contributor Author

delfrrr commented Jul 12, 2021

@heshan0131 so what are the next steps? is there anything I can do t get this merged? I guess you need to approve some workflows to run.

@heshan0131 heshan0131 merged commit 16fab11 into keplergl:master Jul 18, 2021
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.

None yet

2 participants