Skip to content

Button API alignment #2893

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

Merged
merged 36 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
c5d5d5d
copy from other pr
langermank Feb 10, 2023
ce151b7
snippy snaps
langermank Feb 10, 2023
b09569d
Create .changeset/flat-onions-sort.md
langermank Feb 10, 2023
fa46713
color swaps
langermank Feb 10, 2023
e883481
Merge branch 'button-api-breaking-changes' of https://github.com/prim…
langermank Feb 10, 2023
24960bb
snappy snips
langermank Feb 10, 2023
df4ca84
add missing type
langermank Feb 10, 2023
cff1ce8
Merge branch 'main' into button-api-breaking-changes
broccolinisoup Feb 16, 2023
526cdf1
use the correct prop for ActionMenu.Button
broccolinisoup Feb 16, 2023
90f8241
Merge branch 'main' into button-api-breaking-changes
broccolinisoup Feb 17, 2023
9925d0c
type fixes
broccolinisoup Feb 17, 2023
1b28ee6
Merge branch 'main' into button-api-breaking-changes
broccolinisoup Feb 17, 2023
d189074
Merge branch 'main' of https://github.com/primer/react into button-ap…
langermank Apr 13, 2023
333deb5
fix docs
langermank Apr 13, 2023
32217f1
snaps
langermank Apr 13, 2023
b9b9a0e
Update flat-onions-sort.md
langermank Apr 13, 2023
43d1226
Merge branch 'next-major' into button-api-breaking-changes
langermank Apr 19, 2023
f5b93c8
fix merge conflict?
langermank Apr 19, 2023
8d2fea3
change to count prop
langermank May 2, 2023
2068339
Merge branch 'next-major' into button-api-breaking-changes
broccolinisoup May 4, 2023
43a7649
remove Button.Counter in favor of count prop
broccolinisoup May 4, 2023
6e555d8
Merge branch 'next-major' into button-api-breaking-changes
broccolinisoup May 11, 2023
d32f2cc
Hoping to revert some unnecessary snaps diff
broccolinisoup May 12, 2023
ad7a6e8
update Button.Counter to use count prop
broccolinisoup May 12, 2023
e5b9201
update changeset and remove outine button examples from the story
broccolinisoup May 12, 2023
1a4db9c
Update generated/components.json
broccolinisoup May 12, 2023
f53c55a
Merge branch 'next-major' into button-api-breaking-changes
broccolinisoup May 24, 2023
acdd42d
test: remove outline tests
joshblack Jun 5, 2023
f9bbce7
Merge branch 'next-major' into button-api-breaking-changes
broccolinisoup Jun 6, 2023
ea92994
Merge branch 'next-major' into button-api-breaking-changes
joshblack Jun 6, 2023
de0facf
test(e2e): remove outline tests
joshblack Jun 8, 2023
fd99baf
test(e2e): remove outdated snapshots
joshblack Jun 8, 2023
761b226
Merge branch 'next-major' into button-api-breaking-changes
joshblack Jun 8, 2023
e138704
test(vrt): update snapshots
joshblack Jun 8, 2023
7b9586d
fix stories, fix count and trailing vis conflict
langermank Jun 9, 2023
de3dc1c
test(vrt): update snapshots
langermank Jun 9, 2023
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
7 changes: 7 additions & 0 deletions .changeset/flat-onions-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@primer/react": major
---

- Changes `leadingIcon` and `trailingIcon` to `leadingVisual` and `trailingVisual`
- Removes `Button.Counter` as a child component, replacing it with a `count` prop. This change allows us to use the `trailingVisual` slot for counters.
- Removes the `outline` button variant as we wish to only support `invisible` buttons.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
2 changes: 1 addition & 1 deletion docs/content/ActionMenu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ const Example = () => {

return (
<ActionMenu>
<ActionMenu.Button leadingIcon={selectedType.icon}>{selectedType.name}</ActionMenu.Button>
<ActionMenu.Button leadingVisual={selectedType.icon}>{selectedType.name}</ActionMenu.Button>
<ActionMenu.Overlay width="medium">
<ActionList selectionVariant="single">
{fieldTypes.map((type, index) => (
Expand Down
25 changes: 6 additions & 19 deletions docs/content/Button.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ The `danger` variant of `Button` is used to warn users about potentially destruc
<Button variant="danger">Danger</Button>
```

### Outline button

The `outline` variant of `Button` is typically used as a secondary button

```jsx live
<Button variant="outline">Outline</Button>
```

### Invisible button

The `invisible` variant of `Button` indicates that the action is a low priority one.
Expand Down Expand Up @@ -68,11 +60,11 @@ It is recommended to use an octicon here.

```jsx live
<>
<Button leadingIcon={SearchIcon}>Search</Button>
<Button trailingIcon={SearchIcon} sx={{mt: 2}}>
<Button leadingVisual={SearchIcon}>Search</Button>
<Button trailingVisual={SearchIcon} sx={{mt: 2}}>
Search
</Button>
<Button leadingIcon={SearchIcon} trailingIcon={CheckIcon} sx={{mt: 2}}>
<Button leadingVisual={SearchIcon} trailingVisual={CheckIcon} sx={{mt: 2}}>
Search
</Button>
</>
Expand All @@ -98,17 +90,12 @@ A separate component called `IconButton` is used if the action shows only an ico
</>
```

### Counter component
### Button with counter

A common use case for primer is a button with a counter component which shows the child count value.
We provide `Button.Counter` as a composite component which requires you to provide a number as child.
The counter will match the `variant` styles of the parent button.
To show a count value as a trailing visual inside `Button`, pass a value to the `count` prop. The counter will match the `variant` styles of the parent button.

```jsx live
<Button>
Watch
<Button.Counter>1</Button.Counter>
</Button>
<Button count="1">Watch</Button>
```

### Block button
Expand Down
8 changes: 4 additions & 4 deletions docs/content/drafts/PageHeader.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ import {PageHeader} from '@primer/react/drafts'
<PageHeader.Actions>
<IconButton aria-label="Workflows" icon={WorkflowIcon} />
<IconButton aria-label="Insights" icon={GraphIcon} />
<Button variant="primary" trailingIcon={TriangleDownIcon}>
<Button variant="primary" trailingVisual={TriangleDownIcon}>
Add Item
</Button>
<IconButton aria-label="Settings" icon={GearIcon} />
Expand Down Expand Up @@ -212,7 +212,7 @@ import {PageHeader} from '@primer/react/drafts'
<PageHeader.ParentLink href="http://github.com">Parent Link</PageHeader.ParentLink>

<PageHeader.ContextAreaActions>
<Button size="small" leadingIcon={GitBranchIcon}>
<Button size="small" leadingVisual={GitBranchIcon}>
Main
</Button>
<IconButton size="small" aria-label="More Options" icon={KebabHorizontalIcon} />
Expand Down Expand Up @@ -241,7 +241,7 @@ import {PageHeader} from '@primer/react/drafts'
</PageHeader.ContextBar>

<PageHeader.ContextAreaActions>
<Button size="small" leadingIcon={GitBranchIcon}>
<Button size="small" leadingVisual={GitBranchIcon}>
Main
</Button>
<IconButton size="small" aria-label="More Options" icon={KebabHorizontalIcon} />
Expand Down Expand Up @@ -274,7 +274,7 @@ import {PageHeader} from '@primer/react/drafts'
</PageHeader.ContextBar>

<PageHeader.ContextAreaActions hidden={false}>
<Button size="small" leadingIcon={GitBranchIcon}>
<Button size="small" leadingVisual={GitBranchIcon}>
Main
</Button>
<IconButton size="small" aria-label="More Options" icon={KebabHorizontalIcon} />
Expand Down
34 changes: 0 additions & 34 deletions e2e/components/Button.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,40 +275,6 @@ test.describe('Button', () => {
}
})

test.describe('Outline', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-button-features--outline',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Button.Outline.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-button-features--outline',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Primary', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
34 changes: 0 additions & 34 deletions e2e/components/LinkButton.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,40 +241,6 @@ test.describe('LinkButton', () => {
}
})

test.describe('Outline', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-linkbutton-features--outline',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`LinkButton.Outline.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-linkbutton-features--outline',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})

test.describe('Primary', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
25 changes: 7 additions & 18 deletions generated/components.json
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,12 @@
"type": "React.ReactNode",
"description": "The content of the button."
},
{
"name": "count",
"required": true,
"type": "number | string",
"description": "For counter buttons, the number to display."
},
{
"name": "variant",
"type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'",
Expand Down Expand Up @@ -922,24 +928,7 @@
"passthrough": {
"element": "button",
"url": "https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attributes"
},
"subcomponents": [
{
"name": "Button.Counter",
"props": [
{
"name": "children",
"required": true,
"type": "number",
"description": "The counter value."
},
{
"name": "sx",
"type": "SystemStyleObject"
}
]
}
]
}
},
"icon_button": {
"id": "icon_button",
Expand Down
4 changes: 2 additions & 2 deletions src/ActionMenu/ActionMenu.examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const GroupsAndDescriptions = () => {

return (
<ActionMenu open>
<ActionMenu.Button variant="default" trailingIcon={GearIcon}>
<ActionMenu.Button variant="default" trailingVisual={GearIcon}>
Milestone
</ActionMenu.Button>
<ActionMenu.Overlay width="medium">
Expand Down Expand Up @@ -201,7 +201,7 @@ export const MixedSelection = () => {

return (
<ActionMenu>
<ActionMenu.Button leadingIcon={selectedOption ? selectedOption.icon : undefined}>
<ActionMenu.Button leadingVisual={selectedOption ? selectedOption.icon : undefined}>
{selectedOption ? `Group by ${selectedOption.text}` : 'Group items by'}
</ActionMenu.Button>
<ActionMenu.Overlay width="medium">
Expand Down
16 changes: 6 additions & 10 deletions src/Button/Button.dev.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,20 @@ export const InvisibleVariants = () => {
return (
<div style={{display: 'flex', flexDirection: 'row', gap: '1rem'}}>
<Button variant="invisible">Button</Button>
<Button variant="invisible" leadingIcon={SearchIcon}>
<Button variant="invisible" leadingVisual={SearchIcon}>
Button
</Button>
<Button variant="invisible" trailingAction={TriangleDownIcon}>
Button
</Button>
<Button variant="primary">
<Button variant="primary" count={count}>
Button
<Button.Counter>{count}</Button.Counter>
</Button>
<Button variant="invisible" leadingIcon={EyeIcon}>
<Button variant="invisible" leadingVisual={EyeIcon} count={count}>
Button
<Button.Counter>{count}</Button.Counter>
</Button>
<Button variant="invisible" leadingIcon={EyeIcon} trailingAction={TriangleDownIcon}>
<Button variant="invisible" leadingVisual={EyeIcon} trailingAction={TriangleDownIcon} count={count}>
Button
<Button.Counter>{count}</Button.Counter>
</Button>
<IconButton icon={TriangleDownIcon} variant="invisible" aria-label="Invisible" />
</div>
Expand Down Expand Up @@ -67,7 +64,7 @@ export const TestSxProp = () => {
>
Pink
</Button>
<Button leadingIcon={SearchIcon} variant="invisible" sx={{color: 'deeppink'}}>
<Button leadingVisual={SearchIcon} variant="invisible" sx={{color: 'deeppink'}}>
Pink
</Button>
<Button
Expand All @@ -89,9 +86,8 @@ export const TestSxProp = () => {
<Button size="small" block variant="invisible" sx={{width: 320}}>
Overriden Block
</Button>
<Button>
<Button sx={{fontSize: 32}} count={count}>
Watch
<Button.Counter sx={{fontSize: 32}}>{count}</Button.Counter>
</Button>
</div>
)
Expand Down
27 changes: 8 additions & 19 deletions src/Button/Button.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
"type": "React.ReactNode",
"description": "The content of the button."
},
{
"name": "count",
"required": true,
"type": "number | string",
"description": "For counter buttons, the number to display."
},
{
"name": "variant",
"type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'",
Expand Down Expand Up @@ -49,22 +55,5 @@
"passthrough": {
"element": "button",
"url": "https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attributes"
},
"subcomponents": [
{
"name": "Button.Counter",
"props": [
{
"name": "children",
"required": true,
"type": "number",
"description": "The counter value."
},
{
"name": "sx",
"type": "SystemStyleObject"
}
]
}
]
}
}
}
Loading