-
Notifications
You must be signed in to change notification settings - Fork 8
Adding force-simulation layout algorithm to draw nodes #119
Conversation
Pull Request Test Coverage Report for Build 2513019511Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @henrypalacios , great PR to play with!
Some issues and doubts from my side:
-
Can we write layout names using upper and lower-case letters? I mean, as a stupid end-user, it was not clear to me what this or that type of diagram means. And when I googled, I found that fcose is actually fCoSE (easies to google it when know a certain name). And klay is KLayout..
-
fCoSE layout always changes its position when press on 'reset' button. Maybe instead/or besides of the 'Reset button we should show 'Play' button if it works this way. WDYT?
-
KLayout looks too weird with a lot of overlapped elements, especially for graphs with few nodes
Maybe we should hide it for such graphs? -
In this transaction with klay layout 2 arrows are always blinking when press on the Reset button
-
I think it would be nice to 'save' layout selection when switch between Graph/Orders tabs
Thanks
First of all, very nice. It's very useful for some more unusual batches like the ones you shared. Now, regarding Elena's points:
And some points from me:
Screen.Recording.2022-06-21.at.18.01.10.mov
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
Hey @henrypalacios , great changes!
Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback team,
There is still pending the jump that @alfetopito found, with a big tx like 0xf2a00ef766bc5caee96cbee8e1842e8ed29ca5059b1a5daa71258d6232d1d522, I'll see it with @alongoni in another PR.
☝️ regarding this last point, I think it would be great to use a URLQueryParam like |
Summary
Closes #114
Adding layout graph selector
To Test
Open any of the following transactions: