Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow Button to display an icon without a label #1654

Merged
merged 35 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3230f5d
Changed children to label and added icons
dlnr Aug 23, 2024
14e1261
refactor: Remove unused children prop in Button component
dlnr Aug 23, 2024
d9699bb
Button label update, and allow children poc
dlnr Sep 3, 2024
51271bc
Merge branch 'develop' of https://github.com/Amsterdam/design-system …
dlnr Oct 8, 2024
6bfb91c
Updates to new changes in figma branch, revert to enum icon position
dlnr Oct 8, 2024
479bd06
Test fixes
dlnr Oct 9, 2024
6eb720a
Refactor Button component to support icon-only buttons
dlnr Oct 11, 2024
ad3dc19
Test adjustments
dlnr Oct 11, 2024
8d3e922
Revert "Test adjustments"
dlnr Oct 14, 2024
86e0b33
Revert "Refactor Button component to support icon-only buttons"
dlnr Oct 14, 2024
00c8d0b
Reverting to viewer props
dlnr Oct 14, 2024
d9319dd
Merge branch 'develop' of https://github.com/Amsterdam/design-system …
dlnr Oct 14, 2024
be63fca
Test fix
dlnr Oct 14, 2024
e42cabb
Change in documentation
dlnr Oct 14, 2024
13ef881
More docs changes
dlnr Oct 14, 2024
b313447
Button text in stories
dlnr Oct 14, 2024
724b05a
Update documentation on variants ans icons
VincentSmedinga Oct 14, 2024
0b68fb5
Suggested changes
dlnr Oct 15, 2024
e1473c9
Merge branch 'develop' of https://github.com/Amsterdam/design-system …
dlnr Oct 15, 2024
4a0f2a5
Button story fix
dlnr Oct 15, 2024
a9d7f96
Merge branch 'develop' into feature/DES-859-icon-only-button
VincentSmedinga Oct 18, 2024
a4285d6
Update storybook/src/components/Button/Button.docs.mdx
dlnr Oct 18, 2024
5ab59ca
Suggestions
dlnr Oct 18, 2024
1756232
iconProps suggestion
dlnr Oct 18, 2024
20a164b
Class change
dlnr Oct 18, 2024
8e67d2e
Use a universally recognized as per our own docs
VincentSmedinga Oct 18, 2024
e8cd4db
Fix class name
VincentSmedinga Oct 18, 2024
2a1b43f
Test icon positions with a span
dlnr Oct 21, 2024
771da4a
Icon control fix and example buttons
dlnr Oct 21, 2024
7c4a52c
Icon should only be square without a label
dlnr Oct 21, 2024
e972a60
Prevent iconposition class without icon and token naming
dlnr Oct 21, 2024
025b0fb
Refactor Button component to separate IconButton and TextButton props
dlnr Oct 21, 2024
3e2ef6c
Merge branch 'develop' into feature/DES-859-icon-only-button
VincentSmedinga Oct 21, 2024
8cf5ef6
Obsolete value for icon in stories
dlnr Oct 21, 2024
b96ebbf
Merge branch 'develop' of https://github.com/Amsterdam/design-system …
dlnr Oct 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/css/src/components/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,17 @@
color: var(--ams-button-tertiary-hover-color);
}
}

.ams-button--icon-only {
padding-block: var(--ams-button-icon-only-padding-block);
padding-inline: var(--ams-button-icon-only-padding-inline);
}

// TODO: Proposition
// .ams-button--icon-start {
// padding-inline-start: var(--ams-button-icon-start-padding-inline-start);
// }

