add collapsible card #35
Conversation
size-limit report 📦
|
src/card/Card.tsx
Outdated
| css={cardCSS} | ||
| css={ | ||
| collapsible | ||
| ? collapsibleCardCSS({ cardHeight: cardHeaderHeight }) |
There was a problem hiding this comment.
why does the card header height need to be passed in if it never changes?
There was a problem hiding this comment.
since the header and the card will share the same height in collapsed position, wanted to keep it explicit that they were linked.
If you prefer it not to be passed in, let me know if there's a better strategy for this, using a theme value perhaps?
There was a problem hiding this comment.
I see. So this is actually just the card header height that it shrinks too. In that case I would just remove the parameter and make it a variable in the styles.js - as is it's confusing since it makes it seem as though something is configurable
src/card/Card.tsx
Outdated
| css={cardCSS} | ||
| css={ | ||
| collapsible | ||
| ? collapsibleCardCSS({ cardHeight: cardHeaderHeight }) |
There was a problem hiding this comment.
I see. So this is actually just the card header height that it shrinks too. In that case I would just remove the parameter and make it a variable in the styles.js - as is it's confusing since it makes it seem as though something is configurable
src/card/styles.ts
Outdated
| } | ||
| /* required to override any passed in height values in a closed state */ | ||
| &:not(.is-open) { | ||
| height: ${cardHeight}px !important; |
There was a problem hiding this comment.
| height: ${cardHeight}px !important; | |
| /* shrink the height to the card title so the body is hidden*/ | |
| height: ${cardTitleHeight}px !important; |
src/card/styles.ts
Outdated
| @@ -31,3 +31,41 @@ export const headerCSS = ({ | |||
| height: ${height}px; | |||
| ${bordered ? headerBorderCSS : ''} | |||
| `; | |||
|
|
|||
| export const collapsibleCardCSS = ({ | |||
| cardHeight, | |||
There was a problem hiding this comment.
No need to pass this in since it's not actually configurable.
closes https://github.com/Arize-ai/arize/issues/7048
using
Accordionas a reference https://github.com/Arize-ai/ui-components/blob/3b88026be755c7e4ea6d1485f9a0c1cf68fe86ef/src/accordion/Accordion.tsxwasn't sure how much to split into a new component, similar to Tabbed card, or if we could leverage existing Card
https://user-images.githubusercontent.com/5273390/142048104-37c789ba-f98a-4fce-bb04-7657f4b15f8f.movpending: