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

DagreGroupsLayout doesn't keep the graph order #230

Closed
lordrip opened this issue Aug 7, 2024 · 1 comment · Fixed by #231
Closed

DagreGroupsLayout doesn't keep the graph order #230

lordrip opened this issue Aug 7, 2024 · 1 comment · Fixed by #231

Comments

@lordrip
Copy link
Contributor

lordrip commented Aug 7, 2024

Describe the problem
Using the DagreGroupsLayout class forces the groups to be rendered first in the graph, altering the layout when expanding/collapsing elements.

How do you reproduce the problem?

  1. Having the following graph
    image

  2. Collapse the when node
    image

  3. Notice how otherwise is now at the left of the now collapsed when node

Expected behavior
For deterministic graphs like those used for Camel, the order matters, so the graph must always keep the order.

Is this issue blocking you?
I ended up copying the DagreGroupsLayout class and adding the @dagrejs/dagre dependency to the project so I can incorporate a sort by index.

What is your environment?

  • OS: [Linux]
  • Browser [Firefox]
  • Version [130.0b1 (64-bit)]

What is your product and what release date are you targeting?

Kaoto

@github-project-automation github-project-automation bot moved this to Needs triage in PatternFly Issues Aug 7, 2024
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 7, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 7, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 7, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 7, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 7, 2024
feat(Canvas): Add containers for branches

Currently, all children are rendered as individual nodes in the Canvas,
making complicated to understand when a step belongs to a parent EIP or
if it comes as a next step in the Camel Route.

This commit adds support for rendering EIP branches as containers.

fix: KaotoIO#368

feate(Canvas): Extend DagreGroupsLayout

Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 8, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 8, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 8, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 8, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 8, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
lordrip added a commit to KaotoIO/kaoto that referenced this issue Aug 8, 2024
Currently, `DagreLayout` is used to render the graph, and while it does
work, it doesn't support nested groups properly due to a limitation in
Dagre itself.

In the `@patternfly/react-topology` there's another layout called `DagreGroupsLayout` that performs individual layout inside each group, providing a better result when the graph contains nested groups.

This commit brings that class over and uses the initial node index for
sorting the graph node, this way the order is kept when expanding/collapsing nodes in the Canvas.

Related issue: patternfly/react-topology#230
@jeff-phillips-18 jeff-phillips-18 self-assigned this Aug 8, 2024
@github-project-automation github-project-automation bot moved this from Needs triage to Done in PatternFly Issues Aug 8, 2024
Copy link

github-actions bot commented Aug 8, 2024

🎉 This issue has been resolved in version 5.4.0-prerelease.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

lordrip added a commit to lordrip/kaoto that referenced this issue Aug 8, 2024
Now that patternfly/react-topology#230 is
completed, we can remove the extended version of the
`DagreGroupsLayout` and use the library directly.

relates: patternfly/react-topology#230
lhein pushed a commit to lordrip/kaoto that referenced this issue Aug 9, 2024
Now that patternfly/react-topology#230 is
completed, we can remove the extended version of the
`DagreGroupsLayout` and use the library directly.

relates: patternfly/react-topology#230
lhein pushed a commit to KaotoIO/kaoto that referenced this issue Aug 9, 2024
Now that patternfly/react-topology#230 is
completed, we can remove the extended version of the
`DagreGroupsLayout` and use the library directly.

relates: patternfly/react-topology#230
lordrip added a commit to lordrip/kaoto that referenced this issue Aug 9, 2024
After upgrading [`@patternfly/react-topology`](KaotoIO#1302)
calculating the layout inside of Kaoto is not needed anymore, so
`@dagre/dagrejs` is also not needed.

relates: patternfly/react-topology#230
lordrip added a commit to KaotoIO/kaoto that referenced this issue Aug 9, 2024
After upgrading [`@patternfly/react-topology`](#1302)
calculating the layout inside of Kaoto is not needed anymore, so
`@dagre/dagrejs` is also not needed.

relates: patternfly/react-topology#230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants