Skip to content

Conversation

@siuyin
Copy link

@siuyin siuyin commented Dec 25, 2016

Hi, This is just a documentation update which I think Springy users may find helpful.

This just saves them a bit of time in reading the source.

@dhotson
Copy link
Owner

dhotson commented Dec 28, 2016

Hey thanks! This is a feature I've been meaning to add for a while now. :)

I reckon this would be better as something built-in rather than asking users to patch a function in the readme. It looks like you're pretty close to getting it working already.

@siuyin
Copy link
Author

siuyin commented Dec 29, 2016

Yes, you are right. I thought of preserving the API. With the capability built-in I would need to extend the API -- say with a loadJSONRich command.

A new name is needed so that it does not disrupt the existing users of loadJSON.

@dhotson
Copy link
Owner

dhotson commented Dec 29, 2016

Hmm, do you think it'd be possible to extend the existing loadJSON and still retain compatibility?

Currently nodes are expected to be passed as simple string ids, which makes passing extra metadata difficult. Perhaps nodes could be in an extended format?

e.g.

graph.loadJSON({
  nodes: [
    "a",
    ["b", { label: "B" }] // Maybe like this?
  ],
  edges: [
    ["a", "b", { label: "ab" }], // I think this already works
  ]
});

@siuyin
Copy link
Author

siuyin commented Dec 29, 2016

Thanks for the insight -- yes, I think that could work...
I'll do some experiments.

@siuyin
Copy link
Author

siuyin commented Dec 29, 2016

Added capability into springy.js and also updated README to reflect changes.
Should be ready for merge -- but do remember to update the version number in the README.

Springy 1.2+ also accepts JSON with node attributes see
[demo-json.html](http://dhotson.github.com/springy/demo-json.htm

@techtonik
Copy link
Contributor

I would rebase/squash commits and probably split JSON instructions into two parts.

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.

3 participants