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.

Sign up for GitHub

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

Jump to bottom

feat: update node / edge #38

Merged
merged 8 commits into from
Feb 22, 2024
Merged

feat: update node / edge #38

merged 8 commits into from
Feb 22, 2024

Conversation

sroussey
Copy link
Contributor

No description provided.

@sroussey
Copy link
Contributor Author

Still working on this. There is a lot of internal state footguns, so I will protect against those or remove some

@clarkmcc
Copy link
Owner

@sroussey yes, there are plenty of foot guns. I reported concerns with combining custom state management with the setNodes and setEdges functions on the xyflow discord and they said that they're working on cleaning this up. In the meantime, I had to introduce our own state management solution because it was preparatory to introducing node groups.

Let me know when you're happy with this and I'll take a peak

@clarkmcc clarkmcc mentioned this pull request Feb 11, 2024
Draft
@sroussey
Copy link
Contributor Author

OK, this is what I have been using. I took out the replace version. you can always replace everything in an update, or do a remove and add. I don't think it was usefull.

@clarkmcc
Copy link
Owner

Is this ready for review? I see there's a method commented out so wasn't sure whether you were happy with where this was at.

@sroussey
Copy link
Contributor Author

I don't see a method commented out. It looks ready, and I've been using it.

@clarkmcc clarkmcc merged commit be934bb into clarkmcc:main Feb 22, 2024
1 check passed
sroussey added a commit to sroussey/ngraph that referenced this pull request May 2, 2024
@sroussey
feat: update node / edge (clarkmcc#38)
12169f7 
* feat: update node / edge

* Add get Node/Edge

* remove the replace Node/Edge

* Update formatting

* remove comment about fn not available
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sroussey @clarkmcc