Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arrows don't point to nodes as expected #265

Closed
antoninklopp opened this issue Nov 15, 2019 · 6 comments
Closed

Arrows don't point to nodes as expected #265

antoninklopp opened this issue Nov 15, 2019 · 6 comments
Labels
enhancement improving existent functionality or performance related feature request priority high

Comments

@antoninklopp
Copy link
Collaborator

Describe the bug
The arrows when directed is checked and they are either before the element or after.

To Reproduce
Steps to reproduce the behavior:
Check the sandbox example with this directed checked. The arrows are behind the images.

Expected behavior
The arrows should point onto the element

Environment:

  • OS: Windows
  • Browser: Chrome

I don't see if this customizable, even after I've read the doc carefully. If this is not a bug and I can change the arrow by passing props that I want, please tell me !

Thanks

@danielcaldas
Copy link
Owner

Hey there @antoninklopp thanks for reporting. Although in fact you see the arrows underneath the image in that example, that's not the usual behavior. In the example it happens because the node is manipulated by the client (im this case the example Sandbox integration) to shape the node with some custom elements and there's probably some resizing involved as well. The library core is not aware of this changes thus the arrows being rendered under the node... Not really sure if there's an workaround... Maybe manipulating the arrow with a css class that would override the default spacing, but that's a huge hack.

What do you think of an approach where we expose a property that tells react-d3-graph how far the arrow should be placed from the node @LonelyPrincess?

@antoninklopp
Copy link
Collaborator Author

What I don't really get is that, in my example, I'm drawing a simple circle with a div with 50% border and 100% width and height.
I choose circle in the config.

With these information, the library should be aware of the form and size of my object, but the arrow still lies under my object.
I, of course, understand the difficulty to make the arrow point towards any object, customized very complicated shapes.

I think that this would be really cool to be able to customize all this personally.

@danielcaldas danielcaldas added enhancement improving existent functionality or performance related feature request labels Nov 16, 2019
@antoninklopp
Copy link
Collaborator Author

Hi @danielcaldas ,

After investing a bit where we could add this in the code, I think that the right place is here

With adding and subtracting the size * unit_vector of linked nodes we could have better results for arrows.
There would be the simple code change for circle like nodes (excuse for poor code quality !) :

    var x1 = nodes?.[source]?.x || 0;
    var y1 = nodes?.[source]?.y || 0;
    var x2 = nodes?.[target]?.x || 0; 
    var y2 = nodes?.[target]?.y || 0; 

    let dirx1x2 = x2 - x1; 
    let diry1y2 = y2 - y1;  
    let norm = dirx1x2 * dirx1x2 + diry1y2 * diry1y2; 
    dirx1x2 /= norm; 
    diry1y2 /= norm; 

    x1 += config.node.size * dirx1x2; 
    x2 -= config.node.size * dirx1x2;
    y1 += config.node.size * diry1y2; 
    y2 -= config.node.size * diry1y2; 

Before :
before_code_change

After :
after_code_change

The less complicated maths are for the circle of course, but I think it could be great to adapt it for every supported form.

@danielcaldas
Copy link
Owner

That could be an approach and it looks very promising, we could potentially set it up only from circles and slowly increment from there 👍

@emersxw
Copy link

emersxw commented May 20, 2020

was this launched already?

@danielcaldas
Copy link
Owner

danielcaldas commented May 21, 2020

Hi @emersondflopes, I'll close the issue while this is not released. The PR was merged, we just need to adjust a few things before releasing this together with a bunch of other features & bug fixes. Stay stunned.

edit cc @emersondflopes this feature is available with the latest version react-d3-graph@2.5.0

Repository owner locked as resolved and limited conversation to collaborators May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement improving existent functionality or performance related feature request priority high
Projects
None yet
Development

No branches or pull requests

3 participants