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

FIX: layout problem on collapsing a node with many children 2 #5

Merged

Conversation

catosaurusrex2003
Copy link
Contributor

Changes in the PR:

  1. added a removeEdgesByParent function to remove the remnant edges of a node when we are collapsing it.
  2. moved instance of dagreGraph and getLayoutedElements one level up from Node.tsx -> App.tsx so that every time our App.tsx rerenders a new fresh layout is created.
  3. added copyfiles as dev dependency cuz it is used in a script in package.json

related:
asyncapi/website#3334

@catosaurusrex2003 catosaurusrex2003 changed the title done FIX: layout problem on collapsing a node with many children 2 Nov 2, 2024
Copy link
Owner

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Can you get rid of the changes in your package.lock file?

@catosaurusrex2003
Copy link
Contributor Author

I removed the changes to package-lock.json as requested.

Just so I can better understand for future contributions, could you share when to include or exclude package-lock.json changes ?

@AceTheCreator
Copy link
Owner

AceTheCreator commented Nov 15, 2024

I removed the changes to package-lock.json as requested.

Just so I can better understand for future contributions, could you share when to include or exclude package-lock.json changes ?

That was a misunderstanding, i didn't realize you made changes to the package.json file 😄

@AceTheCreator
Copy link
Owner

@catosaurusrex2003 please kindly resolve the conflict and please return the package.lock so i can review and merge it 🙏🏽

@catosaurusrex2003
Copy link
Contributor Author

catosaurusrex2003 commented Nov 15, 2024

@AceTheCreator done resolving
( i accidentally made one commit from my friends git creds 😆 )

Copy link
Owner

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Looks good @catosaurusrex2003

my tiny suggestion would be when a node is collapsed, we should set the center of the view to the collapsed node. wyt?

@catosaurusrex2003
Copy link
Contributor Author

@AceTheCreator
nice suggestion, done 👍

@catosaurusrex2003
Copy link
Contributor Author

one more problem I found is

when we open or close any nodes.
the whole layout changes. which makes it hard to keep track of where other nodes are as they keep on changing position on every click.

I am willing to fix this but this will require a lot of changes.
should i do it in this pr.
or wait for this to get merged and create a new one for it.

@AceTheCreator
Copy link
Owner

one more problem I found is

when we open or close any nodes. the whole layout changes. which makes it hard to keep track of where other nodes are as they keep on changing position on every click.

I am willing to fix this but this will require a lot of changes. should i do it in this pr. or wait for this to get merged and create a new one for it.

Yea, feel free to open a new PR 👍🏾

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.

3 participants