Skip to content

Commit

Permalink
feat!: Make API for icon in button friendlier (#1700)
Browse files Browse the repository at this point in the history
Co-authored-by: alimpens <[email protected]>
  • Loading branch information
VincentSmedinga and alimpens authored Oct 25, 2024
1 parent 48dd441 commit 33c4c0e
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 46 deletions.
6 changes: 3 additions & 3 deletions packages/css/src/components/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
}
}

.ams-button--icon-position-only {
padding-block: var(--ams-button-icon-position-only-padding-block);
padding-inline: var(--ams-button-icon-position-only-padding-inline);
.ams-button--icon-only {
padding-block: var(--ams-button-icon-only-padding-block);
padding-inline: var(--ams-button-icon-only-padding-inline);
}
33 changes: 17 additions & 16 deletions packages/react/src/Button/Button.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ShareIcon } from '@amsterdam/design-system-react-icons'
import { CloseIcon } from '@amsterdam/design-system-react-icons'
import { render, screen } from '@testing-library/react'
import '@testing-library/jest-dom'
import { createRef } from 'react'
Expand Down Expand Up @@ -114,50 +114,51 @@ describe('Button', () => {

it('renders a button with an icon at the end', () => {
render(
<Button icon={ShareIcon}>
<span>Share</span>
<Button icon={CloseIcon}>
<span>Sluiten</span>
</Button>,
)

const button = screen.getByRole('button', {
name: 'Share',
name: 'Sluiten',
})
const icon = button.querySelector('.ams-icon:last-child')

expect(button).toBeInTheDocument()
const icon = button.querySelector('.ams-icon:last-child')
expect(icon).toBeInTheDocument()
})

it('renders a button with an icon at the start', () => {
it('renders a button with an icon before the label', () => {
render(
<Button icon={ShareIcon} iconPosition="start">
<span>Share</span>
<Button icon={CloseIcon} iconBefore>
<span>Sluiten</span>
</Button>,
)

const button = screen.getByRole('button', {
name: 'Share',
name: 'Sluiten',
})
const icon = button.querySelector('.ams-icon:first-child')

expect(button).toBeInTheDocument()
const icon = button.querySelector('.ams-icon:first-child')
expect(icon).toBeInTheDocument()
})

it('renders a button with an icon only', () => {
render(
<Button icon={ShareIcon} iconPosition="only" variant="tertiary">
Share
<Button icon={CloseIcon} iconOnly variant="tertiary">
Sluiten
</Button>,
)

const button = screen.getByRole('button', {
name: 'Share',
name: 'Sluiten',
})
const icon = button.querySelector('.ams-icon')
const label = button.querySelector('.ams-visually-hidden')

expect(button).toBeInTheDocument()
expect(button).toHaveClass('ams-button--icon-position-only')
const label = button.querySelector('.ams-visually-hidden')
expect(label).toHaveTextContent('Share')
expect(icon).toBeInTheDocument()
expect(label).toBeInTheDocument()
})
})
55 changes: 40 additions & 15 deletions packages/react/src/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,31 @@

import clsx from 'clsx'
import { forwardRef } from 'react'
import type { ButtonHTMLAttributes, ForwardedRef, PropsWithChildren } from 'react'
import type { ButtonHTMLAttributes, ForwardedRef, PropsWithChildren, ReactNode } from 'react'
import { Icon } from '../Icon'
import type { IconProps } from '../Icon'

type IconBeforeProp = {
/** Shows the icon before the label. Requires a value for `icon`. Cannot be used together with `iconOnly`. */
iconBefore?: boolean
iconOnly?: never
}

type IconOnlyProp = {
iconBefore?: never
/** Shows the icon without the label. Requires a value for `icon`. Cannot be used together with `iconBefore`. */
iconOnly?: boolean
}

type IconButtonProps = {
/** An icon to add to the button. */
/** Adds an icon to the button, showing it after the label. */
icon: IconProps['svg']
/** The position of the icon. The default is after the label. */
iconPosition?: 'start' | 'only'
}
} & (IconBeforeProp | IconOnlyProp)

type TextButtonProps = {
icon?: never
iconPosition?: never
iconBefore?: never
iconOnly?: never
}

export type ButtonProps = {
Expand All @@ -29,27 +40,41 @@ export type ButtonProps = {

export const Button = forwardRef(
(
{ children, className, disabled, icon, iconPosition, type, variant = 'primary', ...restProps }: ButtonProps,
{ children, className, disabled, icon, iconBefore, iconOnly, type, variant = 'primary', ...restProps }: ButtonProps,
ref: ForwardedRef<HTMLButtonElement>,
) => {
const content = (): ReactNode => {
switch (true) {
case !icon:
return children
case iconBefore:
return [<Icon svg={icon} size="level-5" />, children]
case iconOnly:
return [
<Icon key={1} svg={icon} size="level-5" square={true} />,
<span className="ams-visually-hidden" key={2}>
{children}
</span>,
]
default:
return [children, <Icon svg={icon} size="level-5" />]
}
}

return (
<button
{...restProps}
ref={ref}
disabled={disabled}
className={clsx(
'ams-button',
`ams-button--${variant}`,
icon && iconPosition === 'only' && `ams-button--icon-position-only`,
icon && iconOnly && !iconBefore && `ams-button--icon-only`,
className,
)}
disabled={disabled}
ref={ref}
type={type || 'button'}
>
{icon && (iconPosition === 'start' || iconPosition === 'only') && (
<Icon svg={icon} size="level-5" square={iconPosition === 'only'} />
)}
{icon && iconPosition === 'only' ? <span className="ams-visually-hidden">{children}</span> : children}
{icon && !iconPosition && <Icon svg={icon} size="level-5" />}
{content()}
</button>
)
},
Expand Down
2 changes: 1 addition & 1 deletion proprietary/tokens/src/components/ams/button.tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"color": { "value": "{ams.color.neutral-grey2}" }
}
},
"icon-position-only": {
"icon-only": {
"padding-block": { "value": "{ams.space.sm}" },
"padding-inline": { "value": "{ams.space.sm}" }
}
Expand Down
8 changes: 4 additions & 4 deletions storybook/src/components/Button/Button.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ They are also a good choice for buttons with an icon only.

<Canvas of={ButtonStories.Tertiary} />

### With an icon
### With icon

Add an icon if it makes it easier or faster to understand its purpose.
The icon will appear after the label.

<Canvas of={ButtonStories.WithIcon} />

### With an icon at the start
### With icon before

The icon can also appear before the label.

<Canvas of={ButtonStories.WithIconAtStart} />
<Canvas of={ButtonStories.WithIconBefore} />

### With an icon only
### With icon only

A button can even consist of an icon only.
Do this only for icons that are universally recognized.
Expand Down
26 changes: 19 additions & 7 deletions storybook/src/components/Button/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const meta = {
args: {
children: 'Versturen',
disabled: false,
icon: undefined,
iconBefore: false,
iconOnly: false,
variant: 'primary',
},
argTypes: {
Expand All @@ -27,12 +30,21 @@ const meta = {
options: [undefined, ...Object.keys(Icons)],
mapping: Icons,
},
iconPosition: {
iconBefore: {
control: {
type: 'inline-radio',
labels: { undefined: 'end', start: 'start', only: 'only' },
type: 'boolean',
},
if: {
arg: 'icon',
},
},
iconOnly: {
control: {
type: 'boolean',
},
if: {
arg: 'icon',
},
options: [undefined, 'start', 'only'],
},
},
} satisfies Meta<typeof Button>
Expand Down Expand Up @@ -64,19 +76,19 @@ export const WithIcon: Story = {
},
}

export const WithIconAtStart: Story = {
export const WithIconBefore: Story = {
args: {
children: 'Sluiten',
icon: Icons.CloseIcon,
iconPosition: 'start',
iconBefore: true,
},
}

export const WithIconOnly: Story = {
args: {
children: 'Sluiten',
icon: Icons.CloseIcon,
iconPosition: 'only',
iconOnly: true,
variant: 'tertiary',
},
}
Expand Down

0 comments on commit 33c4c0e

Please sign in to comment.