-
Notifications
You must be signed in to change notification settings - Fork 887
feat: add frontend for locked workspaces #8655
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
Conversation
- Fix workspaces query for locked workspaces.
@@ -25,8 +25,8 @@ export const WorkspaceStatusBadge: FC< | |||
return ( | |||
<ChooseOne> | |||
{/* <ImpendingDeletionBadge/> determines its own visibility */} | |||
<Cond condition={Boolean(ImpendingDeletionBadge({ workspace }))}> | |||
<ImpendingDeletionBadge workspace={workspace} /> | |||
<Cond condition={Boolean(LockedBadge({ workspace }))}> |
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.
You probably don't need this since the LockedBadge
is already doing the check and returning null
if it should not be displayed.
setLockedWorkspaces([]) | ||
} | ||
}, [experimentEnabled, data, filterProps.filter.query]) | ||
|
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 logic feels very similar to the useWorkspacesToBeLocked
, why not use it here as well?
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.
The useWorkspacesToBeLocked
takes a template which the workspaces page doesn't mandate. I could refactor it, but I'd rather merge this as-is and go back and clean up things as we move it out of experimental.
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.
Just minor comments but overall, it looks good to go 👍
/deploy-pr |
🚀 Deploying PR 8655 ... |
No description provided.