From 0e414cd5ee0757cc8df24144f9bed6a5853f514a Mon Sep 17 00:00:00 2001 From: Timo Zehnle Date: Tue, 14 Dec 2021 11:14:28 +0100 Subject: [PATCH] feat(comp): remove span + add space prop to button --- .changeset/happy-peaches-arrive.md | 6 ++++ docs/content/components/button.mdx | 15 +++++---- docs/content/components/inline.mdx | 2 +- .../src/ActionGroup/ActionGroup.test.tsx | 2 +- .../components/src/Button/Button.test.tsx | 32 +++++++++++++++---- packages/components/src/Button/Button.tsx | 8 +++-- 6 files changed, 46 insertions(+), 19 deletions(-) create mode 100644 .changeset/happy-peaches-arrive.md diff --git a/.changeset/happy-peaches-arrive.md b/.changeset/happy-peaches-arrive.md new file mode 100644 index 0000000000..7b498ae214 --- /dev/null +++ b/.changeset/happy-peaches-arrive.md @@ -0,0 +1,6 @@ +--- +'docs': patch +'@marigold/components': patch +--- + +feat(comp): remove span from button and add some styles to the button element, add space prop to button diff --git a/docs/content/components/button.mdx b/docs/content/components/button.mdx index de6bd16a94..e28d1cc107 100644 --- a/docs/content/components/button.mdx +++ b/docs/content/components/button.mdx @@ -19,6 +19,7 @@ The style variant and size of the button can be added with the variant prop and | `variant` (optional) | `primary`, `secondary`, `ghost`, `text` | `primary` | | `size` (optional) | `small`, `large` | `large` | | `disabled` (optional) | boolean | | +| `space` (optional) | `ResponsiveStyleValue` | `none` | ## Import @@ -136,20 +137,20 @@ They can be used in a similar way of a secondary button, but they are meant to b Icons can be added in Order to make clearer what a button does. Buttons should not be made only with icons, because every user may understand them on a different way. ```tsx - ``` ```tsx - ``` ### Small Buttons -If there are many actions in one page, is it possible to use small buttons instead of big ones. As well if the page is required to be onpen on mobile devices. +If there are many actions in one page, is it possible to use small buttons instead of big ones. As well if the page is required to be open on mobile devices. ```tsx @@ -202,25 +203,25 @@ If there are many actions in one page, is it possible to use small buttons inste Icons can be added in Order to make clearer what a button does. Buttons should not be made only with icons, because every user may understand them on a different way.Icons can be added in Order to make clearer what a button does. Buttons should not be made only with icons, because every user may understand them on a different way. ```tsx - ``` ```tsx - ``` ```tsx - ``` ```tsx - ``` diff --git a/docs/content/components/inline.mdx b/docs/content/components/inline.mdx index a248a4b64e..e14670aeed 100644 --- a/docs/content/components/inline.mdx +++ b/docs/content/components/inline.mdx @@ -13,7 +13,7 @@ Layout component to inline elements with space. This component uses the spaces f | Property | Type | Default | | :------- | :----------------------------- | :------- | | `space` | `ResponsiveStyleValue` | `none` | -| `align` | `left, right, center` | `center` | +| `align` | `center, top, bottom` | `center` | ## Import diff --git a/packages/components/src/ActionGroup/ActionGroup.test.tsx b/packages/components/src/ActionGroup/ActionGroup.test.tsx index 1540190903..98a7f9a56a 100644 --- a/packages/components/src/ActionGroup/ActionGroup.test.tsx +++ b/packages/components/src/ActionGroup/ActionGroup.test.tsx @@ -78,6 +78,6 @@ test('supports verticalAlignment prop', () => { const button1 = screen.getByText(/Button1/); const button2 = screen.getByText(/Button2/); - expect(getTopPadding(button1)).toEqual(''); + expect(getTopPadding(button1.parentElement!)).toEqual(''); expect(button2.parentElement).toHaveStyle(`padding-top: 2px`); }); diff --git a/packages/components/src/Button/Button.test.tsx b/packages/components/src/Button/Button.test.tsx index a274f9dc11..71032f33b4 100644 --- a/packages/components/src/Button/Button.test.tsx +++ b/packages/components/src/Button/Button.test.tsx @@ -19,6 +19,10 @@ const theme = { fontFamily: 'Arial', }, }, + space: { + none: 0, + small: 2, + }, }; test('supports default variant', () => { @@ -29,7 +33,7 @@ test('supports default variant', () => { ); const button = screen.getByText(/button/); - expect(button.parentElement).toHaveStyle(`font-family: Inter`); + expect(button).toHaveStyle(`font-family: Inter`); }); test('supports default size', () => { @@ -40,7 +44,7 @@ test('supports default size', () => { ); const button = screen.getByText(/button/); - expect(button.parentElement).toHaveStyle(`padding: 16px`); + expect(button).toHaveStyle(`padding: 16px`); }); test('accepts other variant than default', () => { @@ -51,7 +55,7 @@ test('accepts other variant than default', () => { ); const button = screen.getByText(/button/); - expect(button.parentElement).toHaveStyle(`font-family: Arial`); + expect(button).toHaveStyle(`font-family: Arial`); }); test('accepts other size than default', () => { @@ -62,7 +66,7 @@ test('accepts other size than default', () => { ); const button = screen.getByText(/button/); - expect(button.parentElement).toHaveStyle(`padding: 16px`); + expect(button).toHaveStyle(`padding: 16px`); }); test('renders + + ); + const button = screen.getByTitle(/iconbutton/); + + const style = window.getComputedStyle(button); + expect(style.columnGap).toBe(`2px`); +}); + test('accepts custom styles prop className', () => { render( diff --git a/packages/components/src/Button/Button.tsx b/packages/components/src/Button/Button.tsx index 389963fbcc..e9965c3031 100644 --- a/packages/components/src/Button/Button.tsx +++ b/packages/components/src/Button/Button.tsx @@ -16,6 +16,7 @@ export const Button: PolymorphicComponentWithRef = as = 'button', variant = 'primary', size = 'large', + space = 'none', disabled, children, className, @@ -37,13 +38,14 @@ export const Button: PolymorphicComponentWithRef = {...buttonProps} {...props} as={as} + display="inline-flex" + alignItems="center" variant={[`button.${variant}`, `button.${size}`]} className={className} ref={ref} + css={{ columnGap: space }} > - - {children} - + {children} ); }