feat(aci): grid cell components for monitors #84281
Conversation
github-actions
bot
added
the
Scope: Frontend
| ${Wrapper} { | ||
| color: ${p => (p.disabled ? p.theme.disabled : p.theme.textColor)}; | ||
| font-size: ${p => p.theme.fontSizeMedium}; | ||
| } |
There was a problem hiding this comment.
Selecting nested components like this is usually a sign something about the component needs to be more flexible than it is...
color and font-size are both inheritable, so we should be able to set these on the IssueWrapper root itself and nested components should respect it. Maybe we modify the ShortId component so that the font-size is inheritable and move the current font-size declaration to its wrapper?
| display: flex; | ||
| flex-direction: row; | ||
| gap: ${space(0.5)}; | ||
| color: ${p => p.theme.subText}; |
There was a problem hiding this comment.
Would prefer using <Flex gap={space(0.5)} /> for this
| const text = (type: 'monitor' | 'automation', length: number) => { | ||
| if (type === 'monitor') { | ||
| return tn('%s monitor', '%s monitors', length); | ||
| } | ||
| return tn('%s automation', '%s automations', length); | ||
| }; |
There was a problem hiding this comment.
Instead of a type prop that is limited to monitor | automation, what if this component accepted a renderSuffix function? That would be slightly more composable and avoid hardcoding this logic inline.
TitleCellrefactored + renamed
AutomationTitleCellso that it can be used for monitors and automationsConnectionCellrefactored + renamed
MonitorsCellso that it can be used for connected monitors and automationsIssueCellScreen.Recording.2025-01-29.at.3.15.26.PM.mov
NumberCell