Closed TejasQ closed 6 years ago
Shouldn't we separate columns from content headers (you called them CardTitle)? You could have multiple CardTitles and content in one column.
How about <CardColumn>
purely for arranging content?
@micha-f The <CardColumn title="Tags"></CardColumn>
is just a shortcut for:
<CardColumn>
<CardTitle>Tags</CardTitle>
{/* content */}
</CardColumn>
The idea is to provide an easy way for simple cases, but of course, if we have more than one title, it make sense to not use this shortcut.
@fabien0102 that's nice!
What @fabien0102 said. Syntax sugar is something that is loved and so we think API sugar could also be a nice thing to give our users.
Why do the actions need to be functions? Is this a convention I'm not aware of?
Also, I'm assuming we might want to allow some card content to be rendered outside of the card columns, right? I'm thinking it makes sense to have a CardColumns container component that groups CardColumn components.
I would say that a function provides more flexibility and can return different values when it's re-evaluated - for example if it closes over some variable which it uses during evaluation.
It's not really a function in this context (it's a function but not only) it's a React.ComponentType
.
The idea is to permit to easily extract any sub-component outside and give more flexibility ;)
So with the same example, we can have this kind of implementations:
import CardAction from "./CardAction" // this export a props=>JSX.element or a classic React.Component
<Card
title="Bundle Information"
action={CardAction}
>
<CardTitle>Contributors</CardTitle>
<AvatarGroup collaborators={[]} />
<CardTitle>Members</CardTitle>
<AvatarGroup members={[]} />
</Card>
And can also give any CardAction
props inside the Card
component.
If we do this (with the jsx notation):
<Card action={<CardAction />} />
You can't give any props to CardAction
into Card
😉
The idea behind having actions as React components (which can be called functions), is that each component can have its own onClick
handler to do whatever it wants to, or just be something purely visual, like descriptive, helpful text.
Consider,
const MyReactComponentAction = () => <div onClick={() => alert('eat KFC!')}>Give me bad advice!</div>
/* later, when using this component, */
<Card
title="Bundle Information"
action={MyReactComponentAction}
{/* could also be written like below */}
action={() => <MyReactComponentAction />}
>
<CardTitle>Contributors</CardTitle>
<AvatarGroup collaborators={[]} />
<CardTitle>Members</CardTitle>
<AvatarGroup members={[]} />
</Card>
@ImogenF your question is a really good one! Should we call them actions
when they're components? Seems a little ambiguous because one would expect actions
to almost be a dictionary of actions. Maybe we can call them something else? Should we?
About columns, I think you're absolutely right. What do you think about this example illustrating a card that has columns and stuff outside the columns?
<Card
title="Bundle Information"
action={MyAction}
>
Hello, I live outside the columns and I'm a free man/woman/child/lgbtq/gender neutral construct!
<CardColumns>
<CardColumn title="Contributors">
<AvatarGroup collaborators={[]} />
</CardColumn>
<CardColumn title="Tags">
<Chip>agent-view</Chip>
<Chip>agent-view</Chip>
</CardColumn>
</CardColumns>
</Card>
@fabien0102 what do you think?
@TejasQ how about keeping it simple and going with actionComponents
?
And the CardColumns structure is exactly what I've been playing around with.
Great idea about actionComponents
, @ImogenF! But what about the (I expect common) case that you have just one action component?
@TejasQ I put this in and then took it out again after a discussion with @fabien0102.
Summary:
Components
suffix is inconsistent with how other components are namedSo it's staying as action
until/unless we can figure out something better 😉
Makes sense. We could even describe it by saying action
is the name of an area in a CardHeader that is the upper-right side.
To fit the new design, we'll need to update
Card
s to look like the above figure.Proposed API
Card Layout with Columns
Card Layout without Columns