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

Error clicking nodes composed of mdi-react icons #201

Closed
zerans opened this issue May 10, 2019 · 2 comments · Fixed by #207
Closed

Error clicking nodes composed of mdi-react icons #201

zerans opened this issue May 10, 2019 · 2 comments · Fixed by #207
Assignees
Labels

Comments

@zerans
Copy link

zerans commented May 10, 2019

Describe the bug
When using an icon from mdi-react in a node, clicking the node throws an error.

Looks like the offending code is here:

      if (e.target.tagName.toUpperCase() === "SVG" && e.target.attributes.name.value === "svg-container-" + _this.state.id) {
        _this.props.onClickGraph && _this.props.onClickGraph();
      }

The icon node is SVG, but doesn't have a name attribute.

To Reproduce

  1. Configure a graph with a viewGenerator like this, where AlertIcon is imported from mdi-react
        viewGenerator: (node) => (<AlertIcon/>),
  1. Click a few nodes on the graph (might take a few clicks for some reason)

  2. Observe error:

Graph.js:442 Uncaught TypeError: Cannot read property 'value' of undefined
    at Graph._this.onClickGraph (Graph.js:442)
    at HTMLUnknownElement.callCallback (react-dom.development.js:147)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:196)
    at invokeGuardedCallback (react-dom.development.js:250)
    at invokeGuardedCallbackAndCatchFirstError (react-dom.development.js:265)
    at executeDispatch (react-dom.development.js:571)
...

Expected behavior
The onClickNode handler should be called

Environment:

  • OS: macOS
  • Browser: Chrome
  • Version: 74
  • react-d3-graph version react-d3-graph@2.0.2
@danielcaldas
Copy link
Owner

Hi @zerans thanks for reporting, will look into it asap, in the meantime if anyone wants to take a look at it It just a matter of making sure that this e.target.attributes.name.value is accessible by checking for the existence of the property.

@stale stale bot added the wontfix label Jun 21, 2019
Repository owner deleted a comment from stale bot Jun 22, 2019
@stale stale bot removed the wontfix label Jun 22, 2019
@danielcaldas danielcaldas self-assigned this Jun 23, 2019
@danielcaldas
Copy link
Owner

Released in [2.1.0](Released in 2.1.0 🎉) 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants