Allow extensions to register kube-object menus + details #1108
Conversation
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
jakolehm
added
enhancement
|
|
||
| export interface KubeObjectMenuRegistration { | ||
| kind: string; | ||
| apiVersions: string[]; |
There was a problem hiding this comment.
This allows extensions to have different components for different apiVersions (for example v1alpha1 vs v1). It also allows extension to register multiple versions for a component.
| @@ -1,4 +0,0 @@ | |||
| export * from "./certificates" | |||
There was a problem hiding this comment.
Removed everything related to certmanager because it's using old apiversion and this actually should be an extension.
|
I pulled the PR and did After running |
| "devDependencies": { | ||
| "ts-loader": "^8.0.4", | ||
| "typescript": "^4.0.3", | ||
| "webpack": "^4.44.2", | ||
| "mobx": "^5.15.5", | ||
| "react": "^16.13.1" | ||
| } |
There was a problem hiding this comment.
this (and maybe other things) will be necessary for all extensions, right? Are we going to template it or something?
There was a problem hiding this comment.
Yes, we should probably mark these as dependencies of @k8slens/extensions package (topic for another PR).
| @@ -1,2 +1,4 @@ | |||
| export type { DynamicPageType, PageRegistry } from "../page-registry" | |||
| export type { AppPreferenceRegistry } from "../app-preference-registry" | |||
| export type { KubeObjectMenuRegistry } from "../../renderer/api/kube-object-menu-registry" | |||
| export type { KubeObjectDetailRegistry } from "../../renderer/api/kube-object-details-registry" | |||
There was a problem hiding this comment.
naming should be consistent (KubeObjectDetailsRegistry?) (details <=> Detail)
| registerKubeObjectMenus(registry: KubeObjectMenuRegistry) { | ||
| return | ||
| } |
There was a problem hiding this comment.
I don't understand why these methods have to be defined by the subclasses. Can't it be something like
registerKubeObjectMenus(kubeObjectMenus: KubeObjectMenuRegistration[]) {
for (menu of kubeObjectMenus) {
this.disposers.push(this.registry.add(menu))
}
}
(with registry being a class member)? I'm probably missing something... tsx versus ts?
There was a problem hiding this comment.
This is actually a very good point. I also hate the fact that disposers are now exposed and "mandatory" to use within extensions. We need to discuss about pros/cons (this is out of scope of this PR).
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
| import { CrdResourceDetails, CrdResourceMenu } from "../+custom-resources"; | ||
| import { CrdResourceDetails } from "../+custom-resources"; | ||
| import { KubeObjectMenu } from "./kube-object-menu" | ||
| import { kubeObjectDetailRegistry } from "../../api/kube-object-detail-registry"; | ||
|
|
||
| export interface KubeObjectDetailsProps<T = KubeObject> { |
There was a problem hiding this comment.
Should this be KubeObjectDetailProps? Maybe 'details' is appropriate in some places, 'detail' in other?
| apiVersions: ["v1"], | ||
| components: { | ||
| Details: (props) => <ConfigMapDetails {...props} /> |
There was a problem hiding this comment.
nit: some places (props: any) is used
This is a quite big refactoring. Basically this PR replaces old
apiManager.set/getViewswith a registry pattern that is used in extension-api. PR also moves node/pod menus to extensions so that we actually eat our own dogfood.