// .ams-button--icon-end {
// padding-inline-end: var(--ams-button-icon-end-padding-inline-end);
// }
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
44 changes: 43 additions & 1 deletion packages/react/src/Button/Button.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ShareIcon } 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 @@ -104,10 +105,51 @@ describe('Button', () => {
it('is able to pass a React ref', () => {
const ref = createRef<HTMLButtonElement>()

const { container } = render(<Button ref={ref} />)
const { container } = render(<Button ref={ref}>Click me!</Button>)

const button = container.querySelector(':only-child')

expect(ref.current).toBe(button)
})

it('renders a button with an icon', () => {
alimpens marked this conversation as resolved.
Show resolved Hide resolved
render(<Button icon={ShareIcon}>Share</Button>)

const button = screen.getByRole('button', {
name: 'Share',
})

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

it('renders a button with an iconStart', () => {
render(<Button iconStart={ShareIcon}>Share</Button>)

const button = screen.getByRole('button', {
name: 'Share',
})

expect(button).toBeInTheDocument()
const icon = button.querySelector('.ams-icon')
expect(button.firstChild).toBe(icon)
})

it('renders a button with an iconOnly', () => {
render(
<Button icon={ShareIcon} iconOnly>
Share
</Button>,
)

const button = screen.getByRole('button', {
name: 'Share',
})

expect(button).toBeInTheDocument()
const icon = button.querySelector('.ams-icon')
const visuallyHidden = button.querySelector('.ams-visually-hidden')
expect(icon && visuallyHidden).toBeInTheDocument()
})
})
24 changes: 20 additions & 4 deletions packages/react/src/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,42 @@
import clsx from 'clsx'
import { forwardRef } from 'react'
import type { ButtonHTMLAttributes, ForwardedRef, PropsWithChildren } from 'react'
import { Icon } from '../Icon'

type ButtonIcon = {
icon?: Function
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
iconStart?: never
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
iconOnly?: boolean
}

type ButtonIconStart = {
icon?: never
iconStart?: Function
iconOnly?: boolean
}

export type ButtonProps = {
/** The level of prominence. Use a primary button only once per page or section. */
variant?: 'primary' | 'secondary' | 'tertiary'
} & PropsWithChildren<ButtonHTMLAttributes<HTMLButtonElement>>
} & (ButtonIcon | ButtonIconStart) &
PropsWithChildren<ButtonHTMLAttributes<HTMLButtonElement>>

export const Button = forwardRef(
(
{ children, className, type, disabled, variant = 'primary', ...restProps }: ButtonProps,
{ children, className, type, disabled, variant = 'primary', icon, iconStart, iconOnly, ...restProps }: ButtonProps,
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
ref: ForwardedRef<HTMLButtonElement>,
) => {
return (
<button
{...restProps}
ref={ref}
disabled={disabled}
className={clsx('ams-button', `ams-button--${variant}`, className)}
className={clsx('ams-button', `ams-button--${variant}`, { 'ams-button--icon-only': iconOnly }, className)}
type={type || 'button'}
>
{children}
{iconStart && <Icon svg={iconStart} size="level-5" square />}
{iconOnly ? <span className="ams-visually-hidden">{children}</span> : children}
{icon && <Icon svg={icon} size="level-5" square />}
</button>
)
},
Expand Down
10 changes: 10 additions & 0 deletions proprietary/tokens/src/components/ams/button.tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@
"background-color": { "value": "transparent" },
"color": { "value": "{ams.color.neutral-grey2}" }
}
},
"icon-only": {
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
"padding-block": { "value": "{ams.space.sm}" },
"padding-inline": { "value": "{ams.space.sm}" }
},
"icon-start": {
"padding-inline-start": { "value": "{ams.space.sm}" }
},
"icon-end": {
"padding-inline-end": { "value": "{ams.space.sm}" }
}
}
}
Expand Down
14 changes: 12 additions & 2 deletions storybook/src/components/Button/Button.docs.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,19 @@ Use tertiary Buttons for unimportant calls to action – as many as necessary.

<Canvas of={ButtonStories.Tertiary} />

### With icon
### Button with a start icon
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved

<Canvas of={ButtonStories.WithIcon} />
<Canvas of={ButtonStories.StartIcon} />

### Button with a end icon

<Canvas of={ButtonStories.EndIcon} />

### Button with an icon only

To comply with accessibility guidelines, the icon-only button should have a label.

<Canvas of={ButtonStories.OnlyIcon} />

### Text wrapping

Expand Down
32 changes: 21 additions & 11 deletions storybook/src/components/Button/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Copyright Gemeente Amsterdam
*/

import { Icon } from '@amsterdam/design-system-react'
import { Button } from '@amsterdam/design-system-react/src'
import { ShareIcon } from '@amsterdam/design-system-react-icons'
import { Meta, StoryObj } from '@storybook/react'
Expand All @@ -12,14 +11,16 @@ const meta = {
title: 'Components/Buttons/Button',
component: Button,
args: {
children: 'Button label',
children: 'Actietekst',
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
variant: 'primary',
disabled: false,
},
argTypes: {
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
children: {
description: 'The text for the label and/or an icon.',
table: { disable: false },
icon: {
table: { disable: true },
},
iconStart: {
table: { disable: true },
},
disabled: {
description: 'Prevents interaction. Avoid if possible.',
Expand All @@ -45,14 +46,23 @@ export const Tertiary: Story = {
},
}

export const WithIcon: Story = {
export const StartIcon: Story = {
args: {
children: ['Button label', <Icon key="icon" svg={ShareIcon} size="level-5" />],
iconStart: ShareIcon,
},
argTypes: {
children: {
table: { disable: true },
},
}

export const EndIcon: Story = {
args: {
icon: ShareIcon,
},
}

export const OnlyIcon: Story = {
args: {
variant: 'tertiary',
icon: ShareIcon,
iconOnly: true,
},
}

Expand Down