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

Global `viewGenerator` included in default config object #131

Merged

Conversation

@LonelyPrincess
Copy link
Collaborator

LonelyPrincess commented Oct 13, 2018

As described in #130 , the viewGenerator function value was been ignored when it was set in the Graph config object. This was probably due to the fact that the config object is not stored as it comes, since its properties are filtered to match only those which exists in the default config object.

This PR addresses this issue, and now it's possible to successfully add a high level viewGenerator function without having to override its value for every node in the graph.

When installing the library in a project, the `viewGenerator` function
defined in the config object by the user did have no effect. The reason
for this was that properties not included in the default config object
(graph.config.js) were been ignored when building the graph and
rendering it in the browser.

In order to solve this issue, a `viewGenerator` property is now included
in the default config object, with an initial value of `null`.
The current sandbox example created a form that allowed all of the
available config properties to be customized in real time.

The `viewGenerator` property, though, cannot be customized with a form.
Since its value must be a function, the user cannot set it via a form
field.

Also, the fact that it's initialized to null made some of the sandbox
examples to throw an error because the form schema generator doesn't
support neither null or undefined values. Functions are not allowed
either, and in this case an error would appear on screen by saying that
this type of data is not supported.

The current change filters the config elements so that neither null,
undefined or function values are included in the config form.
Copy link
Owner

danielcaldas left a comment

Hey thanks for this @LonelyPrincess

@danielcaldas danielcaldas merged commit c3029ab into danielcaldas:master Oct 14, 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
…s#131)

* fix: Add `viewGenerator` to default config

When installing the library in a project, the `viewGenerator` function
defined in the config object by the user did have no effect. The reason
for this was that properties not included in the default config object
(graph.config.js) were been ignored when building the graph and
rendering it in the browser.

In order to solve this issue, a `viewGenerator` property is now included
in the default config object, with an initial value of `null`.

* fix: Don't show function or null properties in sandbox

The current sandbox example created a form that allowed all of the
available config properties to be customized in real time.

The `viewGenerator` property, though, cannot be customized with a form.
Since its value must be a function, the user cannot set it via a form
field.

Also, the fact that it's initialized to null made some of the sandbox
examples to throw an error because the form schema generator doesn't
support neither null or undefined values. Functions are not allowed
either, and in this case an error would appear on screen by saying that
this type of data is not supported.

The current change filters the config elements so that neither null,
undefined or function values are included in the config form.

* test: Update expected value for `viewGenerator`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.