feat: add 'impending deletion' badges to workspaces page #7530
Conversation
| className?: string | ||
| } | ||
|
|
||
| const ImpendingDeletionBadge: FC< | ||
| PropsWithChildren<Partial<WorkspaceStatusBadgeProps>> |
There was a problem hiding this comment.
We are not using any children prop here so we can remove PropsWithChildren
|
|
||
| if (!allowAdvancedScheduling || !allowWorkspaceActions) { | ||
| return null | ||
| } |
There was a problem hiding this comment.
Since we are already checking if we should or not display the badge on displayImpendingDeletion I think we should move those checks to be there since they are doing the same thing.
There was a problem hiding this comment.
@BrunoQuaresma those checks rely on a hook, which has to be called in a component. Would you rather I call the hook in the WorkspaceStatusBadge component? I definitely could; I just didn't want to screw up all the storybook stories.
Alternatively, I thought about calling this hook in WorkspacesPage and then passing down props from there - but there are like 7 layers of prop passing and it just seemed like a lot to me.
There was a problem hiding this comment.
The thing for me is to have the "should display or not" check in one single place. Splitting this check into different places looks confusing to me. But if it is hard, I would add a comment so that in the future, if we want to change the display behavior, we know there are two places to look for.
There was a problem hiding this comment.
Agreed - as the ideal behavior is to have the check in one place, I think the best path forward may be to call the hook in the badge component and then ignore Storybook's complaints, rather than let our testing tool dictate how we code.
I'll adjust.
Definitely! This is part of a much larger design. I think the explanation will be handled in this ticket: #7365 |
resolves #7349