build(deps): upgrade to Patternfly 5 #1303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewazores
added
dependencies
|
Hi @andrewazores! Add at least one of the required labels to this PR Required labels are : chore,ci,cleanup,docs,feat,fix,perf,refactor,style,test |
github-actions
bot
added
the
needs-triage
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
andrewazores
changed the title
andrewazores
removed
the
needs-triage
This was referenced Jul 25, 2024
|
Dropdown menu heights look like a PF 5.3.0 feature: https://github.com/patternfly/patternfly-react/releases/tag/v5.3.0
|
7 tasks
andrewazores
force-pushed
the
pf5
branch
2 times, most recently
from
53a4c68 to
da18bc0
Compare
* chore(topology): fix broken element colors * chore(topology): fix broken topology toolbar and controlbar * build(deps): bump pf5 deps to latest v5 * fix(topology): display options should show * chore(topology): topology filters should close when clicking outside * fix: node badge should show correct color * chore: correct css var names * fix: truncate long key/value * fix: quicksearch modal should show * fix: mini search menu should show * chore: correct css var to use v5 conventions * fix: port fixes for topology filters * fix: decorators should show * eslint:apply
Signed-off-by: Thuan Vo <thuan.votann@gmail.com>
7 tasks
I think the https://staging-v6.patternfly.org/components/about-modal/react/basic/ |
7 tasks
|
Ah, okay. That PNG was actually made for the upstream https://cryostat.io website and was introduced in this PR. I can ask James if he still has the design file for that and if he's able to provide an SVG version, but considering the design of it with a lot of gradient colours and texture, I don't think it will lend itself well to being vectorized. If we can restyle the modal with some CSS to get the existing PNG to fit again, that would be great. Otherwise we might need to get a whole new SVG asset just for this background, or else simply reuse one of the SVG icons we have. |
|
Actually, given the styling of the PF5 About Modal component, the right move looks like it would be to just use the Cryostat icon SVG in both locations of the modal, except a desaturated/monochrome version and maybe partially cut off or outside of the viewport.
|
Sounds good to me! Though, maybe James or the team can provide one here that matches the requirements above? |
|
I can ask. But, I'm not convinced we really need to have this. The About Modal component says that "the modal image should be the same as the background image you use for your application," but we don't use an application background image. https://www.patternfly.org/components/about-modal/design-guidelines |
Oh, does this mean we can remove the background for the About modal for this upgrade? Btw, I played around with the icon a bit and get this. Just that confirmed Editted: Updated screenshot since last comment.
SVG: cryostat_icon_bg.zip |
The issue seems to be put into 2024 Q3 so hopefully it will be fixed right when we finished upgrading and fixing tests :D |
|
I don't think that is necessarily a blocker anyway. We have other reasons to upgrade to PF5 that are more important, so if some minor styling is missing at first, I say we go ahead regardless and let that get fixed later. |
|
I think the views are pretty much in good shape now (i.e. just need to a quick check on quickstarts)? Here are the list of available tests by directory with assignments:
Just in case anyone is also fixing the tests to void duplicating work :D Editted: All test fixes will now be included in #1347 :D |
|
Thanks for all your hard work on this @tthvo , I really appreciate it. |
This was referenced Sep 8, 2024
* chore(pf5): add missing buildInfo in mocked health response Signed-off-by: Thuan Vo <thuan.votann@gmail.com> * fix(mirageJS): testing target def should not create target * chore: fix type check issues --------- Signed-off-by: Thuan Vo <thuan.votann@gmail.com>
…ete/clear buttons (#1346) Signed-off-by: Thuan Vo <thuan.votann@gmail.com>
Co-authored-by: Max Cao <macao@redhat.com>
|
@tthvo any other planned work for this PR or can I start looking to review and merge this thing? If there are any minor updates (ex. a minor or patch version upgrade for Patternfly) etc. those can be done as follow-ups. |
|
I think this is ready for review, other than the |
|
We do have one issue with credential table though where credential notification returns 0 match. I found this comment from cryostat source: For the time being, is this issue tolerable? Or some workaround desired? |
In API v4 I think this bug is going to go away naturally anyway. |
Very nice! Then, this should be good then :D |
|
Actually, that bug is unfortunately not solved with the V4 work. I also thought it might be with the Quarkus 3.8 upgrade, but it's still present there too - and even with Quarkus 3.8 + API v4. The bug looks something like this in the server logs: In short, something about the Credential/MatchExpression relationship, or the |
7 tasks
Ahh that's unfortunate. For the time being, I have a simple workaround for this issue. Basically just send a follow-up API request to get the updated credential instance: #1350 |
…y on notification (#1350) Signed-off-by: Thuan Vo <thuan.votann@gmail.com>
This was referenced Sep 16, 2024
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Welcome to Cryostat! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/mainFixes: #1110
TODO update from PF 5.1.0 to 5.3.0Notably broken things:
maxMenuHeight- https://www.patternfly.org/components/menus/dropdown#dropdown :