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

Component | Graph: Allow user to set node positions #397

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

lee00678
Copy link
Collaborator

@lee00678 lee00678 commented Jun 3, 2024

  • Simply extend GraphInputNode to allow the user to set node positions. This should make save and reload chart possible.
  • Added a dev example for this
  • Updated Doc

@lee00678 lee00678 requested a review from reb-dev June 3, 2024 22:01
@reb-dev
Copy link
Collaborator

reb-dev commented Jun 4, 2024

@lee00678 I don't see a new dev example, did you forget to add it in in this commit 6786b79?

Copy link
Collaborator

@reb-dev reb-dev left a comment

Choose a reason for hiding this comment

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

I think we need to add some more implementation for this.

Right now, providing x and y will automatically force the node positions (at least for GraphLayoutType.Force, the other ones would get overridden I think). Shouldn't we prevent that from happening if GraphLayoutType.Precalculated isn't specified?

@lee00678 lee00678 force-pushed the qian/graph-input-node branch from 607ad26 to 6b59a6e Compare June 4, 2024 17:59
@lee00678
Copy link
Collaborator Author

lee00678 commented Jun 4, 2024

I think we need to add some more implementation for this.

Right now, providing x and y will automatically force the node positions (at least for GraphLayoutType.Force, the other ones would get overridden I think). Shouldn't we prevent that from happening if GraphLayoutType.Precalculated isn't specified?

That's a good point. I think you are right. I will update.

@lee00678 lee00678 force-pushed the qian/graph-input-node branch from 60ea605 to 3332be5 Compare June 5, 2024 19:26
@reb-dev
Copy link
Collaborator

reb-dev commented Jun 26, 2024

My previous comment no longer applies after merging #396, we can just add x and y to nodes without it interfering with force layout.

@lee00678 Can you revert this PR to include your original changes? Then we can merge it

@lee00678 lee00678 force-pushed the qian/graph-input-node branch from 3332be5 to ad76252 Compare June 26, 2024 19:40
@lee00678
Copy link
Collaborator Author

@lee00678 Can you revert this PR to include your original changes? Then we can merge it

@reb-dev Done!

@rokotyan
Copy link
Contributor

@lee00678 Maybe I'm missing something but the changelog still shows me this
image

@lee00678 lee00678 force-pushed the qian/graph-input-node branch from ad76252 to 15a5e6e Compare June 26, 2024 20:18
@lee00678
Copy link
Collaborator Author

@lee00678 Maybe I'm missing something but the changelog still shows me this image

Fixed.

@reb-dev reb-dev merged commit 9849774 into main Jun 27, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants