Skip to content

Commit

Permalink
Revert ButtonGroup changes from last release 37.7.0 (#5412)
Browse files Browse the repository at this point in the history
* Revert "chore(ButtonGroup): Remove the CSS modules feature flag from ButtonGroup (#5339)"

This reverts commit dc2f083.

* Revert "BugFix: Use first-of-type for buttongroup selector (#5343)"

This reverts commit 70005b8.

* Add changeset

* @primer/react v37.7.1
  • Loading branch information
jonrohan authored Dec 11, 2024
1 parent 5e0deac commit d9e62e9
Show file tree
Hide file tree
Showing 29 changed files with 210 additions and 93 deletions.
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.
Binary file not shown.
Binary file not shown.
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.
163 changes: 111 additions & 52 deletions e2e/components/ButtonGroup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
}
})
})
2 changes: 1 addition & 1 deletion examples/app-router/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion examples/codesandbox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion examples/theming/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions packages/react/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/react/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
17 changes: 0 additions & 17 deletions packages/react/src/ButtonGroup/ButtonGroup.dev.stories.tsx

This file was deleted.

6 changes: 3 additions & 3 deletions packages/react/src/ButtonGroup/ButtonGroup.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
103 changes: 86 additions & 17 deletions packages/react/src/ButtonGroup/ButtonGroup.tsx
Original file line number Diff line number Diff line change
@@ -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({
Expand All @@ -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'
Expand Down

0 comments on commit d9e62e9

Please sign in to comment.