Skip to content

Conversation

adamnsch
Copy link
Collaborator

@adamnsch adamnsch commented Apr 11, 2025

Thank you for your contribution to the Graph Data Science Client project.

Before submitting this PR, please read Contributing to the Neo4j Ecosystem.

Make sure:

  • You signed the Neo4j CLA (Contributor License Agreement) so that we are allowed to ship your code in our library
  • Your contribution is covered by tests

@adamnsch adamnsch changed the title Start adding hover effects Add hover effects for node IDs Apr 11, 2025
@adamnsch adamnsch marked this pull request as ready for review April 11, 2025 14:15
@adamnsch adamnsch requested a review from ckanz April 11, 2025 14:15
@FlorentinD FlorentinD requested a review from Copilot April 11, 2025 14:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@adamnsch
Copy link
Collaborator Author

adamnsch commented Apr 11, 2025

Some things we should consider doing:

  • Add option to VG.render for disabling hover effect
  • Add min-width and max-width to the tooltip div

Copy link
Member

@ckanz ckanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adamnsch, the tooltip looks great! I've got a few comments, mostly just optional code style improvements, except for the issue with the tooltip showing in the wrong instance when having multiple NVL frames, which we should address before merging.

Let me know what you think!

@adamnsch
Copy link
Collaborator Author

Thanks @adamnsch, the tooltip looks great! I've got a few comments, mostly just optional code style improvements, except for the issue with the tooltip showing in the wrong instance when having multiple NVL frames, which we should address before merging.

Let me know what you think!

Thanks for the comments! We'll look into it :)

adamnsch and others added 3 commits April 22, 2025 10:38
Co-Authored-By: Florentin Dörre <florentin.dorre@neotechnology.com>
Co-Authored-By: Clemens Anzmann <clemens.anzmann@gmail.com>
@adamnsch adamnsch merged commit 77e0704 into main Apr 22, 2025
11 checks passed
@adamnsch adamnsch deleted the hover-nodes branch April 22, 2025 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants