Add ListItem component #7844
Add ListItem component #7844
Conversation
olmoh
force-pushed
the
create-listitem-component-des-1895
branch
from
788246d to
d69127e
Compare
| export const ListItemGroup = styled(Flex).attrs({ | ||
| $alignItems: 'center', | ||
| $gap: 'small', | ||
| })``; |
There was a problem hiding this comment.
thought: I think it makes sense to ensure all components have the same public API. What I mean is that when we export the styled component as the component, we actually allow the all the usages which styled component allows, for better or worse.
This might be confusing DX if we allow some ListItem subcomponents to receive for example a forwardedAs prop, but other subcomponents can't.
suggestion: Add a function component which consumes the styled component and export that.
export const ListItemGroup = ({ children }) => {
return <StyledFlex $alignItems="center" $gap="small">{children}</StyledFlex>
}| export const ListItemFooter = styled(Flex).attrs({ | ||
| $padding: { horizontal: 'medium' }, | ||
| })``; |
There was a problem hiding this comment.
thought: I think it makes sense to ensure all components have the same public API. What I mean is that when we export the styled component as the component, we actually allow the all the usages which styled component allows, for better or worse.
This might be confusing DX if we allow some ListItem subcomponents to receive for example a forwardedAs prop, but other subcomponents can't.
suggestion: Add a function component which consumes the styled component and export that.
export const ListItemFooter = ({ children }) => {
return <StyledFlex $padding={{ horizontal: "medium" }}>{children}</StyledFlex>
}| 0: { enabled: 'rgba(41, 77, 115, 1)', disabled: 'rgba(31, 58, 87, 1)' }, | ||
| 1: { enabled: 'rgba(35, 65, 97, 1)', disabled: 'rgba(31, 58, 87, 1)' }, | ||
| 2: { enabled: 'rgba(31, 58, 87, 1)', disabled: 'rgba(28, 52, 78, 1)' }, | ||
| 3: { enabled: 'rgba(28, 52, 78, 1)', disabled: 'rgba(27, 49, 74, 1)' }, | ||
| 4: { enabled: 'rgba(27, 49, 74, 1)', disabled: 'rgba(27, 49, 74, 1)' }, |
There was a problem hiding this comment.
question: From what I can tell, some of these colors not represented in the Colors/ColorTokens enums in foundations.
suggestion:
- Import and use the colors which are available in
foundations. - Add the missing colors to our
foundationsif possible. If it is not appropriate right now to do so, we might want to add a// TODOcomment to replace these values later when we have added the colors.
| &:hover ${StyledFlex} { | ||
| background-color: rgba(56, 86, 116, 1); | ||
| } | ||
| &:active ${StyledFlex} { | ||
| background-color: rgba(62, 95, 129, 1); | ||
| } |
There was a problem hiding this comment.
question: From what I can tell, these colors not represented in the Colors/ColorTokens enums in foundations.
suggestion: Add these colors to our foundations if possible. If it is not appropriate right now to do so, we might want to add a // TODO comment to replace these values later when we have added the colors.
This change is