Revert ButtonGroup changes from last release 37.7.0

e2e/components/ButtonGroup.test.ts

@@ -2,57 +2,116 @@ import {test, expect} from '@playwright/test'
 import {visit} from '../test-helpers/storybook'
 import {themes} from '../test-helpers/themes'
 
-const stories = [
-  {
-    title: 'Default',
-    id: 'components-buttongroup--default',
-  },
-  {
-    title: 'Playground',
-    id: 'components-buttongroup--playground',
-  },
-  {
-    title: 'Icon Buttons',
-    id: 'components-buttongroup-features--icon-buttons',
-  },
-  {
-    title: 'As Toolbar',
-    id: 'components-buttongroup-features--as-toolbar',
-  },
-  {
-    title: 'SX Prop',
-    id: 'components-buttongroup-dev--sx-prop',
-  },
-] as const
-
 test.describe('ButtonGroup', () => {
-  for (const story of stories) {
-    test.describe(story.title, () => {
-      for (const theme of themes) {
-        test.describe(theme, () => {
-          test('@vrt', async ({page}) => {
-            await visit(page, {
-              id: story.id,
-              globals: {
-                colorScheme: theme,
-              },
-            })
-
-            // Default state
-            expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.${story.title}.${theme}.png`)
-          })
-
-          test('axe @aat', async ({page}) => {
-            await visit(page, {
-              id: story.id,
-              globals: {
-                colorScheme: theme,
-              },
-            })
-            await expect(page).toHaveNoViolations()
-          })
-        })
-      }
-    })
-  }
+  test.describe('Default', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup--default',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Default.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup--default',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Playground', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup--playground',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Playground.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup--playground',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Icon Buttons', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--icon-buttons',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Icon Buttons.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--icon-buttons',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('As Toolbar', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--as-toolbar',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.As Toolbar.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--as-toolbar',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
 })

examples/app-router/package.json

@@ -10,7 +10,7 @@
     "type-check": "tsc --noEmit"
   },
   "dependencies": {
-    "@primer/react": "37.7.0",
+    "@primer/react": "37.7.1",
     "next": "^14.2.10",
     "react": "^18.3.1",
     "react-dom": "^18.3.1",

examples/codesandbox/package.json

@@ -20,7 +20,7 @@
     "@typescript-eslint/eslint-plugin": "^7.11.0",
     "@typescript-eslint/parser": "^7.3.1",
     "@vitejs/plugin-react": "^4.3.3",
-    "@primer/react": "37.7.0",
+    "@primer/react": "37.7.1",
     "eslint": "^8.56.0",
     "eslint-plugin-react-hooks": "^4.6.0",
     "eslint-plugin-react-refresh": "^0.4.7",

examples/theming/package.json

@@ -11,7 +11,7 @@
   },
   "dependencies": {
     "@primer/octicons-react": "^19.9.0",
-    "@primer/react": "37.7.0",
+    "@primer/react": "37.7.1",
     "clsx": "^1.2.1",
     "next": "^14.2.10",
     "react": "^18.3.1",

packages/react/CHANGELOG.md

@@ -1,5 +1,11 @@
 # @primer/react
 
+## 37.7.1
+
+### Patch Changes
+
+- [#5412](https://github.com/primer/react/pull/5412) [`7d195fc`](https://github.com/primer/react/commit/7d195fc7c60e5d480e28e71928ce72e152703a48) Thanks [@jonrohan](https://github.com/jonrohan)! - Revert changes to ButtonGroup component
+
 ## 37.7.0
 
 ### Minor Changes

packages/react/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@primer/react",
-  "version": "37.7.0",
+  "version": "37.7.1",
   "description": "An implementation of GitHub's Primer Design System using React",
   "main": "lib/index.js",
   "module": "lib-esm/index.js",

packages/react/src/ButtonGroup/ButtonGroup.module.css

@@ -3,18 +3,18 @@
   vertical-align: middle;
   isolation: isolate;
 
-  & > *:not([data-loading-wrapper]):is(button, a) {
+  & > *:not([data-loading-wrapper]) {
     /* stylelint-disable-next-line primer/spacing */
     margin-inline-end: -1px;
     position: relative;
     border-radius: 0;
 
-    &:first-of-type {
+    &:first-child {
       border-top-left-radius: var(--borderRadius-medium);
       border-bottom-left-radius: var(--borderRadius-medium);
     }
 
-    &:last-of-type {
+    &:last-child {
       border-top-right-radius: var(--borderRadius-medium);
       border-bottom-right-radius: var(--borderRadius-medium);
     }

packages/react/src/ButtonGroup/ButtonGroup.tsx

@@ -1,24 +1,88 @@
+import styled from 'styled-components'
 import React from 'react'
+import {get} from '../constants'
+import sx from '../sx'
+import type {ComponentProps} from '../utils/types'
 import classes from './ButtonGroup.module.css'
+import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent'
 import {clsx} from 'clsx'
+import {useFeatureFlag} from '../FeatureFlags'
 import {FocusKeys, useFocusZone} from '../hooks/useFocusZone'
 import {useProvidedRefOrCreate} from '../hooks'
 import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
-import {defaultSxProp} from '../utils/defaultSxProp'
-import Box from '../Box'
-import type {SxProp} from '../sx'
 
-export type ButtonGroupProps = {
-  /** The role of the group */
-  role?: string
-  /** className passed in for styling */
-  className?: string
-} & SxProp
+const StyledButtonGroup = toggleStyledComponent(
+  'primer_react_css_modules_ga',
+  'div',
+  styled.div`
+    display: inline-flex;
+    vertical-align: middle;
+    isolation: isolate;
+
+    && > *:not([data-loading-wrapper]) {
+      margin-inline-end: -1px;
+      position: relative;
+      border-radius: 0;
+
+      :first-child {
+        border-top-left-radius: ${get('radii.2')};
+        border-bottom-left-radius: ${get('radii.2')};
+      }
+
+      :last-child {
+        border-top-right-radius: ${get('radii.2')};
+        border-bottom-right-radius: ${get('radii.2')};
+      }
+
+      :focus,
+      :active,
+      :hover {
+        z-index: 1;
+      }
+    }
+
+    // if child is loading button
+    [data-loading-wrapper] {
+      :first-child {
+        button,
+        a {
+          border-top-left-radius: ${get('radii.2')};
+          border-bottom-left-radius: ${get('radii.2')};
+        }
+      }
+
+      :last-child {
+        button,
+        a {
+          border-top-right-radius: ${get('radii.2')};
+          border-bottom-right-radius: ${get('radii.2')};
+        }
+      }
+    }
+
+    [data-loading-wrapper] > * {
+      margin-inline-end: -1px;
+      position: relative;
+      border-radius: 0;
+
+      :focus,
+      :active,
+      :hover {
+        z-index: 1;
+      }
+    }
+
+    ${sx};
+  `,
+)
+
+export type ButtonGroupProps = ComponentProps<typeof StyledButtonGroup>
 
 const ButtonGroup = React.forwardRef<HTMLElement, ButtonGroupProps>(function ButtonGroup(
-  {className, role, sx, ...rest},
+  {children, className, role, ...rest},
   forwardRef,
 ) {
+  const enabled = useFeatureFlag('primer_react_css_modules_ga')
   const buttonRef = useProvidedRefOrCreate(forwardRef as React.RefObject<HTMLDivElement>)
 
   useFocusZone({
@@ -28,13 +92,18 @@ const ButtonGroup = React.forwardRef<HTMLElement, ButtonGroupProps>(function But
     focusOutBehavior: 'wrap',
   })
 
-  if (sx !== defaultSxProp) {
-    return (
-      <Box as="div" className={clsx(className, classes.ButtonGroup)} role={role} {...rest} sx={sx} ref={buttonRef} />
-    )
-  }
-
-  return <div ref={buttonRef} className={clsx(className, classes.ButtonGroup)} role={role} {...rest} />
+  return (
+    <StyledButtonGroup
+      ref={buttonRef}
+      className={clsx(className, {
+        [classes.ButtonGroup]: enabled,
+      })}
+      role={role}
+      {...rest}
+    >
+      {children}
+    </StyledButtonGroup>
+  )
 }) as PolymorphicForwardRefComponent<'div', ButtonGroupProps>
 
 ButtonGroup.displayName = 'ButtonGroup'