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(topology): fix Cola layout failing due to pending outdated graph elements #1336

Merged
merged 9 commits into from
Sep 3, 2024

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Aug 26, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Related to #1303

Description of the change:

Graph Layout was previously failing due to undefined Controller (i.e. via getController func). My speculation is that as new Model (i.e. on discovery tree refresh) is set on the controller, old graph elements are still animating, thus causing the issue. The combination of the following changes seem to fix:

  • Debounce (100ms) discovery tree refresh when new target events are emitted. Optional but nice to have.
  • Destroy graph layout before setting new model.

Minor changes

Changes Description Screenshot
Reset view should expand all collapsed groups A quick way to re-expand all collapsed groups. Previously, manual unpacking for each group is required N/A
Alert and resolver for nodes should be rendered as a vertical list and action button is now inline with sentence Previously, Flex renders them inline (horizontally) if enough width is available image
Destroy old graph before initializing new model Previously, stale elements are causing a recursive fails on graph layouts N/A
Set Menu Max Height for filter select menus Previously, the menu can become very long and overflow the view image

@tthvo tthvo requested review from andrewazores and a team August 26, 2024 23:41
@tthvo tthvo marked this pull request as ready for review August 26, 2024 23:41
@tthvo
Copy link
Member Author

tthvo commented Aug 26, 2024

Hey @andrewazores, with these changes, I haven't encountered the issue again. But maybe can you double-check?

@tthvo tthvo changed the title fix(topology): UI improvement for topology view fix(topology): fix Cola layout failing due to pending outdated graph elements Aug 26, 2024
@andrewazores
Copy link
Member

Related: #1334 (comment)

Copy link
Member

@andrewazores andrewazores 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, the previous issue is not exactly easy to reproduce but this doesn't cause any new apparent issues at least.

@andrewazores andrewazores merged commit 7eadcff into cryostatio:pf5 Sep 3, 2024
17 of 21 checks passed
@tthvo tthvo deleted the pf5-topology branch September 3, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants