humanitiesplusdesign / palladio

An application that brings humanities research methods to data visualization.
BSD 3-Clause "New" or "Revised" License
170 stars 31 forks source link

Changing stuff in data view after viewing visualizations leads to broken project files #116

Closed jiemakel closed 8 years ago

jiemakel commented 8 years ago

I'm guessing that when Palladio detects a change in the data model, it throws away the old visualizations and whips up new ones. However, the old ones still stick around and are saved when exporting the project file.

This thenleads to the wrong behavior in the importer, because in the default Palladio app configuration, only one view of each type is defined, and that view thus sees only the state of the first visualization in the stack, which is probably badly configured at this point.

esjewett commented 8 years ago

Thanks - have been meaning to rework this whole workflow, but haven't gotten to it, and fixing the problem you describe here should be pretty quick. The old visualizations must not be getting properly torn down and un-registered. Will look into it!

esjewett commented 8 years ago

I didn't get to this last night. I'll shoot for the weekend. My belief is that the issue is in the teardown of the views.

Each view registers save functions with Palladio. For the map view, that happens here: https://github.com/humanitiesplusdesign/palladio/blob/master/src/components/palladio-map-view/palladio-map-view.js#L1762

When the view is destroyed, those save functions are supposed to get deregistered. That happens here: https://github.com/humanitiesplusdesign/palladio/blob/master/src/components/palladio-map-view/palladio-map-view.js#L1363

I think that either the component is never really getting destroyed (just removed from the page), the $destroy event is not getting fired, or there is a bug in the deregistration code that is resulting in the save function not getting deregistered.

esjewett commented 8 years ago

A silly bug in calling function.bind. Should be fixed now. I'll try to pick these 2 fixes and get a new version pushed out as mentioned in #119.

esjewett commented 8 years ago

Sorry for the delay - this should now be fixed on both the master branch and the main site in 1.1.4