Skip to content

Commit dc2f083

Browse files
jonrohanjonrohan
authoredDec 5, 2024··
chore(ButtonGroup): Remove the CSS modules feature flag from ButtonGroup (#5339)
* Remove the CSS modules feature flag * Create four-plants-thank.md * test(vrt): update snapshots * Revert changes to overlay screenshots --------- Co-authored-by: jonrohan <jonrohan@users.noreply.github.com>
·
@primer/react@37.18.0@primer/react@37.7.0
1 parent 4c7056b commit dc2f083

13 files changed

+91
-197
lines changed
 

‎.changeset/four-plants-thank.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/react": minor
3+
---
4+
5+
Remove the CSS modules feature flag from ButtonGroup

‎e2e/components/ButtonGroup.test.ts

+52-111
Original file line numberDiff line numberDiff line change
@@ -2,116 +2,57 @@ import {test, expect} from '@playwright/test'
22
import {visit} from '../test-helpers/storybook'
33
import {themes} from '../test-helpers/themes'
44

5-
test.describe('ButtonGroup', () => {
6-
test.describe('Default', () => {
7-
for (const theme of themes) {
8-
test.describe(theme, () => {
9-
test('default @vrt', async ({page}) => {
10-
await visit(page, {
11-
id: 'components-buttongroup--default',
12-
globals: {
13-
colorScheme: theme,
14-
},
15-
})
16-
17-
// Default state
18-
expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Default.${theme}.png`)
19-
})
20-
21-
test('axe @aat', async ({page}) => {
22-
await visit(page, {
23-
id: 'components-buttongroup--default',
24-
globals: {
25-
colorScheme: theme,
26-
},
27-
})
28-
await expect(page).toHaveNoViolations()
29-
})
30-
})
31-
}
32-
})
33-
34-
test.describe('Playground', () => {
35-
for (const theme of themes) {
36-
test.describe(theme, () => {
37-
test('default @vrt', async ({page}) => {
38-
await visit(page, {
39-
id: 'components-buttongroup--playground',
40-
globals: {
41-
colorScheme: theme,
42-
},
43-
})
44-
45-
// Default state
46-
expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Playground.${theme}.png`)
47-
})
48-
49-
test('axe @aat', async ({page}) => {
50-
await visit(page, {
51-
id: 'components-buttongroup--playground',
52-
globals: {
53-
colorScheme: theme,
54-
},
55-
})
56-
await expect(page).toHaveNoViolations()
57-
})
58-
})
59-
}
60-
})
5+
const stories = [
6+
{
7+
title: 'Default',
8+
id: 'components-buttongroup--default',
9+
},
10+
{
11+
title: 'Playground',
12+
id: 'components-buttongroup--playground',
13+
},
14+
{
15+
title: 'Icon Buttons',
16+
id: 'components-buttongroup-features--icon-buttons',
17+
},
18+
{
19+
title: 'As Toolbar',
20+
id: 'components-buttongroup-features--as-toolbar',
21+
},
22+
{
23+
title: 'SX Prop',
24+
id: 'components-buttongroup-dev--sx-prop',
25+
},
26+
] as const
6127

62-
test.describe('Icon Buttons', () => {
63-
for (const theme of themes) {
64-
test.describe(theme, () => {
65-
test('default @vrt', async ({page}) => {
66-
await visit(page, {
67-
id: 'components-buttongroup-features--icon-buttons',
68-
globals: {
69-
colorScheme: theme,
70-
},
71-
})
72-
73-
// Default state
74-
expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Icon Buttons.${theme}.png`)
75-
})
76-
77-
test('axe @aat', async ({page}) => {
78-
await visit(page, {
79-
id: 'components-buttongroup-features--icon-buttons',
80-
globals: {
81-
colorScheme: theme,
82-
},
83-
})
84-
await expect(page).toHaveNoViolations()
85-
})
86-
})
87-
}
88-
})
89-
90-
test.describe('As Toolbar', () => {
91-
for (const theme of themes) {
92-
test.describe(theme, () => {
93-
test('default @vrt', async ({page}) => {
94-
await visit(page, {
95-
id: 'components-buttongroup-features--as-toolbar',
96-
globals: {
97-
colorScheme: theme,
98-
},
99-
})
100-
101-
// Default state
102-
expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.As Toolbar.${theme}.png`)
103-
})
104-
105-
test('axe @aat', async ({page}) => {
106-
await visit(page, {
107-
id: 'components-buttongroup-features--as-toolbar',
108-
globals: {
109-
colorScheme: theme,
110-
},
111-
})
112-
await expect(page).toHaveNoViolations()
113-
})
114-
})
115-
}
116-
})
28+
test.describe('ButtonGroup', () => {
29+
for (const story of stories) {
30+
test.describe(story.title, () => {
31+
for (const theme of themes) {
32+
test.describe(theme, () => {
33+
test('@vrt', async ({page}) => {
34+
await visit(page, {
35+
id: story.id,
36+
globals: {
37+
colorScheme: theme,
38+
},
39+
})
40+
41+
// Default state
42+
expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.${story.title}.${theme}.png`)
43+
})
44+
45+
test('axe @aat', async ({page}) => {
46+
await visit(page, {
47+
id: story.id,
48+
globals: {
49+
colorScheme: theme,
50+
},
51+
})
52+
await expect(page).toHaveNoViolations()
53+
})
54+
})
55+
}
56+
})
57+
}
11758
})

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

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import React from 'react'
2+
import type {Meta} from '@storybook/react'
3+
import ButtonGroup from './ButtonGroup'
4+
import {Button} from '../Button'
5+
6+
export default {
7+
title: 'Components/ButtonGroup/Dev',
8+
component: ButtonGroup,
9+
} as Meta<typeof ButtonGroup>
10+
11+
export const SxProp = () => (
12+
<ButtonGroup sx={{border: '1px solid red'}}>
13+
<Button>Button 1</Button>
14+
<Button>Button 2</Button>
15+
<Button>Button 3</Button>
16+
</ButtonGroup>
17+
)

‎packages/react/src/ButtonGroup/ButtonGroup.tsx

+17-86
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,24 @@
1-
import styled from 'styled-components'
21
import React from 'react'
3-
import {get} from '../constants'
4-
import sx from '../sx'
5-
import type {ComponentProps} from '../utils/types'
62
import classes from './ButtonGroup.module.css'
7-
import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent'
83
import {clsx} from 'clsx'
9-
import {useFeatureFlag} from '../FeatureFlags'
104
import {FocusKeys, useFocusZone} from '../hooks/useFocusZone'
115
import {useProvidedRefOrCreate} from '../hooks'
126
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
7+
import {defaultSxProp} from '../utils/defaultSxProp'
8+
import Box from '../Box'
9+
import type {SxProp} from '../sx'
1310

14-
const StyledButtonGroup = toggleStyledComponent(
15-
'primer_react_css_modules_ga',
16-
'div',
17-
styled.div`
18-
display: inline-flex;
19-
vertical-align: middle;
20-
isolation: isolate;
21-
22-
&& > *:not([data-loading-wrapper]):is(button, a) {
23-
margin-inline-end: -1px;
24-
position: relative;
25-
border-radius: 0;
26-
27-
:first-of-type {
28-
border-top-left-radius: ${get('radii.2')};
29-
border-bottom-left-radius: ${get('radii.2')};
30-
}
31-
32-
:last-of-type {
33-
border-top-right-radius: ${get('radii.2')};
34-
border-bottom-right-radius: ${get('radii.2')};
35-
}
36-
37-
:focus,
38-
:active,
39-
:hover {
40-
z-index: 1;
41-
}
42-
}
43-
44-
// if child is loading button
45-
[data-loading-wrapper] {
46-
:first-child {
47-
button,
48-
a {
49-
border-top-left-radius: ${get('radii.2')};
50-
border-bottom-left-radius: ${get('radii.2')};
51-
}
52-
}
53-
54-
:last-child {
55-
button,
56-
a {
57-
border-top-right-radius: ${get('radii.2')};
58-
border-bottom-right-radius: ${get('radii.2')};
59-
}
60-
}
61-
}
62-
63-
[data-loading-wrapper] > * {
64-
margin-inline-end: -1px;
65-
position: relative;
66-
border-radius: 0;
67-
68-
:focus,
69-
:active,
70-
:hover {
71-
z-index: 1;
72-
}
73-
}
74-
75-
${sx};
76-
`,
77-
)
78-
79-
export type ButtonGroupProps = ComponentProps<typeof StyledButtonGroup>
11+
export type ButtonGroupProps = {
12+
/** The role of the group */
13+
role?: string
14+
/** className passed in for styling */
15+
className?: string
16+
} & SxProp
8017

8118
const ButtonGroup = React.forwardRef<HTMLElement, ButtonGroupProps>(function ButtonGroup(
82-
{children, className, role, ...rest},
19+
{className, role, sx, ...rest},
8320
forwardRef,
8421
) {
85-
const enabled = useFeatureFlag('primer_react_css_modules_ga')
8622
const buttonRef = useProvidedRefOrCreate(forwardRef as React.RefObject<HTMLDivElement>)
8723

8824
useFocusZone({
@@ -92,18 +28,13 @@ const ButtonGroup = React.forwardRef<HTMLElement, ButtonGroupProps>(function But
9228
focusOutBehavior: 'wrap',
9329
})
9430

95-
return (
96-
<StyledButtonGroup
97-
ref={buttonRef}
98-
className={clsx(className, {
99-
[classes.ButtonGroup]: enabled,
100-
})}
101-
role={role}
102-
{...rest}
103-
>
104-
{children}
105-
</StyledButtonGroup>
106-
)
31+
if (sx !== defaultSxProp) {
32+
return (
33+
<Box as="div" className={clsx(className, classes.ButtonGroup)} role={role} {...rest} sx={sx} ref={buttonRef} />
34+
)
35+
}
36+
37+
return <div ref={buttonRef} className={clsx(className, classes.ButtonGroup)} role={role} {...rest} />
10738
}) as PolymorphicForwardRefComponent<'div', ButtonGroupProps>
10839

10940
ButtonGroup.displayName = 'ButtonGroup'

0 commit comments

Comments
 (0)
Please sign in to comment.