Adding inner margins and padding to one component should be avoided. In one screen it may make sense to do this, but as soon as other screen doesn't require that margin, the component becomes incorrect. Do not create new components with inner margins. There are a few ways to undo this. Let's first create an incorrect example:
❌ Wrong
type ProfileCardItemProps = {
label: string
value: string
}
function ProfileCardItem({ label, value }: ProfileCardItemProps) {
return (
<GridItem marginTop={3}>
<Text
wordBreak="break-word"
fontWeight={500}
fontSize={{ base: 'sm', lg: 'md' }}
color="gray.800"
>
{label}
</Text>
<Text wordBreak="break-word" fontWeight={600} fontSize="xl" color="gray.900">
{value}
</Text>
</GridItem>
)
}
This may seem fine at first. This could be adding some margin between this component and another section, or even adding margin between items in a list. It accomplishes its goal. But what happens if I try to add this to another screen? The margin will still be there. Someone may be inclined to fix this issue by doing this:
❌ Wrong
type ProfileCardItemProps = {
label: string
value: string
marginTop?: string | number
}
function ProfileCardItem({ label, value, marginTop = 3 }: ProfileCardItemProps) {
return (
<GridItem marginTop={marginTop}>
<Text
wordBreak="break-word"
fontWeight={500}
fontSize={{ base: 'sm', lg: 'md' }}
color="gray.800"
>
{label}
</Text>
<Text wordBreak="break-word" fontWeight={600} fontSize="xl" color="gray.900">
{value}
</Text>
</GridItem>
)
}
This code does not scale. Try to imagine a few months of development like this. Picture how this component will look like if every prop that this component needs requires another property being added to ProfileCardItemProps
. It will eventually be HUGE, right? So what can we do instead?
Components should be responsible for what they do and what they are. A <TextInput>
component shouldn't have props that manipulate the wrapper container it has. The general rule for a component should be to occupy as much space as it can, and be flexible in size for other screens. If you think it makes sense for the component to have a wrapper that manipulate something around the component, instead of changing how you write this component, you will change how you use it.
✅ Correct
export default function Page() {
return (
<Box marginTop={3}>
<ProfileCardItem label="Hello" value="World">
</Box>
)
}
function ProfileCardItem({ label, value }: ProfileCardItemProps) {
return (
<Flex direction="column">
<Text
wordBreak="break-word"
fontWeight={500}
fontSize={{ base: 'sm', lg: 'md' }}
color="gray.800"
>
{label}
</Text>
<Text wordBreak="break-word" fontWeight={600} fontSize="xl" color="gray.900">
{value}
</Text>
</Flex>
)
}
Making it as "dumb" as you can will allow it to be as flexible as it could.
This post was written with the help of @santospatrick and @brunoeduardodev.
✅ LGTM