Feature/add button full width prop #129
Conversation
| @@ -118,6 +118,7 @@ export const baseButtonStyles = ({ | |||
| buttonType, | |||
| isLoading, | |||
There was a problem hiding this comment.
Is it possible to move the loading spinner into the same area as the text here? It currently pushes way out to the side.
|
@mjaltamirano as far as the height thing goes, it's definitely weird. Would removing the fontSize property on the full width variant change anything? |
We went over the current Loader icon spacing fix in person, which looked good to you, but for this I actually stumbled upon what appeared to be a decent fix while troubleshooting how to get the Loader spacing improved. You can see that in the code here: https://github.com/PocketDerm/radiance-ui/pull/129/files#diff-e54f785f0d62d47619cf001bc205d9fbR171 |
| @@ -35,7 +35,7 @@ const ButtonLoader = styled.div` | |||
| right: 0; | |||
| top: 0; | |||
| margin-top: -6px; | |||
| width: 38px; | |||
| width: ${({ isFullWidth }) => (isFullWidth ? `25%` : `38px`)}; | |||
There was a problem hiding this comment.
25% was fairly arbitrary but was relatively close for very wide buttons (which isn't a pattern we want to use, since we're adding this for mobile), but also close enough with narrower viewports (320-400px).
This PR adds a
fullWidthprop to Button components to make them expand to take the width of their container.There is a noticeable issue where, because the Icon does not display when the component is loading, there is an empty svg present with spatial properties that inadvertently messes up the height of the button. You can see it in the lack of perfect alignment of the full-width buttons in this screenshot:
Setting the
display: noneon the svg fixes the alignment, but we currently rely on it for positioning the text and loading animation. Similarly, if you turn offfont-size: 0.75remon both side-by-side button components, they are again aligned.I'm not concerned with it from a cosmetic standpoint in the storybook but it is something that could possibly cause an issue in use.