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

chore(pf5): revamp general table styles and upgrade archive/events views to PF5 #1334

Merged
merged 15 commits into from
Sep 3, 2024

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Aug 23, 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:

  • Implemented ideas for improving overall UI related to Tables.
  • Upgrades to PF5 are included for the following view along the way:
    • Archive view to Patternfly 5 (Dark theme should also be compatible here).
    • Event view to Patternfly 5 (Dark theme should also compatible here).

Notes: Security view can use some of these changes here. Maybe those can be applied in #1333

Minor changes

Changes Description Screenshot
Update placeholder texts to reflect "finding" functionality & Use <SearchInput /> instead of <TextInput /> component Finding is not the same as filtering (i.e. persistent) image
Extend Recording table card to fill all available spaces & Toolbar is not sticky anymore This is done to support scrolling (i.e. needed a concrete height) & The table is already sticky so toolbar does not need to. image
Tables in Archive view are now extended to fill and scrollable Consistent with Recording view image
Archive counts are presented with clickable file icons. If clicked, expanded view is toggle It gives a feeling of "open"/"close" to see/unsee files image
A minimal/clean & copy-able code block look for Match Expression Nicer look for "code-like" data image

Major changes

Changes Description Screenshot
New Automated Rule Table with combined description, scrollability and additional search bar "About" definition is closer to the table. Search bar allows easier finding rules. image

@tthvo tthvo added chore Refactor, rename, cleanup, etc. feat New feature or request safe-to-test labels Aug 23, 2024
@tthvo tthvo requested a review from a team August 23, 2024 11:22
@tthvo tthvo marked this pull request as draft August 24, 2024 00:12
@tthvo tthvo marked this pull request as ready for review August 24, 2024 02:32
@tthvo tthvo changed the title chore(pf5): revamp general table styles and upgrade archive views to PF5 chore(pf5): revamp general table styles and upgrade archive/events views to PF5 Aug 24, 2024
@andrewazores
Copy link
Member

Something funny happens when scrolling the Achives nested tables:

image

image

It seems like the inner table can overlap the outer table's column headers.

@andrewazores
Copy link
Member

Not sure exactly what I did to trigger this - I was just on Topology, clicking around on things and getting notifications from harvester pushes and automated rules.

20240826_11h22m59s_grim

@tthvo
Copy link
Member Author

tthvo commented Aug 26, 2024

Something funny happens when scrolling the Achives nested tables:

Ahh, z-index was rather funny. Fixed now :D

Not sure exactly what I did to trigger this - I was just on Topology, clicking around on things and getting notifications from harvester pushes and automated rules.

Oh yes, I have been investigating this for some time. I think it has something to do with Cola layout performance or layout was something messed up. Still checking but I think a solution can be to use a different layout https://www.patternfly.org/topology/layouts.

Might be related: patternfly/react-topology#10

@tthvo
Copy link
Member Author

tthvo commented Aug 26, 2024

Also, all reviews above for localization are addressed ^^

@tthvo
Copy link
Member Author

tthvo commented Aug 26, 2024

Not sure exactly what I did to trigger this - I was just on Topology, clicking around on things and getting notifications from harvester pushes and automated rules.

Oh yes, I have been investigating this for some time. I think it has something to do with Cola layout performance or layout was something messed up. Still checking but I think a solution can be to use a different layout https://www.patternfly.org/topology/layouts.

Might be related: patternfly/react-topology#10

I noticed that sometimes when this happens, reloading will render the view with errors and requests for discovery tree/target lists are returning status code 500. Logs should some error with JDBC connection.

{"details":"Error id 59d3b77c-e39f-41a6-b01f-5581497fba5f-343","stack":""}

Log: https://gist.github.com/tthvo/05e808ad824bbf728ab4af097e20b7dd

@andrewazores andrewazores merged commit d7ce2a5 into cryostatio:pf5 Sep 3, 2024
14 of 18 checks passed
@tthvo tthvo deleted the pf5-tables branch September 3, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. feat New feature or request safe-to-test
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants