-
Notifications
You must be signed in to change notification settings - Fork 2
Add Page to view secret details and secret events #2140
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 Page to view secret details and secret events #2140
Conversation
…/weave-gitops-enterprise into 1953-view-secret-details
…/weave-gitops-enterprise into 1953-view-secret-details
…/weave-gitops-enterprise into 1953-view-secret-details
…/weave-gitops-enterprise into 1953-view-secret-details
…/weave-gitops-enterprise into 1953-view-secret-details
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.
Good job 👏
const ListEvents = (props: Props) => { | ||
const { error, data, isLoading } = useListEvents(props); | ||
return ( | ||
<> |
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.
What do you think if we can make use of WorkspaceTabsWrapper
that we did in workspaces to use it here as well & maybe we can change the name to LoadingWrapper
as it'll save us some duplicate code 🤔 ?
<WorkspaceTabsWrapper loading={isLoading} errorMessage={error?.message}>
<TableWrapper id="...">
<EventsTable events={data.events} />
...
width: 100%; | ||
`; | ||
const DetailsHeadersWrapper = styled.div` | ||
div { |
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.
Is it nested div > div
to use it this way ?. If no, we can remove the div {
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.
yes i want to apply the style for the nested div div > div
]; | ||
|
||
return ( | ||
<div> |
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.
dev
not needed here as CustomSubRouterTabs
wrap what we need to return
return ( | ||
<div> | ||
<CustomSubRouterTabs rootPath={`${path}/secretDetails`}> | ||
<RouterTab name="Details" path={`${path}/secretDetails`}> |
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.
Can we change secretDetails
to just details, or it may breaks something ?
}, | ||
]; | ||
|
||
console.log(secretDetails); |
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.
console.log
😉
|
||
console.log(secretDetails); | ||
return ( | ||
<> |
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.
<>
not needed here as well
ui-cra/src/utils/nav.ts
Outdated
|
||
Secrets= '/secrets', | ||
Secrets = '/secrets', | ||
SecretDetails = '/details' |
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.
/details
can we change it /secrets/details
or what do you think ?
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,
can we rename WorkspaceTabsWrapper
to LoadingWrapper
?
Closes #1935 #1936
What changed?
Why was this change made?
to view more details about the secret and view the secret events list
How was this change implemented?
How did you validate the change?
Release notes
Documentation Changes