search

grtjn / ml-visjs-graph.js

Graph visualization for triples stored in MarkLogic, based on the VisJS Network library
https://grtjn.github.io/ml-visjs-graph.js/
Apache License 2.0
3 stars 3 forks source link

Why nodeOptions and edgeOptions in setData #25

Open patrickmcelwee opened 6 years ago

patrickmcelwee commented 6 years ago

I was wondering why you mixed these particular options with data. Wouldn't it make more sense just to update those with setOptions? https://github.com/grtjn/ml-visjs-graph.js/blob/master/src/mlvisjs.global.js#L206

grtjn commented 6 years ago

Yeah, not sure why I came up with that. Maybe in case you want to set different styling while updating nodes, but a setOptions directly works equally well..

Not quite sure why vis.js has these options as part of datasets to be honest..

grtjn commented 6 years ago

I was looking over the docs just the other week, and I think the setData can take nodes/edges specific options, looks different from the network options. I might be handling them wrong to be honest, never tested that code..

patrickmcelwee commented 6 years ago

The visjs setData() function doesn't accept options ... just arrays of nodes and edges. http://visjs.org/docs/network/#methods

I vote we remove them, since it sounds like you don't have a current use case either.

grtjn commented 6 years ago

Sorry, my bad. I meant to say that DataSet has its own setOptions method, with its own options object (complete different from the network options)..

grtjn commented 6 years ago

Let's expose another setData that just takes nodes and edges, and no options..

grtjn commented 6 years ago

or do some smart handling of arguments to handle both if options are passed in or not.. I'd be reluctant to make a breaking change if not necessary..

patrickmcelwee commented 6 years ago

Ah, I hadn't seen those options before ... http://visjs.org/docs/data/dataset.html#Construction