Reorganizing the computation of arrows and links for circle nodes

[antoninklopp]

As explained in #265, I think that it would be better to make arrows points to nodes and not to the middle (under) the nodes.

This PR solves this arrow problem when config is circle

Some tests fail, as it changes from snapshots. I will change the snapshots if the PR is accepted

[danielcaldas]

Thanks a lot for looking into this, please check some of the comments 🙂

[antoninklopp]

@danielcaldas , I think that I resolved everything, please tell me if there is more 😄

[danielcaldas]

Just one final comment, thanks for the good work 👍

[danielcaldas]

Looking good now @antoninklopp 👏 You can go ahead and update the snapshot tests that are failing at the moment due to the changes introduced and push the new snapshots.

[danielcaldas]

Looking good, ready to merge as soon snapshots are updated

[antoninklopp]

@danielcaldas I updated the snapshots, the tests are passing but I get this error, that seems related to CI pipeline :

The cypress npm package is installed, but the Cypress binary is missing.

We expected the binary to be installed here: /home/travis/.cache/Cypress/3.4.1/Cypress/Cypress

Could you take a look at the travis build please?

[antoninklopp]

@danielcaldas Any update on this?

[danielcaldas]

Sorry for the delay @antoninklopp, I'm currently with no much time to look into this. I cleaned the cache on travis to see if the issue goes away. Once the build it's green, I'll try to move forward with this as soon as I have some time.

[antoninklopp]

Hi @danielcaldas, after trying to integrate this with my project, I discovered that this works not really great with Custom nodes (I have to make some tests on this).
I also discovered that the custom nodes are not drawn the same way as the normal nodes, and even if I use the circle node type, my changes won't be taken into account on my node.

Would you like me to do the changes on that too?
You can have a look at the changes I made here

[danielcaldas]

Hi @danielcaldas, after trying to integrate this with my project, I discovered that this works not really great with Custom nodes (I have to make some tests on this).

Would it be possible to have this newly introduced logic not run for custom nodes? And for now, we leave it only for the default nodes? Otherwise, feel free to go ahead and try to make the changes for custom nodes, it will tough takes as long because it will be a broader change, we will need more eyes on this and more testing.

[anothrNick]

Any update on this @antoninklopp @danielcaldas ? This would be a very helpful addition.

[antoninklopp]

Hi @danielcaldas,

I just made a change which should remove the new feature for custom nodes.
I will create a new PR for custom nodes.

Does this sound OK?

Thanks

[danielcaldas]

Looks good now @antoninklopp! Thanks a lot for this contribution, and sorry for the long wait.