NETOBSERV-1924 pf5 migration #658
Conversation
|
Skipping CI for Draft Pull Request. |
jpinsonneau
force-pushed
the
pf5-update
branch
6 times, most recently
from
233f64e to
efbdbf2
Compare
jpinsonneau
force-pushed
the
pf5-update
branch
from
efbdbf2 to
9bea91d
Compare
jpinsonneau
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=7601fa5 make set-plugin-image |
| "webpack-cli": "^4.10.0", | ||
| "webpack-dev-server": "^4.6.0", | ||
| "webpack-node-externals": "^3.0.0" | ||
| "webpack": "5.75.0", |
There was a problem hiding this comment.
that's a downgrade, is it necessary?
There was a problem hiding this comment.
I can double check on that one but it aligns with OCP console version + sdk one
| "react-router": "^5.3.0", | ||
| "react-router-dom": "^5.3.0" |
There was a problem hiding this comment.
(we must make sure it won't introduce security regressions)
There was a problem hiding this comment.
Yes. As written in the description, dynamic-plugin-sdk-webpack is now doing checks on our dependencies and forces compatible versions with OCP Console ones.
web/src/app.tsx
Outdated
| <ToggleGroup> | ||
| <ToggleGroupItem | ||
| icon={ | ||
| <svg className="pf-v5-svg" viewBox="0 0 512 512"> |
There was a problem hiding this comment.
Is it netobserv icon? Where is it displayed, I don't see it anywhere
Or that's just for the standalone console
There was a problem hiding this comment.
No this is the Light / Dark theme switch. The icons are not available in PF Icons.
I can simply show text instead as:
<ToggleGroup>
<ToggleGroupItem
text="Light"
isSelected={!isDark}
onClick={() => onThemeSelect(false)}
/>
<ToggleGroupItem
text="Dark"
isSelected={isDark}
onClick={() => onThemeSelect(true)}
/>
</ToggleGroup>
|
I see there's no more little icons/decorators on graph nodes |
| /> | ||
| )} | ||
| > | ||
| {sizePx > 450 ? sd.name : sd.shortName} |
There was a problem hiding this comment.
I don't think this shortName is still needed ? Since it's now written horizontally
There was a problem hiding this comment.
sure, I can remove that part
There was a problem hiding this comment.
We will be able to remove the shortname from the config on controller side too
|
Finished the review and only did a quick testing: overall lgtm but I'll do a couple more tests before validating. Anyway since it's a big PR we don't want to wait too much for it, so if there's some little things to polish we can do that in follow-ups. BTW we also need to create a pf4 branch that won't receive this PR. Thanks for this big upgrade @jpinsonneau ! |
github-actions
bot
removed
the
ok-to-test
Good catch @jotak ! I missed that. The tooltips were messing up with the rendering so I separated these two using a ref. I took the opportunity to only show the decorators when hovered or selected. It simplify a lot the view. Also fixed the CSS ! |
jotak
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=1376325 make set-plugin-image |
|
Thanks @jpinsonneau |
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
github-actions
bot
removed
the
ok-to-test
|
Removed hardcoded styling import as it's supposed to be fixed in OCP 4.15.42 |
jpinsonneau
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=5c05e45 make set-plugin-image |
Well it's not !
We need to retry this on future releases... |
|
@jpinsonneau: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
Update to PF5 using dynamic sdk 1.0.0 and PF 5.4.0:
onChangearguments been inverted 🙃dynamic-plugin-sdk-webpackis now checking thisWe will probably have to maintain 2 plugins versions to support OCP 4.15+ and 4.12 -> 4.14.
@openshift-console/dynamic-plugin-sdk@openshift-console/dynamic-plugin-sdk-webpack@openshift-console/dynamic-plugin-sdk@openshift-console/dynamic-plugin-sdk-webpack@openshift-console/dynamic-plugin-sdk@openshift-console/dynamic-plugin-sdk-webpack@openshift-console/dynamic-plugin-sdk@openshift-console/dynamic-plugin-sdk-webpack@openshift-console/dynamic-plugin-sdk@openshift-console/dynamic-plugin-sdk-webpack@openshift-console/dynamic-plugin-sdk@openshift-console/dynamic-plugin-sdk-webpackhttps://github.com/openshift/console/tree/master/frontend/packages/console-dynamic-plugin-sdk#openshift-console-versions-vs-sdk-versions
Previous attempt: #453
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.