-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add Plugin Settings Detail View and Plugin Config Store #1678
Conversation
c9265e5
to
ef90abd
Compare
ef90abd
to
067fa3c
Compare
frontend/src/components/App/PluginSettings/pluginSettingsDetail.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/App/PluginSettings/pluginSettingsDetail.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/App/PluginSettings/pluginSettingsDetail.tsx
Outdated
Show resolved
Hide resolved
067fa3c
to
68f3fb7
Compare
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.
Nice work @yolossn, added some suggestions.
frontend/src/components/App/PluginSettings/pluginSettingsDetail.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/App/PluginSettings/pluginSettingsDetail.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.
Added a few notes, will look more later. DCO and conflicts.
fda3ee5
to
0d2d8ea
Compare
167bb59
to
8a71f4b
Compare
8a71f4b
to
80357b9
Compare
Backend Code coverage changed from 55.5% to 56.1%. Change: .6% 😃. |
6fb6fea
to
7d92c68
Compare
Backend Code coverage changed from 55.5% to 56.1%. Change: .6% 😃. |
7d92c68
to
3c4ca51
Compare
Backend Code coverage changed from 55.5% to 56.2%. Change: .7% 😃. |
frontend/src/components/App/PluginSettings/PluginSettingsDetails.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/App/PluginSettings/PluginSettingsDetails.tsx
Outdated
Show resolved
Hide resolved
this patch adds plugin config store that is responsible for handling the storage and retrieval of plugin configs Signed-off-by: yolossn <[email protected]>
this patch adds deletePlugin function to apiProxy, it calls the delete plugin endpoint with all necessary headers to handle auth. Signed-off-by: yolossn <[email protected]>
this patch updates the plugin settings view, filter is added to the table, Origin now can be configured to link to a custom URL and enable buttons UI is improved. Signed-off-by: yolossn <[email protected]>
this commit adds PluginDetails page and storybook for it. Signed-off-by: yolossn <[email protected]>
this patch adds plugin settings component which can be used by the user to configure custom logo by providing the URL. Signed-off-by: yolossn <[email protected]>
this patch registers a Settings Detail Component which allows the user to set the error message to be displayed when the pods count cannot be retrieved. Signed-off-by: yolossn <[email protected]>
this patch adds docs for registerPluginSettings and its related types. Signed-off-by: yolossn <[email protected]>
this patch makes the plugin settings available in all modes, earlier it wasn't available in in-cluster mode. Signed-off-by: yolossn <[email protected]>
3c4ca51
to
848a90c
Compare
Backend Code coverage changed from 55.4% to 56.0%. Change: .6% 😃. |
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.
I think this LGTM now.
BTW, I thought the "now enabled" routes were initially added here and that was a mistake as you mentioned @yolossn . But making the routes available does mean users will see the "disable" button, and I think disabling plugins shouldn't work unless in the app mode.
This can be added in a second PR, but maybe it's important to have it for the release?
WDYT @illume ?
A check is added to make sure the Delete plugin is only visible in the app mode. So the button wont be visible to the user. And similarly check is added in the backend to make sure the delete API route isn't available in the in-cluster mode. |
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.
🎉🎈
This PR adds Plugin Settings Details View and Plugin Config Store that can be used by plugin developers to store and retrieve plugin configurations.
To Do:
Demo:
https://github.com/headlamp-k8s/headlamp/assets/22731013/8a43a75c-014c-4884-b641-ca9d5396b3be
How to test:
change-logo
andpod-counter
plugins.