-
Notifications
You must be signed in to change notification settings - Fork 22
NETOBSERV-1071: Custom icons #1189
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
base: main
Are you sure you want to change the base?
Conversation
|
@jpinsonneau: This pull request references NETOBSERV-1071 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@jpinsonneau: This pull request references NETOBSERV-1071 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@jpinsonneau: This pull request references NETOBSERV-1071 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
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=a08ad5d make set-plugin-image |
leandroberetta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpinsonneau thanks for the PR, looks good to me although I have some small comments to discuss.
| render() { | ||
| const { size, className, style } = this.props; | ||
| const sizeValue = typeof size === 'number' ? size : parseFloat(size as string) || 18; | ||
| return <IconWrapper icon={GrGateway} size={sizeValue} className={className} style={style} />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gateway icon changed, I think I like the previous one, is it possible to use the patternfly one?. Also, it seems to have a black background:
Previous one was GlobeRouteIcon:
Also, is that the icon for deployments or should be different? I think we can look for a similar one to the existing today? What do you thinnk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1189 +/- ##
==========================================
- Coverage 52.95% 52.48% -0.48%
==========================================
Files 209 214 +5
Lines 10950 11154 +204
Branches 1391 1413 +22
==========================================
+ Hits 5799 5854 +55
- Misses 4602 4749 +147
- Partials 549 551 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
leandroberetta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
/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=d3f82ca make set-plugin-image |
Sure, let me fix that
It could be anything we want. I just took it from the svgs in https://github.com/kubernetes/community/tree/master/icons/svg/resources/unlabeled
Yeah I'm not sure why I didn't found those implemented already 🤔 Thanks for your feedback ! |
|
New changes are detected. LGTM label has been removed. |
|
Added the missing icons and fixed alignment: 3fc9829 |
|
Added the missing icons and fixed alignment: 0881543
Also updated the mocks so everybody can have a quick look 😸 |
|
/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=8741167 make set-plugin-image |
|
Alignment looks good now. VM's and network have icons too now. |
yeah let's grab more feedback and merge when we agree. Thanks @Amoghrd ! |




















Description
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.