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

Added ability to configure a node's width and height separately #342

Merged

Conversation

terahn
Copy link
Collaborator

@terahn terahn commented Jul 1, 2020

From #336, added the ability to configure a node's width and height separately when using custom nodes. For example:

node:  {
    size: {
        width: 300,
        height: 200,
    },
}

This does not break any existing configurations. Using this with the standard node shapes will cause the links to disappear, however there is no reason to use a different height and width for these anyways since the d3 shapes cannot be stretched. I explicitly stated that this should only be used for custom nodes in the documentation, as well as added a console.warn() to discourage this incorrect use.

I think this is an important feature for people who want to use custom nodes because when using a custom node with an unequal height and width, unnecessary white space has to be added in order for the node to not be cut off. For example:

react-d3-graph-node-size-pull

Let me know your thoughts on this!

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.

Thanks! This is looking great, just have a few comments that I would like you to address before we move on and merge this. 👏 👏

sandbox/utils.js Show resolved Hide resolved
src/components/graph/graph.builder.js Outdated Show resolved Hide resolved
src/components/graph/graph.builder.js Outdated Show resolved Hide resolved
src/components/node/Node.jsx Outdated Show resolved Hide resolved
src/components/node/Node.jsx Outdated Show resolved Hide resolved
@terahn terahn force-pushed the feature/node-size-width-and-height branch from 8fa862a to d42105b Compare July 2, 2020 19:43
@terahn terahn force-pushed the feature/node-size-width-and-height branch from d42105b to 3c52d04 Compare July 2, 2020 19:52
@terahn
Copy link
Collaborator Author

terahn commented Jul 2, 2020

Thanks for the feedback, I've implemented those changes now in the latest commit.

p.s. Ignore the flurry of force pushes, I messed up a bit initially and wanted to clean up the commits a bit 😆

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.

Ready to go 🚀

@danielcaldas danielcaldas merged commit 3e1af4a into danielcaldas:master Jul 3, 2020
danielcaldas added a commit that referenced this pull request Jul 3, 2020
danielcaldas added a commit that referenced this pull request Jul 3, 2020
* Misc improvements cleanup from previous PRs

-  Revert DOCUMENTATION.md to previous version
- Adjustments in documentation
- Add copy to clipboard button to sandbox for graph configuration field
- Remove eslint rule new-line-after-var
- Small refactor around graph.builder and graph.helper
- Small refactor marker.helper

* Small improvements in sandbox improve copy to clipboard

* Fix unit test

* Add sample for new size API @terahn #342

* Allow users to select the node.labelPosition in the form

* Fix #322

* Fix #323
@terahn terahn deleted the feature/node-size-width-and-height branch July 3, 2020 16: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.

None yet

2 participants