Updates DataSet and DataView remove event payload #2189

Merged
merged 3 commits into from Oct 20, 2016

Projects

None yet

3 participants

@ZacBrownBand
Contributor
ZacBrownBand commented Oct 19, 2016 edited

The remove event that is triggered in DataView and DataSet only includes the ids of the items that were removed, and since the items are no longer available, it is hard to tell what items were removed.

This pr would add an oldItems property to the object that is sent with the remove event. This is supplementary to the items containing the ids to maintain reverse compatibility.

This effects

  • Items getting removed from DataSets
  • Items getting removed from DataViews
  • Clearing a DataSet using ds.clear()
  • Setting the data for a DataView using dv.setData() or when the DataView is created

Also updates the documentation for dataset Callback section.

@ZacBrownBand ZacBrownBand Updates all 'remove' events in DataSet and DataView to include the ob…
…jects that were removed in addition to the the ids.
8aecd34
@mojoaxel
Member

@ZacBrownBand Thanks! Looks complete. Could you maybe provide a simple example that demonstrates how to use this!? That would be great.

@ZacBrownBand
Contributor

A simple example would be if you wanted to know how long something was in a dataset for. The item in the dataset could have a created property marking the time is was added to the dataset. When handling the remove event you would have access to that data, not just the id. While handling the remove, you could just do a simple time subtraction,

To accomplish this now, you would have to maintain a hashmap of the ids and times items were added, so when you get the remove event with the id you can look up the time.

This was inspired by an application I am working that uses vis for data management. We have an issue were we have internal and external ids since we manage data from multiple sources, this leads to use needing more than just the id in special cases. By having the full object that was removed it is much easier to do entity resolution and ensure other things are updated accordingly.

@mojoaxel
Member

@ZacBrownBand I see. I guess you need a kind of special case to use this. But why not! Thanks!
Did you test your code? I don't want to write a test for that...

@ZacBrownBand
Contributor

I wrote unit tests in my application, I'm happy to add one in here!

@mojoaxel
Member

I wrote unit tests in my application, I'm happy to add one in here!

We don't have any unit tests. But if you want to start feel free to provide a pull request ;-)

@mojoaxel

Thanks!

@mojoaxel mojoaxel merged commit 776f4cc into almende:develop Oct 20, 2016
@Bradmoon Bradmoon added a commit to StateFarmIns/vis that referenced this pull request Oct 20, 2016
@Bradmoon Bradmoon Develop - update with master project (#2)
* Added handling of settings backgroundColor and dataColor

* Managing help offers (#2178)

* removed reference to the style-guide
* added a how-to-help; added a document describing the current maintainer status; fixes #2170; closes #1781
* listing the members of the support team
* use search before creating a new issue
* fixes typos

* Added handling of settings for camera

* linked users to there profile

* Add Italian support for timeline

* Added step to defaults; fixes to detect and survive inconstent data

* Adjusted STYLE enum to previous

* Add Italian locale support for network (#2184)

Thanks @federicoco for providing the translation!

* cleaned up locale; added all locals to example; set users local as default

* Added fields tooltip and showLegend to defaults

* Fixed oversight with showTooltip

* Added commond handling for settings with internal prefix 'default'

* Final fixes

* Line graph draw segments by depth order (#2200)

* Updates DataSet and DataView remove event payload (#2189)

* Updates all 'remove' events in DataSet and DataView to include the objects that were removed in addition to the the ids.
* Updated the documentation to reflect changes.
* Fixed possible object mutation when typeof id is object in DataSet.remove.
ea1251e
@mojoaxel
Member
mojoaxel commented Nov 5, 2016

Sadly this introduced an error in DataView.refresh in line 120. I'll fix it.
@ZacBrownBand That happens! Next time please run npm test before committing.

@ZacBrownBand
Contributor

@mojoaxel Sorry about that, thanks for catching it. I didn't realize there was npm test, I know for next time.

+ item = this._remove(ids[i]);
+ if (item) {
+ itemId = item[this._fieldId];
+ if (itemId) {
@p1erce
p1erce Dec 13, 2016

This brings in some old bugs. Like I want to remove {id: 0, from: 4, to: 5} from DataSet edges.

@mojoaxel
mojoaxel Dec 14, 2016 Member

@p1erce Can you please create an issue so we don't forget this! If you want you also could provide a simple fix :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment