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

OnNodePositionChange fires unreliably. #284

Closed
azraeiden opened this issue Jan 9, 2020 · 6 comments
Closed

OnNodePositionChange fires unreliably. #284

azraeiden opened this issue Jan 9, 2020 · 6 comments
Labels
duplicate already reported wontfix

Comments

@azraeiden
Copy link

Describe the bug
When creating a graph with OnNodePositionChange set to a function, upon manually moving a node, the OnNodPositionChange function only fires sometimes.

To Reproduce
Steps to reproduce the behavior:

The most basic graph i can create, where I copy the 3 node example into a create-react-app, exhibits this behavior

Code:
App.js:
/////////////////////////////////////////////////////////////////////////////////////////////////////
import React from 'react';
import logo from './logo.svg';
// @ts-ignore
import './App.css';
import Thegraph from './components/Thegraph.js'
// import About from './About'

function App() {
return (




);
}

export default App;

//////////////////////////////////////////////////////////////////////////////////////////////

Thegraph.js:

/////////////////////////////////////////////////////////////////////////////////////////////////////
import React from 'react';
import { Graph } from 'react-d3-graph';

// graph payload (with minimalist structure)
const data = {
nodes: [{ id: "Harry" }, { id: "Sally" }, { id: "Alice" }],
links: [{ source: "Harry", target: "Sally" }, { source: "Harry", target: "Alice" }],
};

// the graph configuration, you only need to pass down properties
// that you want to override, otherwise default ones will be used

function Thegraph() {

const myConfig = {
nodeHighlightBehavior: true,
node: {
color: "lightgreen",
size: 120,
highlightStrokeColor: "blue",
},
link: {
highlightColor: "lightblue",
},
directed: true
};

// graph event callbacks
const onClickGraph = function() {
console.log(Clicked the graph background);
};

const onClickNode = function(nodeId) {
console.log(Clicked node ${nodeId});
};

// eslint-disable-next-line
const onDoubleClickNode = function(nodeId) {
console.log(Double clicked node ${nodeId});
};

const onRightClickNode = function(event, nodeId) {
console.log(Right clicked node ${nodeId});
};

// eslint-disable-next-line
const onMouseOverNode = function(nodeId) {
console.log(Mouse over node ${nodeId});
};

// eslint-disable-next-line
const onMouseOutNode = function(nodeId) {
console.log(Mouse out node ${nodeId});
};

const onClickLink = function(source, target) {
console.log(Clicked link between ${source} and ${target});
};

const onRightClickLink = function(event, source, target) {
console.log(Right clicked link between ${source} and ${target});
};

// eslint-disable-next-line
const onMouseOverLink = function(source, target) {
console.log(Mouse over in link between ${source} and ${target});
};

// eslint-disable-next-line
const onMouseOutLink = function(source, target) {
console.log(Mouse out link between ${source} and ${target});
};

const onNodePositionChange = function(nodeId, x, y) {
console.log(Node ${nodeId} is moved to new position. New position is x= ${x} y= ${y});
};

return <Graph
id="graph-id" // id is mandatory, if no id is defined rd3g will throw an error
data={data}
config={myConfig}
onClickNode={onClickNode}
onRightClickNode={onRightClickNode}
onClickGraph={onClickGraph}
onClickLink={onClickLink}
onRightClickLink={onRightClickLink}
onNodePositionChange={onNodePositionChange}
/>
}

export default Thegraph
///////////////////////////////////////////////////////////////////////////////////////////

Expected behavior
OnNodePositionChange should fire the function every time a node is moved.

Screenshots
If applicable, add screenshots to help explain your problem.

Environment:

  • OS: iOS
  • Browser Chrome
  • Version 78.0.3904.70
  • Node version Unsure
  • react-d3-graph version 2.3.0
  • d3 version 5.14.2
  • react version 16.12.0

Additional context
OnNodePositionChange fires its function much more often with short, very fast movements. It reaches near 100% firing rate under such conditions. With long, slow, movements, the firing rate is more like 50%.

@danielcaldas
Copy link
Owner

Hello @azraeiden thanks for the detailed report. Are you able to reproduce the same issue in the live sandbox example? Also, there is pretty easy to detect onNodePositionChange calls since we log it at every time a node position changes.

image

I tested it myself and I could not find so far any anomaly in the calls to onNodePositionChange. Would you be able to provide more context into this? Thanks in advance!

@danielcaldas danielcaldas added the needs more info not enough proof to specify issue label Jan 11, 2020
@azraeiden
Copy link
Author

azraeiden commented Jan 13, 2020

Thanks for the response @danielcaldas. Yes I was able to reproduce the issue. In the screenshot I have shown below I moved nodes Guardian and Broodmhome a total of four times, and as you can see onNodePositionChange only was called twice. I tested it a few more times, and maintained an onNodePositionChange fire rate of about 60%. Also I'm using the high contrast plugin, which is why the screen may look different, but I disabled the plugin and was still able to reproduce the issue.

Edit: Someone else I work with was also able to reproduce the issue. Still greater than 50% fire rate, but unreliable.

Screen Shot 2020-01-13 at 12 08 01 PM

@Kav91
Copy link
Contributor

Kav91 commented Jan 24, 2020

@danielcaldas @azraeiden pretty sure this is the same thing I fixed awhile back in #264
Fixed it in this PR thats been merged already #266
Would be worth creating a new release, so people can get the latest fixes like this.
I'm still using a personal release because of this.

@danielcaldas
Copy link
Owner

I'll try to have a release out asap @Kav91

@danielcaldas danielcaldas added bug duplicate already reported and removed duplicate already reported labels Mar 1, 2020
@danielcaldas
Copy link
Owner

Closing as a duplicate of #266

@danielcaldas danielcaldas added duplicate already reported wontfix and removed bug needs more info not enough proof to specify issue labels Mar 1, 2020
@azraeiden
Copy link
Author

Hey, just wanted to say thank you for fixing this. It works perfectly.

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

No branches or pull requests

3 participants