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

feat(pipelines): Add pipeline groups layout #157

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

jeff-phillips-18
Copy link
Member

What

Closes #155
Closes RHOAIENG-4186

Description

Adds a DagreGroupsLayout and PipelineDagreGroupsLayout which treats each group as its own layout. Groups are connected to tasks or other groups.

Type of change

  • Feature

Screen shots / Gifs for design review

Horizontal

image

Vertical

image

/cc @andrewballantyne

Surge

https://pipeline-groups.surge.sh/

Copy link
Collaborator

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Group layouts look awesome @jeff-phillips-18 🎉

Wondering it if would make sense to update the default label positioning to "top" in order to align with the designs and visually distinguish where the groups begin... In the vertical example, the labels for Executions 1 and 2 are positioned over forks of edges so it's a bit tricky to figure out which series of nodes the execution label refers to. WDYT @jeff-phillips-18 ?

Screenshot 2024-03-05 at 9 47 37 AM

@andrewballantyne
Copy link

Question: You have a colour shift for the internal groups -- does this mean there are only allowed one layer of groups inside groups? If not, what does it look like if another group is inside the inner group?

@jeff-phillips-18
Copy link
Member Author

Question: You have a colour shift for the internal groups -- does this mean there are only allowed one layer of groups inside groups? If not, what does it look like if another group is inside the inner group?

@andrewballantyne the group background alternates:

image

@jeff-phillips-18 jeff-phillips-18 force-pushed the pipeline-groups-layout branch from b9fa2ed to 0b23db1 Compare March 5, 2024 20:37
@jeff-phillips-18 jeff-phillips-18 force-pushed the pipeline-groups-layout branch from 0b23db1 to f74883d Compare March 5, 2024 20:45
@jeff-phillips-18
Copy link
Member Author

Wondering it if would make sense to update the default label positioning to "top"

@jenny-s51 Moved them to the top for vertical layout (this is in the app code not the toolkit)

image

@andrewballantyne
Copy link

Design wise this lgtm -- I didn't dive into the code.

@dlabaj dlabaj self-requested a review March 7, 2024 21:15
Copy link
Contributor

@dlabaj dlabaj 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 ran it locally with out any issues. Code looks good.

@dlabaj dlabaj merged commit 0b25c55 into patternfly:main Mar 7, 2024
7 checks passed
Copy link

github-actions bot commented Mar 7, 2024

🎉 This PR is included in version 5.3.0-prerelease.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Node Subgraph Layout with Dagre
4 participants