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

Customize nodes with JSX views #103

Merged
merged 11 commits into from Sep 24, 2018

Conversation

@LonelyPrincess
Copy link
Collaborator

LonelyPrincess commented Aug 31, 2018

A new viewGenerator property can now be added to the Graph config object. The value for this property must be a function which will receive a node (exactly as it's passed down to the Graph component via the data prop) and return a view, as illustrated in the following example:

viewGenerator: (node) => <CustomViewComponent node={node} />

The forementioned property makes it possible to fully customize the look and feel of our nodes by using JSX, HTML and CSS. This feature can be useful when shapes, colors, labels and svg images don't provide enough customization for our nodes, mainly when we have multiple pieces of information we want to represent.

Please, note that if a single node has values for both svg and viewGenerator properties, the customized view will always prevail. If we want svg to be applied instead, we must not assign a value to viewGenerator.

A new 'view' property can now be passed down to graph nodes. This new
property will receive a view, which can include plain HTML or even
React components.

Please, note that this is only available as a low level property in
individual nodes and
cannot be defined in the config object. This feature is mostly thought
for dynamic content, so 'svg' property should be enough to cover any
scenario in which the nodes' content don't change depending on their
inner data.

Also, if the node has values for both 'svg' and 'view' properties,
'view' will always prevail. If we want 'svg' to be applied instead, we
must not assign a value to 'view'.
@danielcaldas

This comment has been minimized.

Copy link
Owner

danielcaldas commented Sep 3, 2018

I'll check this as soon as I have some time available, and I'll probably implement a small example on the sandbox to document the usage of this new feature, but overall looking good, nice job 🙌

EDIT: This PR addresses #90

@LonelyPrincess LonelyPrincess changed the title feat: Customize nodes with JSX views Customize nodes with JSX views Sep 4, 2018
@danielcaldas

This comment has been minimized.

Copy link
Owner

danielcaldas commented Sep 9, 2018

As it is right now I don't see what examples I could give to demonstrate the value that this could bring. I would love some feedback from @vivek1623 @igauravsehrawat and also @LonelyPrincess to see whether using this new feature is actually something useful. In the meantime I'm trying also to build some examples on my own

@LonelyPrincess

This comment has been minimized.

Copy link
Collaborator Author

LonelyPrincess commented Sep 10, 2018

It happened to be of use to me, so I hope my explanation can help you there. 🙋

As I said in the PR message, this feature could be useful when the content of the nodes should be dynamic. Right now we can customize the label, shape and color of the node. We can also use a SVG images to choose how the nodes would look in some cases, but... What if we wanted the nodes to contain information on each of the individual nodes?

Let's consider each node represents a person, for example. In this case, our nodes will contain, aside from their id, some additional fields with information such as gender, age or country of origin. With the current tools we have no way of representing all of that data inside of our graph.

We could use SVG if we only wanted to represent gender and country of origin at the same time, but still that would require us to have images for every possible combination. The more information we want to represent, the harder it will get to possibly maintain this with the tools already provided by the library.

With the feature proposed in this PR, however, we can create the node view dynamically, so we don't need to know all of the possibilities before hand. Since the view can receive data as props, it will be possible for us to display information specific for each node easily. Also, since it will basically be HTML / JSX code, we can use CSS to fully customize how our nodes are gonna look.

I think this is a pretty good thing to have for those looking forward to a more advanced layer of customization for their nodes. What we have now is nice, but the amount of information we can display with it is limited, and in my particular case it was not enough to cover the needs of the project I'm working at.

I hope all of this has helped you to determine whether this can be useful or not!

PS: If you're having a hard time trying to come up with an example, I can lend you a hand with that if you want. It should be no problem since it was me who got this feature in to begin with, so don't hesitate to ask. 👍

@danielcaldas

This comment has been minimized.

Copy link
Owner

danielcaldas commented Sep 10, 2018

Good one @LonelyPrincess! Your explanation clarifies a bit the motivation, if you please could add an example would be great. Please create a folder under sandbox/data/<name-of-example> then we can access to that sample via query parameter on the live sandbox, just like the small example
https://goodguydaniel.com/react-d3-graph/sandbox/index.html?data=small

Inside of the 'foreignObject' tag, there is now a body tag which will
wrap around the defined custom view. This wrapper has the same dimensions that were set as node size in the
config object.

Because this wrapper has explicit dimensions now, the developer will be
able to use relative sizes (such as percentages) in their custom view's
parent element.

This fixes an issue where the custom component's height
and width were set to 100%, and it failed to retrieve the size for the
'foreignObject' tag. Having a real parent in the embedded HTML solves the problem, as our new view is no longer unable to retrieve its dimensions.
The sandbox includes now a new example where the custom view property is
demonstrated.

In this case, each node represents a person and, for every one of them,
an additional set of data is included. The defined custom view displays
all of this data: name, gender and whether they're able to drive a bike
and / or a car.

The values for the 'customView' property are generated dynamically in the
'custom-node.data.js' file.

In order for the user to watch this example, they must enter the sandbox
with the following query param:

    {default sandbox url}?data=custom-node
@LonelyPrincess

This comment has been minimized.

Copy link
Collaborator Author

LonelyPrincess commented Sep 19, 2018

Sorry for the late answer, @danielcaldas ! I've finally uploaded an example, just as you asked me to. You can check it out with ?data=custom-view.

It should be good enough to illustrate what this is about and give you an idea of this feature potential, but it presents an issue I need to fix before merging this PR. As it is now, the node contents are completely static in the sandbox. This is because the view property is not being updated when items in the graph data bellow are modified.

This made me realize the current implementation for this feature can be improved, and I'm in the way of doing so. What I want to do now is to pass a view generator function rather than the view itself to the config object:

{
    id: 'myNode',
    viewGenerator: (node) => <CustomViewComponent node={node} />
}

This view generator method will receive a node object with the following structure:

{
   id: 'node id',
   name: 'node name',
   data: {
      extra: 'extra field with additional information',
      extra2: 'another field with extra information, we can add as much as we want'
   }
}

The render method of the Node component will be in charge of calling this genertator function to obtain the updated view whenever the node should be re-rendered. This fixes the issue in the current demo, where the nodes are not being updated real-time when the data set is changed in the graph data form.

By using this new implementation, it will also be possible to add a similar property at global config, rather than having to specify the value of view on every node. It's very likely we'll use the same generator for everything, so it's far more comfortable to add it at root level.

Can you give me your feedback on this approach, @danielcaldas ? Do you have any other suggestion to make it better? 🙂

When trying to remove nodes in the sandbox example for the custom view
feature, the application threw the following error.

```
<body> tried to unmount. Because of cross-browser quirks it is
impossible to unmount some top-level components (eg <html>, <head>, and
<body>) reliably and efficiently. To fix this, have a single top-level
component that never unmounts render these elements.
```

This was introduced when I added a body tag as a wrapper for the
embedded code that serves as a custom view. Replacing that tag with a
lower-level one fixed the issue.
The `view` property where we could assign an already generated view to
our nodes has been replaced by a new property `viewGenerator`.

This new property can be defined both at the global configuration object or at node
level, and it will consists on a function that receives a node object and uses its data to return a
view:

```
viewGenerator: (node) => <CustomViewComponent node={node}
```

With the previous method, the node view was not automatically updated
whenever the node data changed. The only way for this to happen was by
manually overriding the value of the the `view` property.

With the current approach, there are a couple of advantages:

* The view generator can be defined in the config object, so we don't
have to set a property for the view in every node. All of the nodes will
share this global setting.

* The library will be in charge of rendering the node custom view
whenever the data changes, as the generator method receives the current
node data as a parameter and returns a view generated using this values. This means the developer doesn't have to worry about updating a view property on each node everytime their data changes, as the view will be automatically updated anytime the Node component `render` method is called.

The sandbox example for this feature has also been updated to reflect the new
changes.
Copy link
Owner

danielcaldas left a comment

Great job on the example and the viewGenerator idea. I'm really impressed with the results but I think there are a few details that need to be reviewed that is why I left some comments for you 😃 Just a small correction on your last comment, the actual query param is /?data=custom-node since it's the name of the sample directory.

Here is an image so that people can see how awesome and powerful this can be in terms of personalization.

viewgenerator

src/components/graph/graph.config.js Outdated Show resolved Hide resolved
src/components/graph/graph.helper.js Outdated Show resolved Hide resolved
src/components/node/Node.jsx Outdated Show resolved Hide resolved
@igauravsehrawat

This comment has been minimized.

Copy link

igauravsehrawat commented Sep 22, 2018

Regarding the feedback from my side: We started using https://github.com/DrummerHead/react-flow-diagram which is what we wanted for our stuff. There we could customize the node as it was a HTML Component and the layout was in SVG.

But this PR is so good. Kudos to @LonelyPrincess

P.S: Big apologies for late reply.

Thanks

When building a node component props, a new property called
`overrideViewGenerator` is added. This property is set to `true` when
there's a svg provided for an specific node which doesn't have its own
view generator. In this case, we assume the svg should have precedence
over the default view generator set in the global settings.

In any other scenario, the view generator will always be applied instead
of the svg. Only when `overrideViewGenerator` is `true` will we assume
the `svg` property to be more important.

Before, this was done by setting the `viewGenerator` property to `null`,
but the new property makes the code more readable.
Instead of passing only the node information entered by the user via the
`data` prop of the `Graph` component, the `viewGenerator` function will
now receive everything, including auto-generated data such as its
current coordinates in the graph or any values inherited from the
default config object.
@danielcaldas

This comment has been minimized.

Copy link
Owner

danielcaldas commented Sep 23, 2018

Well done! Everything looks nice now, just take a look at some breaking unit tests (due to the newly added properties like overrideGlobalViewGenerator) https://travis-ci.org/danielcaldas/react-d3-graph/builds/432191821?utm_source=github_status&utm_medium=notification and then we can merge this :)

The `buildNodeProps` related tests were failing due to the new
properties added in this PR. This included both `viewGenerator`,
`overrideGlobalViewGenerator`, as well as other properties inherited
from the `that.node` object.
Since some of the recently added values are directly inherited from the
`that.node` data object and `buildNodeProps` does not alter them in any
way, they are no longer hardcoded in the test `toEqual` check.

The object `that.node` contents are added via the spread operator
instead, which will make things easier if the sample data changes at some point.
@LonelyPrincess

This comment has been minimized.

Copy link
Collaborator Author

LonelyPrincess commented Sep 24, 2018

Oh, great! Thanks a lot for your feedback, @danielcaldas ! 😄

I've taken care of it, and the tests are no longer failing. I hope this new feature is useful to you! 😉

@danielcaldas danielcaldas merged commit b66e6e4 into danielcaldas:master Sep 24, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
zvolchak added a commit to zvolchak/react-d3-graph that referenced this pull request Oct 29, 2018
* feat: Customize nodes with JSX views

A new 'view' property can now be passed down to graph nodes. This new
property will receive a view, which can include plain HTML or even
React components.

Please, note that this is only available as a low level property in
individual nodes and
cannot be defined in the config object. This feature is mostly thought
for dynamic content, so 'svg' property should be enough to cover any
scenario in which the nodes' content don't change depending on their
inner data.

Also, if the node has values for both 'svg' and 'view' properties,
'view' will always prevail. If we want 'svg' to be applied instead, we
must not assign a value to 'view'.

* fix: Wrapper for node's custom view

Inside of the 'foreignObject' tag, there is now a body tag which will
wrap around the defined custom view. This wrapper has the same dimensions that were set as node size in the
config object.

Because this wrapper has explicit dimensions now, the developer will be
able to use relative sizes (such as percentages) in their custom view's
parent element.

This fixes an issue where the custom component's height
and width were set to 100%, and it failed to retrieve the size for the
'foreignObject' tag. Having a real parent in the embedded HTML solves the problem, as our new view is no longer unable to retrieve its dimensions.

* feat: Custom view sample added to sandbox

The sandbox includes now a new example where the custom view property is
demonstrated.

In this case, each node represents a person and, for every one of them,
an additional set of data is included. The defined custom view displays
all of this data: name, gender and whether they're able to drive a bike
and / or a car.

The values for the 'customView' property are generated dynamically in the
'custom-node.data.js' file.

In order for the user to watch this example, they must enter the sandbox
with the following query param:

    {default sandbox url}?data=custom-node

* fix: Error unmounting nodes with custom view in sandbox example

When trying to remove nodes in the sandbox example for the custom view
feature, the application threw the following error.

```
<body> tried to unmount. Because of cross-browser quirks it is
impossible to unmount some top-level components (eg <html>, <head>, and
<body>) reliably and efficiently. To fix this, have a single top-level
component that never unmounts render these elements.
```

This was introduced when I added a body tag as a wrapper for the
embedded code that serves as a custom view. Replacing that tag with a
lower-level one fixed the issue.

* feat: View generator function to customize nodes

The `view` property where we could assign an already generated view to
our nodes has been replaced by a new property `viewGenerator`.

This new property can be defined both at the global configuration object or at node
level, and it will consists on a function that receives a node object and uses its data to return a
view:

```
viewGenerator: (node) => <CustomViewComponent node={node}
```

With the previous method, the node view was not automatically updated
whenever the node data changed. The only way for this to happen was by
manually overriding the value of the the `view` property.

With the current approach, there are a couple of advantages:

* The view generator can be defined in the config object, so we don't
have to set a property for the view in every node. All of the nodes will
share this global setting.

* The library will be in charge of rendering the node custom view
whenever the data changes, as the generator method receives the current
node data as a parameter and returns a view generated using this values. This means the developer doesn't have to worry about updating a view property on each node everytime their data changes, as the view will be automatically updated anytime the Node component `render` method is called.

The sandbox example for this feature has also been updated to reflect the new
changes.

* docs: Fixed datatype for viewGenerator in config docs

* refactor: Inner property to override view generator

When building a node component props, a new property called
`overrideViewGenerator` is added. This property is set to `true` when
there's a svg provided for an specific node which doesn't have its own
view generator. In this case, we assume the svg should have precedence
over the default view generator set in the global settings.

In any other scenario, the view generator will always be applied instead
of the svg. Only when `overrideViewGenerator` is `true` will we assume
the `svg` property to be more important.

Before, this was done by setting the `viewGenerator` property to `null`,
but the new property makes the code more readable.

* refactor: Pass all node data to viewGenerator

Instead of passing only the node information entered by the user via the
`data` prop of the `Graph` component, the `viewGenerator` function will
now receive everything, including auto-generated data such as its
current coordinates in the graph or any values inherited from the
default config object.

* refactor: overrideViewGenerator property renamed to overrideGlobalViewGenerator

* test: buildNodeProps tests updated with new attributes

The `buildNodeProps` related tests were failing due to the new
properties added in this PR. This included both `viewGenerator`,
`overrideGlobalViewGenerator`, as well as other properties inherited
from the `that.node` object.

* test refactor: Implicit values inherited from node

Since some of the recently added values are directly inherited from the
`that.node` data object and `buildNodeProps` does not alter them in any
way, they are no longer hardcoded in the test `toEqual` check.

The object `that.node` contents are added via the spread operator
instead, which will make things easier if the sample data changes at some point.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.