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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: expose node information to node related callbacks #385

Merged
merged 4 commits into from
Oct 29, 2020

Conversation

LonelyPrincess
Copy link
Collaborator

This PR exposes the node data to the callback methods for all the node-related events. This will give developers access to all of the node's information (including its coordinates). Therefore, this should solve #306.

The event handlers affected by this change are:

  • onClickNode
  • onDoubleClickNode
  • onRightClickNode
  • onMouseOverNode
  • onMouseOutNode

To obtain the node's information, users can make use of a new parameter on the callback functions, like in the example below:

const onMouseOverNode = function(nodeId, node) {
  window.alert(`Mouse over node ${nodeId} in position (${node.x}, ${node.y})`);
};

The nodeId is still returned first to keep compatibility with previous versions of the library. In the future we may want to delete this property and just return the node itself (as the id can be read from the node object directly).


Task list:

馃捇 Development

  • Send node param from the Graph component's node event handlers to the user defined ones located in the props object.

馃摑 Documentation

  • Update node callback methods usage example in README file.
  • Update node callback methods usage example in DOCUMENTATION file.
  • Update node callback methods usage example in Graph component docs.

馃惓 Project settings

  • Increase version number to 2.6.0.

@LonelyPrincess LonelyPrincess added enhancement improving existent functionality or performance related Hacktoberfest Hacktoberfest label documentation add documentation to the repository or the the code base hacktoberfest-accepted Label for accepted PRs for Hacktoberfest 2020 labels Oct 17, 2020
@LonelyPrincess LonelyPrincess self-assigned this Oct 17, 2020
Copy link
Owner

@danielcaldas danielcaldas left a comment

Choose a reason for hiding this comment

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

Nicelly done 馃憦 I didn't tought of adding the node parameters to all the callbacks in the API, but looking at it done, it made me thought that it's actually better to have all the callback APIs aligned. Thanks! I'll leave this one for @terahn to merge

docs/DOCUMENTATION.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Owner

@danielcaldas danielcaldas left a comment

Choose a reason for hiding this comment

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

Sorry got to carried away by the changes! I actually want to request changes before approving 馃槃 Please see my notes on some of the changes that we need to drop before merging. Cheers!

@LonelyPrincess
Copy link
Collaborator Author

Thanks for the feedback! 馃槂

I didn't tought of adding the node parameters to all the callbacks in the API, but looking at it done, it made me thought that it's actually better to have all the callback APIs aligned.

Yeah, that's what I thought too! It's better if we provide the same information in all callbacks, rather than having this added in just one. It's very possible that users request this data in different scenarios after all: even if the original issue asked for onMouseOverNode, someone else may have a similar use case but when they click on the node, for example.

I actually want to request changes before approving 馃槃 Please see my notes on some of the changes that we need to drop before merging.

I've just dropped the commits that updated the README, DOCUMENTATION and package.json files, just as you requested. Hope it's okay now! 馃憤馃徎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation add documentation to the repository or the the code base enhancement improving existent functionality or performance related Hacktoberfest Hacktoberfest label hacktoberfest-accepted Label for accepted PRs for Hacktoberfest 2020
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants