You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We have a number of component props that differ between their usage and aren't consistent, examples below:
Refs
There's a mix of [componentName]Ref and ref. Generally speaking, every component should be using React.forwardRef and standardize on allowing ref. This is already part of of our component guidelines so new components should already be adhering to this:
Primitive/Pattern components should always use React.forwardRef to provide direct access to the component's primary element
However, we need to do the work to remove existing props (likely as a future breaking change) to standardize on ref.
Sizing
There's a mix of "sizing" props to allow for different "sizes" of components. This differs from variant as variant tends to be purely aesthetic. Some examples:
<Button thin>
<IconButton large>
<Link thin>
<Pagination thin>
<Tabs thin>
<Tooltip variant="big">
For the most part we've been using thin to indicate a more lightweight variant, but that hasn't always worked where the smaller version was already the (e.g. IconButton). The current methodology of using thin may be limiting in the future if we need to add additional sizes later considering we're treating these sizing props as boolean values. We would not want to allow <Button thin large> as that would not be a valid combination of props.
Instead, we should migrate over to using size to allow for greater flexibility in the future. This would allow us to better constrain our types as size="small" size="large" would be much easier to handle vs <Button thin large>.
Shared Component Props
When a component is "composed", we should have a scalable way of specifying props on the composed component. Some examples:
<IconButton tooltipPlacement={...}>
<Pagination tooltipPlacement={...}>
<TextEllipsis tooltipProps={...}>
The primary issue with the specifying individual custom props is if additional props are added to the underlying component or if certain props aren't included, it makes the composed component less flexible. In some cases this may be the desired behavior, but a nested component that receives props via composition should generally have a single prop on the composed component ideally with the pattern [componentName]Props.
The text was updated successfully, but these errors were encountered:
We have a number of component props that differ between their usage and aren't consistent, examples below:
Refs
There's a mix of
[componentName]Ref
andref
. Generally speaking, every component should be usingReact.forwardRef
and standardize on allowingref
. This is already part of of our component guidelines so new components should already be adhering to this:However, we need to do the work to remove existing props (likely as a future breaking change) to standardize on
ref
.Sizing
There's a mix of "sizing" props to allow for different "sizes" of components. This differs from
variant
as variant tends to be purely aesthetic. Some examples:<Button thin>
<IconButton large>
<Link thin>
<Pagination thin>
<Tabs thin>
<Tooltip variant="big">
For the most part we've been using
thin
to indicate a more lightweight variant, but that hasn't always worked where the smaller version was already the (e.g.IconButton
). The current methodology of usingthin
may be limiting in the future if we need to add additional sizes later considering we're treating these sizing props as boolean values. We would not want to allow<Button thin large>
as that would not be a valid combination of props.Instead, we should migrate over to using
size
to allow for greater flexibility in the future. This would allow us to better constrain our types assize="small" size="large"
would be much easier to handle vs<Button thin large>
.Shared Component Props
When a component is "composed", we should have a scalable way of specifying props on the composed component. Some examples:
<IconButton tooltipPlacement={...}>
<Pagination tooltipPlacement={...}>
<TextEllipsis tooltipProps={...}>
The primary issue with the specifying individual custom props is if additional props are added to the underlying component or if certain props aren't included, it makes the composed component less flexible. In some cases this may be the desired behavior, but a nested component that receives props via composition should generally have a single prop on the composed component ideally with the pattern
[componentName]Props
.The text was updated successfully, but these errors were encountered: