Merge 7d195fc into d48afbc

.changeset/five-papayas-turn.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Revert changes to ButtonGroup component

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()
+        })
+      })
+    }
+  })
 })

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'