-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat(ui): Migrate project summary settings to EditablePanel for parity with rest of UI #4400
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4400 +/- ##
==========================================
+ Coverage 41.18% 41.29% +0.10%
==========================================
Files 122 122
Lines 16584 16584
==========================================
+ Hits 6830 6848 +18
+ Misses 8772 8750 -22
- Partials 982 986 +4
Continue to review full report at Codecov.
|
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.
Thanks a lot for working on it @rbreeze !
Added few comments. Also, the orphaned resources panel seems to need a little more polishing. Worked on small PR that implements some ideas: rbreeze#1
- add icons that visualize enabled/disabled state
- add help icons with the additional description in tooltip
- removed redundant 'Orphaned' words from labels within 'Orphaned Resources' panel
- make sure that positions of edit and view controller are consistent
ui/src/app/settings/components/project-details/project-details.tsx
Outdated
Show resolved
Hide resolved
ui/src/app/settings/components/project-details/project-details.tsx
Outdated
Show resolved
Hide resolved
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.
Last couple comments. LGTM after addressing them.
ui/src/app/settings/components/project-details/project-details.tsx
Outdated
Show resolved
Hide resolved
ui/src/app/settings/components/project-details/project-details.tsx
Outdated
Show resolved
Hide resolved
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. Thank you @rbreeze
<EditablePanel | ||
save={item => this.saveProject(item)} | ||
values={proj} | ||
title={<React.Fragment>CLUSTER RESOURCE DENY LIST {helpTip('Cluster-scoped K8s API Groups and Kinds which are not permitted to be deployed')}</React.Fragment>} |
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.
It feels like this change from blacklist
/whitelist
to deny list
/allow list
should have been called out in a commit message / release note...
Checklist: