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
Expose list, table, ResourceLink componens and useK8sModel(s) hooks via plugin SDK #9925
Expose list, table, ResourceLink componens and useK8sModel(s) hooks via plugin SDK #9925
Conversation
dce71fd
to
171836e
Compare
95fd23a
to
9361858
Compare
9361858
to
c72ba1c
Compare
c72ba1c
to
dbb61a7
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.
Also a small nit: in frontend/public/*
sources, I'd suggest putting @console/dynamic-plugin-sdk
imports above the relative path imports (seems like the import sort ESLint rule is not applied to @console/internal
package sources).
@@ -40,7 +24,6 @@ const ListPageFilter: React.FC<ListPageFilterProps> = ({ | |||
nameFilterPlaceholder={nameFilterPlaceholder} | |||
labelFilterPlaceholder={labelFilterPlaceholder} | |||
onFilterChange={onFilterChange} | |||
textFilter={textFilter} |
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.
Don't we need this prop passthrough anymore?
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.
maybe in the future, but in other form.
textFilter
is a string name of some function in https://github.com/openshift/console/blob/master/frontend/public/components/factory/table-filters.ts#L39
which is very dangerous.
If in the future we will need to provide custom implementation of text filter a consumer will be able to pass a new optional property as a function, not string. The optional property wont break the API
So we are good for now.
/label px-approved There are no user visible changes, and we have approval for dynamic plugin APIs. |
/retest |
dbb61a7
to
812c59e
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rawagner, vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.