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): upgrade Settings view to Patternfly 5 #1330

Merged
merged 31 commits into from
Aug 22, 2024

Conversation

tthvo
Copy link
Member

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

  • Upgraded Settings View to Patternfly 5.
  • Dark mode should be functional now.

Minor changes

Changes Description Screenshot
Descriptions for Feature level selection Such explanations can be useful for users that are not familiar with these (i.e. beta) notions image
Full word for unit of websocket retry config This is consistent other configurations, for example, Auto Refresh image
Configurations are formatted to be human-friendly This ensures consistency with Recording View and elsewhere image

Major changes

Changes Description Screenshot
Dedicated action list for saving configuration form This ensures consistency of Form pattern across the UI image
New theme toggle on masthead Switching theme should be quick & seamless. This design is pretty common. For example, Podman docs andn Patternfly docs. For us, we follow the Patternfly way image

@tthvo tthvo added chore Refactor, rename, cleanup, etc. safe-to-test labels Aug 19, 2024
@tthvo tthvo marked this pull request as ready for review August 21, 2024 06:10
@tthvo tthvo requested a review from a team August 21, 2024 06:10
@andrewazores
Copy link
Member

Awesome!

image

I tried typing English (Canada) in the search field here, but it blows up the page :-) As soon as I hit ( there was an uncaught error about "unterminated parenthetical". Same thing if I type ( in the timezone search.

image

Maybe this should be Millisecond*s*?

@tthvo
Copy link
Member Author

tthvo commented Aug 21, 2024

I tried typing English (Canada) in the search field here, but it blows up the page :-) As soon as I hit ( there was an uncaught error about "unterminated parenthetical". Same thing if I type ( in the timezone search.

Uhhh opps, my bad ^^ Looks like we didn't escape the search text (seems to happen everywhere else too). It should be fixed now :D

Maybe this should be Millisecond*s*?

Yep just did^^

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2024

In the latest commit, I added some css fixes for elements (i.e. those that were upgraded to PF5) to be compatible with dark theme. I think we can fix the rest while upgrading other remaining views :D

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.

LGTM, @tthvo please take a quick look over the localizations I extracted in the last handful of commits.

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2024

LGTM, @tthvo please take a quick look over the localizations I extracted in the last handful of commits.

Oh nicee! Looks good to me! Thanks for handling that :D

@andrewazores andrewazores merged commit bc70e39 into cryostatio:pf5 Aug 22, 2024
14 of 18 checks passed
@tthvo tthvo deleted the pf5-settings branch August 22, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. safe-to-test
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants