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

Use fx and fy when initializing nodes #353

Conversation

TranquilMarmot
Copy link
Contributor

@TranquilMarmot TranquilMarmot commented Aug 2, 2020

Fixes #350, fixes #295

Tested by setting sandbox/data/default.js to the following:

module.exports = {
    nodes: [
        {
            id: "node-1",
            fx: 100,
            fy: 100,
        },
        {
            id: "node-2",
            fx: 200,
            fy: 200,
        },
        {
            id: "node-3",
            fx: 300,
            fy: 300,
        },
    ],
    links: [
        {
            source: "node-1",
            target: "node-2",
        },
    ],
};

Before this change, node-3 would render in a different spot every time the page was refreshed.

With this change it always renders at 300,300.

Comment on lines +39 to +42
links: [
{ source: "A", target: "B" },
{ source: "C", target: "A" },
],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes in this file are just from running prettier 😄

@TranquilMarmot
Copy link
Contributor Author

In testing this locally it also seems to fix #295

@chetanladha
Copy link
Contributor

@danielcaldas - Any idea when this can be merged and released? I tried with these changes in node_modules/react-d3-graph and it does fix the issue we are facing. So would like to understand when this fix will be available.

Just to give some context, we are also facing the issue #295. We have are using staticGraph=true and user can filter the graph view. When user switch back from filtered view to unfiltered view, existing nodes do not respect the given x,y position and it results in overlapping nodes.

@TranquilMarmot - is there any work-around, until this fix is released?

@TranquilMarmot
Copy link
Contributor Author

I have not found any workarounds for this, no. I'm currently using a locally-published version of this library with a bunch of changes merged in (basically, all the open PRs against this repo 😄 )

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.

Hey, @TranquilMarmot thanks for the change! Looks good 👍

@danielcaldas danielcaldas merged commit effe595 into danielcaldas:master Aug 7, 2020
@danielcaldas
Copy link
Owner

fyi @piotrdelikat

@chetanladha
Copy link
Contributor

Thanks @danielcaldas and @TranquilMarmot.

@danielcaldas - Is there any release planned in near future?

@danielcaldas
Copy link
Owner

There are a few more PRs in the pipeline before releasing a new version. Should still take a few weeks. Cheers!

Repository owner locked as resolved and limited conversation to collaborators Aug 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants