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

Linking lines are broken after page resize #4

Closed
bogomya opened this issue Jun 15, 2020 · 9 comments
Closed

Linking lines are broken after page resize #4

bogomya opened this issue Jun 15, 2020 · 9 comments
Labels
bug Something isn't working

Comments

@bogomya
Copy link

bogomya commented Jun 15, 2020

image

@antonioru
Copy link
Owner

Thank you for pointing this out @bogomya
I'll have a look at it as soon as possible

@alexratmanpl
Copy link

Hi @antonioru
That's very annoying bug indeed - when working with more nodes, especially when you have to place them inside a scrollable 'div', there's no way to link and manipulate them correctly - I would be very happy to use your tool in my project but because of that bug it's not possible.
You probably could fix that very easily by modifying useEffect function in DiagramCanvas.js and let it refresh the current position of canvas on each update, something like:
useEffect(() => calculateBBox(canvasRef.current));
In that case I suppose you may consider removing useWindowScroll hook as well.
Btw. great job, simple tool, clean code - it was kind of pleasure to search for a possible solution of fixing that bug. Congrats!

@antonioru
Copy link
Owner

@alexratmanpl @bogomya

Thank you for pointing this out and supporting the project, I can't wait to work again on it as I really love this library.
I hope to do it as soon as possible so I can fix this precise bug and bring new improvements.
Unfortunately at the moment I am quite busy with work and personal life, but I would love to have some contributions.
Please feel free to open a PR

thank you

@volkan-a
Copy link

Hi @antonioru, is there any progress? I really would like to use on my project.

@antonioru
Copy link
Owner

Hi @antonioru, is there any progress? I really would like to use on my project.

Hi @volkan-a
thank you for your concern, as mentioned in the previous comment: at the moment I am quite busy with work and personal life, but I would love to have some contributions.
If you feel like you need some improvements please feel free to open a PR :)

@volkan-a
Copy link

I think @alexratmanpl's suggestion works.

Just change line 31 in DiagramCanvas.js as:

useEffect(() => calculateBBox(canvasRef.current), [canvasRef.current, props]);

then it works. But I have no idea if that affects performance.

@antonioru
Copy link
Owner

antonioru commented Sep 19, 2020

@volkan-a
This is an open source software, meaning everyone is titled to contribute.
If you want, make the change by yourself, test it, make sure it does not break anything and does not affect performances then open a PR.
I will be very happy to look at it.

@antonioru antonioru added the bug Something isn't working label Sep 19, 2020
@antonioru
Copy link
Owner

@volkan-a @bogomya
Apparently this bugs appears on Firefox only, can you confirm it?

@antonioru
Copy link
Owner

antonioru commented Sep 25, 2020

@volkan-a @bogomya

Ok I think I solved the bug in the last published version: 0.2.0

Please let me know if it works for you as well... I'm going to close the issue.

In case the bug shows up again, please just open a new bug report.

Thank you all for the patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants