Add data-source-id, data-target-id on Link

[lazToum]

It helps finding links between nodes (i.e. on node mouseOver, find the links related to this node.)

[coveralls]

Coverage remained the same at 99.723% when pulling 869bd3a on lazToum:patch-1 into ef826e6 on bkrem:master.

[bkrem]

Hi @lazToum,

Great call on adding this, thank you! Could you please briefly add a test for this here https://github.com/bkrem/react-d3-tree/blob/master/src/Link/tests/index.test.js#L58 ?

A single test along the lines of "binds IDs of source & target nodes to data-source-id/data-target-id" prop should do the trick 👍 I'll merge, update docs and release once that's done.

Let me know if there are any issues!

[lazToum]

Hi @lazToum,

Great call on adding this, thank you! Could you please briefly add a test for this here https://github.com/bkrem/react-d3-tree/blob/master/src/Link/tests/index.test.js#L58 ?

A single test along the lines of "binds IDs of source & target nodes to data-source-id/data-target-id" prop should do the trick +1 I'll merge, update docs and release once that's done.

Let me know if there are any issues!

Something like this?

react-d3-tree/src/Link/tests/index.test.js

Line 59 in e4286da

it('binds IDs of source & target nodes to data-source-id/data-target-id', () => {

I also added one more in case a source/target does not have an "id" key.

[bkrem]

Yup this is looking good thank you! 👍

I'm fine with merging like this but I'm wondering what the fallback scenario (no id field) is needed if all nodes automatically generate UUIDs?

[lazToum]

Yup this is looking good thank you! +1

I'm fine with merging like this but I'm wondering what the fallback scenario (no id field) is needed if all nodes automatically generate UUIDs?

Well it is only needed if a Link is used as a standalone element like in the test file where one can supply only the x,y props of the source and target. But you are correct, inside a Tree instance this check is not necessary. Should I remove it?

[bkrem]

Yeah I think we can safely disregard this scenario. A node should always have an ID, and if it doesn't returning undefined for data-source-id/data-target-id would be the expected result.

Thank you for taking this into consideration though 💯

Will merge, document & publish once that's done :)

[lazToum]

Yeah I think we can safely disregard this scenario. A node should always have an ID, and if it doesn't returning undefined for data-source-id/data-target-id would be the expected result.

Thank you for taking this into consideration though 100

Will merge, document & publish once that's done :)

Great, thanks!

[bkrem]

Shipped as v1.15.0

Thank you again 👏