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

get x, y and other node informations with event onMouseOverNode() #306

Closed
jdelpierre opened this issue Mar 2, 2020 · 8 comments
Closed
Assignees
Labels
enhancement improving existent functionality or performance related feature request good first issue Hacktoberfest Hacktoberfest label

Comments

@jdelpierre
Copy link

jdelpierre commented Mar 2, 2020

Hello everybody,

Is your item request related to a problem? Please describe your request

I would like to be able to at the onMouseOverNode() event of the :
get the x, y positioning information and the current node property object to position a tooltip containing the node information.

Describe the solution you would like

I would like to be able to have the following arguments: onMouseOverNode(x, y, node)

Describe your use case

Write about what you are building and why you need this feature.

Describe the alternatives you have considered

For the moment, I'm going to go through the original d3js library because I managed to create the tooltip, but I have to look for the positions and information myself at each event, so it's expensive.

Thank you

Issue after discuss about this => #304

@danielcaldas danielcaldas added feature request enhancement improving existent functionality or performance related good first issue labels Mar 2, 2020
@danielcaldas
Copy link
Owner

danielcaldas commented Mar 2, 2020

Option 1

onMouseOverNode(id, x, y) (preffered to minimal API surface and compatible with onNodePositionChange

Option2

onMouseOverNode(id, node), pass along all the node information containing values of x and y.

Option3

onMouseOverNode(node), only pass along all the node information containing values of x and y.

I think we could potentially afford Option 3, but that would be a breaking change... So it could only be released in an eventual version 3.x.x which is not planned to happen soon.

I will prefer Option 1, but if you demonstrate the use case and we see it makes sense and it's has a potentially more general use, I can push for Option 2. What do you think @jdelpierre ?

note: Please try and use the feature request/bug report GitHub templates, it makes understand the issue, a way faster a more structured process.

@jdelpierre
Copy link
Author

jdelpierre commented Mar 2, 2020

I think we can add in this case the X and Y in a {'position' : {x: posX, y: posY}} property if we don't want to wait for the breaking change for a version 3.
Because the best solution for me is option 3, we don't change the structure of the node normally.

I prefer option 2 because node information can be important, and x, y it's possible to add un object node.
The id is used to link them together, but if you need to display other information (status for example, a number of objects), I'm thinking mostly of a property other than the id chooses.

I have updated the description of issue with the template report ;)

@danielcaldas
Copy link
Owner

Hey, there @jdelpierre I currently have no time available to look into this issue. Please feel free to send a PR in my way. I would be more than glad to review and provide guidance. Cheers!

@jdelpierre
Copy link
Author

Thanks for the interest but it's hard for me to find time in this period too :(

@mailuesp
Copy link

Why don't just return the whole object instead of its ID or a limited set of properties? I would do this for every event.

@danielcaldas
Copy link
Owner

danielcaldas commented Aug 14, 2020

Why don't just return the whole object instead of its ID or a limited set of properties? I would do this for every event.

The API was designed with minimal overhead, the ID should give enable you to access all the data you need on your end. Passing the full object is just not retro compatible and it would represent a breaking change to the current API (something I really want to avoid, it complicates the maintenance problem).

I'm looking into Option 2 or Option 3 mentioned above as potential candidates. Just need to see if there's more interest from the community in doing so.

@danielcaldas danielcaldas added the breaking change a breaking change in the library label Aug 14, 2020
@danielcaldas
Copy link
Owner

Hi there, if you're looking into tackling this issue, please consider the Option 2 as an approach to implementing this. Below the new API for onMouseOverNode.

function onMouseOverNode(id, node) {
  // ...
}

Cheers!

@danielcaldas danielcaldas added Hacktoberfest Hacktoberfest label and removed breaking change a breaking change in the library labels Sep 30, 2020
@LonelyPrincess LonelyPrincess self-assigned this Oct 17, 2020
@LonelyPrincess
Copy link
Collaborator

I'm closing this issue as it has been resolved with #385. This feature will become available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improving existent functionality or performance related feature request good first issue Hacktoberfest Hacktoberfest label
Projects
None yet
Development

No branches or pull requests

4 participants