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

Document and test Button component's "small" size #566

Merged
merged 12 commits into from
Apr 12, 2024
6 changes: 6 additions & 0 deletions .changeset/early-kangaroos-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@primer/brand-primitives': patch
'@primer/react-brand': patch
---

Corrected block padding of `size="small"` Button component
joshfarrant marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions apps/docs/content/components/Button/react.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The `subtle` variant of Button indicates that the action is a low priority one.

```jsx live
<Stack direction="vertical" alignItems="flex-start">
<Button size="small">Register now</Button>
<Button size="medium">Register now</Button>
<Button size="large">Register now</Button>
</Stack>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"value": "{base.size.12}"
},
"paddingBlock": {
"value": "{base.size.12}"
"value": "{base.size.8}"
},
"paddingInline": {
"condensed": {
Expand Down
18 changes: 12 additions & 6 deletions packages/react/src/Button/Button.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ export const SubtleDisabled = () => (
</Button>
)

export const Small = () => (
<Button as="a" variant="primary" size="small" href="#">
Small action
</Button>
)

export const Large = () => (
<Button as="a" variant="primary" size="large" href="#">
Large action
Expand All @@ -101,8 +107,8 @@ export const Polymorphism = () => (
</Button>
)

/*
Storybook Interactions cannot natively simulate hover state on CSS pseudo-class.
/*
Storybook Interactions cannot natively simulate hover state on CSS pseudo-class.
Only simulates JS-based state.
TODO: Investigate perf impact of using "https://storybook.js.org/addons/storybook-addon-pseudo-states"
*/
Expand All @@ -120,8 +126,8 @@ WithHoverInteraction.play = async ({canvasElement}) => {
}
WithHoverInteraction.storyName = 'Primary button with hover interaction'

/*
Storybook Interactions cannot natively simulate hover state on CSS pseudo-class.
/*
Storybook Interactions cannot natively simulate hover state on CSS pseudo-class.
Only simulates JS-based state.
TODO: Investigate perf impact of using "https://storybook.js.org/addons/storybook-addon-pseudo-states"
*/
Expand All @@ -139,8 +145,8 @@ SecondaryWithHoverInteraction.play = async ({canvasElement}) => {
}
SecondaryWithHoverInteraction.storyName = 'Secondary button with hover interaction'

/*
Storybook Interactions cannot natively simulate hover state on CSS pseudo-class.
/*
Storybook Interactions cannot natively simulate hover state on CSS pseudo-class.
Only simulates JS-based state.
TODO: Investigate perf impact of using "https://storybook.js.org/addons/storybook-addon-pseudo-states"
*/
Expand Down
27 changes: 16 additions & 11 deletions packages/react/src/Button/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, {render, cleanup} from '@testing-library/react'
import '@testing-library/jest-dom'
import userEvent from '@testing-library/user-event'

import {Button} from './Button'
import {Button, ButtonSizes} from './Button'
import {axe, toHaveNoViolations} from 'jest-axe'
import {SearchIcon} from '@primer/octicons-react'

Expand Down Expand Up @@ -249,24 +249,29 @@ describe('Button', () => {
})

it('spans full width when block prop is true', () => {
const {getByTestId} = render(
<Button block={true} data-testid="test-button">
Full-Width Button
</Button>,
)
const {getByRole} = render(<Button block={true}>Full-Width Button</Button>)

const btnEl = getByTestId('test-button')
const btnEl = getByRole('button')

// Ensure the button has the block CSS class when block is true
expect(btnEl.classList).toContain('Button--block')
})

it('is not block by default', () => {
const {getByTestId} = render(<Button data-testid="test-button">Default Button</Button>)
const {getByRole} = render(<Button>Default Button</Button>)

const btnEl = getByTestId('test-button')
const btnEl = getByRole('button')

// Ensure the button does not have the CSS class when block is false (default)
expect(btnEl.classList).not.toContain('Button--block')
})

it('applies the correct class for each size', () => {
for (const i in ButtonSizes) {
const size = ButtonSizes[i]
const {getAllByRole} = render(<Button size={size}>Button</Button>)

const btnEl = getAllByRole('button')

expect(btnEl[i].classList).toContain(`Button--size-${size}`)
}
})
})
7 changes: 7 additions & 0 deletions packages/react/src/Button/Button.visual.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ test.describe('Visual Comparison: Button', () => {
expect(await page.screenshot()).toMatchSnapshot()
})

test('Button / Small', async ({page}) => {
await page.goto('http://localhost:6006/iframe.html?args=&id=components-button-features--small&viewMode=story')

await page.waitForTimeout(500)
expect(await page.screenshot()).toMatchSnapshot()
})

test('Button / Large', async ({page}) => {
await page.goto('http://localhost:6006/iframe.html?args=&id=components-button-features--large&viewMode=story')

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading