Skip to content

Commit d6a242a

Browse files
committed
add feedback
1 parent 185ddcd commit d6a242a

7 files changed

Lines changed: 31 additions & 6 deletions

File tree

.changeset/hero-button-group.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
'@primer/react-brand': patch
33
---
44

5-
Added `Hero.ButtonGroup` for rendering `Button` and `ActionMenu` children. `Hero.ButtonGroup` is now the defacto way to display buttons in the `Hero`.
5+
Added `Hero.ButtonGroup` for rendering `Button` and `ActionMenu` children. `Hero.ButtonGroup` is now the de facto way to display buttons in the `Hero`.
66

77
⚠️ `Hero.PrimaryAction` and `Hero.SecondaryAction` are now deprecated. Please migrate over to `Hero.ButtonGroup` as they will be removed in a future release.
88

.changeset/soft-banners-sparkle.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44

55
- Restored rounded corners to `CTABanner` while preserving square edges when grid lines are enabled.
66
- Improved `ButtonGroup` to forward custom class names alongside its default styles.
7-
- Added native `ActionMenu` child support to `ButtonGroup`, including automatic positional variants and the `CTABanner.ButtonGroup` wrapper.
7+
- Added native `ActionMenu` child support to `ButtonGroup`, including automatic sizes and positional variants, and the `CTABanner.ButtonGroup` wrapper.

apps/next-docs/content/components/ButtonGroup/react.mdx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ This is the default variant for the ButtonGroup component. The first item in the
3434

3535
Explicit `variant` overrides to `Button` or `ActionMenu.Button` will override the default values.
3636

37+
`buttonSize` applies to both child types. Because `ActionMenu` supports only `small` and `medium`, it remains `medium` when `buttonSize="large"`. Set `size` on `ActionMenu` to override the inherited size.
38+
3739
```jsx live
3840
<ButtonGroup>
3941
<Button>Primary action</Button>
@@ -49,7 +51,7 @@ Explicit `variant` overrides to `Button` or `ActionMenu.Button` will override th
4951

5052
### Sizes
5153

52-
The ButtonGroup component can be rendered in different sizes in `medium` and `large` sizes. The default size is `medium`.
54+
The ButtonGroup component supports `small`, `medium`, and `large` buttons. The default size is `medium`. ActionMenu children support `small` and `medium`.
5355

5456
```jsx live
5557
<ButtonGroup buttonSize="large">
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use client'
22
import {PropTableValues} from '@primer/doctocat-nextjs/components'
33

4-
export const ButtonGroupSizesProp = () => <PropTableValues values={['medium', 'large']} commaSeparated />
4+
export const ButtonGroupSizesProp = () => <PropTableValues values={['small', 'medium', 'large']} commaSeparated />
55
export const ButtonGroupAsProp = () => <PropTableValues values={['button', 'a']} commaSeparated />

packages/react/src/ButtonGroup/ButtonGroup.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const meta = {
1717
description: 'The size of the button elements',
1818
control: {
1919
type: 'radio',
20-
options: ['medium', 'large'],
20+
options: ['small', 'medium', 'large'],
2121
},
2222
},
2323
buttonsAs: {

packages/react/src/ButtonGroup/ButtonGroup.test.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,25 @@ describe('ButtonGroup', () => {
139139
expect(getByRole('menu', {name: 'More actions'})).toBeInTheDocument()
140140
})
141141

142+
it.each([
143+
['small', 'small'],
144+
['medium', 'medium'],
145+
['large', 'medium'],
146+
] as const)('applies the %s group size to ActionMenu as %s', (buttonSize, expectedSize) => {
147+
const {getByRole} = render(
148+
<ButtonGroup buttonSize={buttonSize}>
149+
<ActionMenu>
150+
<ActionMenu.Button>More actions</ActionMenu.Button>
151+
<ActionMenu.Overlay aria-label="More actions">
152+
<ActionMenu.Item value="Contact sales">Contact sales</ActionMenu.Item>
153+
</ActionMenu.Overlay>
154+
</ActionMenu>
155+
</ButtonGroup>,
156+
)
157+
158+
expect(getByRole('button', {name: 'More actions'})).toHaveClass(`Button--size-${expectedSize}`)
159+
})
160+
142161
it('applies variants automatically to ActionMenu children', () => {
143162
const {getByRole} = render(
144163
<ButtonGroup>

packages/react/src/ButtonGroup/ButtonGroup.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const ButtonGroup = forwardRef(
3434
}
3535

3636
const actionMenu = child as PrimerBrandActionMenuType
37+
const actionMenuSize = buttonSize === 'large' ? 'medium' : buttonSize
3738
const actionMenuChildren = React.Children.map(actionMenu.props.children, actionMenuChild => {
3839
if (
3940
React.isValidElement<React.ComponentProps<typeof ActionMenu.Button>>(actionMenuChild) &&
@@ -46,7 +47,10 @@ export const ButtonGroup = forwardRef(
4647
return actionMenuChild
4748
})
4849

49-
return React.cloneElement(actionMenu, {children: actionMenuChildren})
50+
return React.cloneElement(actionMenu, {
51+
children: actionMenuChildren,
52+
size: actionMenu.props.size ?? actionMenuSize,
53+
})
5054
})
5155

5256
return (

0 commit comments

Comments
 (0)