-
Notifications
You must be signed in to change notification settings - Fork 1.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
Table component injection tokens #7754
Changes from all commits
11d708d
3362b3d
29a5292
7fead2b
dca082c
0af8846
3162f5e
fc9a925
ad1e095
6b7812d
11a35a7
eb92b08
a924628
54366c3
6e5fc63
febae16
12efdb1
27de0e7
0066ef7
237c470
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/** | ||
* Copyright (c) OpenLens Authors. All rights reserved. | ||
* Licensed under MIT License. See LICENSE in root directory for more information. | ||
*/ | ||
import { tableComponentInjectionToken } from "@k8slens/table-tokens"; | ||
import { getInjectable } from "@ogre-tools/injectable"; | ||
import { Table } from "../../renderer/components/table/table"; | ||
|
||
const tableComponentInjectable = getInjectable({ | ||
id: "table-component", | ||
instantiate: () => ({ Component: Table }), | ||
injectionToken: tableComponentInjectionToken, | ||
}); | ||
|
||
export default tableComponentInjectable; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/** | ||
* Copyright (c) OpenLens Authors. All rights reserved. | ||
* Licensed under MIT License. See LICENSE in root directory for more information. | ||
*/ | ||
import { getFeature } from "@k8slens/feature-core"; | ||
import tableComponentInjectable from "./table-component.injectable"; | ||
|
||
export const tableFeature = getFeature({ | ||
id: "core-table-feature", | ||
|
||
register: (di) => { | ||
di.register(tableComponentInjectable); | ||
}, | ||
}); | ||
Comment on lines
+8
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you remind me (mostly for learning purpose) why do we need to make it as a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my understanding, it seems to be an architectural decision. If we utilize an injection token in the core and there are two "users" of this token registered, an error will occur. Only one injection token consumer is allowed. However, by incorporating a feature within the open-lens package, we can easily separate features to determine their visibility in all lens variations. Additionally, the feature is beneficial when dealing with multiple injection tokens that need to be registered. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jansav @Iku-turso A good chance to clarify the question and correct me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can't get it.. why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
-- This case seems to have been built in a way that we expect a single implementation of |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# Description | ||
|
||
The package exports tokens needed for external table configuration. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { getInjectionToken } from "@ogre-tools/injectable"; | ||
import type { KubeObject } from "@k8slens/kube-object/src/kube-object"; | ||
import type { | ||
BaseKubeObjectListLayoutColumn, | ||
GeneralKubeObjectListLayoutColumn, | ||
SpecificKubeListLayoutColumn, | ||
} from "@k8slens/list-layout"; | ||
|
||
type Column = ( | ||
| BaseKubeObjectListLayoutColumn<KubeObject> | ||
| SpecificKubeListLayoutColumn<KubeObject> | ||
| GeneralKubeObjectListLayoutColumn | ||
); | ||
|
||
export interface TableComponentProps { | ||
tableId?: string; | ||
columns?: Column[]; | ||
save?: (state: object) => void; | ||
load?: (tableId: string) => object; | ||
} | ||
|
||
export interface TableComponent { | ||
Component: React.ComponentType<TableComponentProps>; | ||
} | ||
|
||
export const tableComponentInjectionToken = getInjectionToken<TableComponent>({ | ||
id: "table-component-injection-token", | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{ | ||
"name": "@k8slens/table-tokens", | ||
"version": "6.5.0-alpha.7", | ||
"description": "Injection token exporter for table components", | ||
"license": "MIT", | ||
"type": "commonjs", | ||
"private": false, | ||
"publishConfig": { | ||
"access": "public", | ||
"registry": "https://registry.npmjs.org/" | ||
}, | ||
"main": "./dist/index.js", | ||
"types": "./dist/index.d.ts", | ||
"files": [ | ||
"dist" | ||
], | ||
"scripts": { | ||
"clean": "rimraf dist/", | ||
"build": "lens-webpack-build" | ||
}, | ||
"devDependencies": { | ||
"@k8slens/webpack": "^6.5.0-alpha.8", | ||
"rimraf": "^4.4.1" | ||
}, | ||
"peerDependencies": { | ||
"@ogre-tools/injectable": "^16.1.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"extends": "@k8slens/typescript/config/base.json", | ||
"include": ["**/*.ts"] | ||
} |
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.
Move this under
open-lens
package and importTable
from@k8slens/core
. Add export if it's not exported yet.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.
@aleksfront