ButtonGroup: Fix button and icon button styling when tooltips are use…

…d on them (#4210)

* fix button group styling for tooltip and tooltip v2

* Add changeset

* take the a tag into account when styling

* add e2e tests for the buttons groups that use tooltip

* test(vrt): update snapshots

---------

Co-authored-by: broccolinisoup <broccolinisoup@users.noreply.github.com>

.changeset/wild-bears-pay.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+ButtonGroup: Fix button and icon button styling when tooltips are used on them

e2e/components/ButtonGroup.test.ts

@@ -86,4 +86,207 @@ test.describe('ButtonGroup', () => {
       })
     }
   })
+  test.describe('Button Group With Tooltip', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--button-group-with-tooltip',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          await page.keyboard.press('Tab')
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ButtonGroup.Button Group With Tooltip.${theme}.png`,
+            {
+              threshold: 0.1,
+            },
+          )
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--button-group-with-tooltip',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Button Group With Tooltip 2', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--button-group-with-tooltip-2',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          await page.keyboard.press('Tab')
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ButtonGroup.Button Group With Tooltip 2.${theme}.png`,
+            {
+              threshold: 0.1,
+            },
+          )
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--button-group-with-tooltip-2',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Icon Buttons With Tooltip', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--icon-buttons-with-tooltip',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          await page.keyboard.press('Tab')
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ButtonGroup.Icon Buttons With Tooltip.${theme}.png`,
+            {
+              threshold: 0.1,
+            },
+          )
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--icon-buttons-with-tooltip',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Icon Buttons With Tooltip 2', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--icon-buttons-with-tooltip-2',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          await page.keyboard.press('Tab')
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ButtonGroup.Icon Buttons With Tooltip 2.${theme}.png`,
+            {
+              threshold: 0.1,
+            },
+          )
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--icon-buttons-with-tooltip-2',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Links With Tooltip', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--links-with-tooltip',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          await page.keyboard.press('Tab')
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ButtonGroup.Links With Tooltip.${theme}.png`,
+            {
+              threshold: 0.1,
+            },
+          )
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--links-with-tooltip',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
+
+  test.describe('Links With Tooltip 2', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--links-with-tooltip-2',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          await page.keyboard.press('Tab')
+          expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot(
+            `ButtonGroup.Links With Tooltip 2.${theme}.png`,
+            {
+              threshold: 0.1,
+            },
+          )
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-buttongroup-features--links-with-tooltip-2',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
 })

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

@@ -1,9 +1,10 @@
 import React from 'react'
 import {ComponentMeta} from '@storybook/react'
 import ButtonGroup from './ButtonGroup'
-import {IconButton} from '../Button'
+import {Button, IconButton} from '../Button'
 import {PlusIcon, DashIcon} from '@primer/octicons-react'
-
+import {Tooltip as TooltipV2} from '../drafts'
+import {Tooltip} from '..'
 export default {
   title: 'Components/ButtonGroup/Features',
   component: ButtonGroup,
@@ -15,3 +16,73 @@ export const IconButtons = () => (
     <IconButton icon={DashIcon} aria-label="Subtract" />
   </ButtonGroup>
 )
+
+export const IconButtonsWithTooltip = () => (
+  <ButtonGroup>
+    <Tooltip text="Add" direction="s">
+      <IconButton icon={PlusIcon} aria-label="Add" />
+    </Tooltip>
+    <Tooltip text="Subtract" direction="s">
+      <IconButton icon={DashIcon} aria-label="Subtract" />
+    </Tooltip>
+  </ButtonGroup>
+)
+
+export const ButtonGroupWithTooltip = () => (
+  <ButtonGroup>
+    <Tooltip text="Add" direction="s">
+      <Button>Add</Button>
+    </Tooltip>
+    <Tooltip text="Subtract" direction="s">
+      <Button>Subtract</Button>
+    </Tooltip>
+  </ButtonGroup>
+)
+
+export const LinksWithTooltip = () => (
+  <ButtonGroup>
+    <Tooltip text="Add" direction="s">
+      <Button as="a" href="https://primer.style">
+        Add
+      </Button>
+    </Tooltip>
+    <Tooltip text="Subtract" direction="s">
+      <Button as="a" href="https://primer.style">
+        Subtract
+      </Button>
+    </Tooltip>
+  </ButtonGroup>
+)
+
+export const IconButtonsWithTooltip2 = () => (
+  <ButtonGroup>
+    <TooltipV2 text="Add" type="label">
+      <IconButton icon={PlusIcon} aria-label="Add" />
+    </TooltipV2>
+    <TooltipV2 text="Subtract" type="label">
+      <IconButton icon={DashIcon} aria-label="Subtract" />
+    </TooltipV2>
+  </ButtonGroup>
+)
+
+export const LinksWithTooltip2 = () => (
+  <ButtonGroup>
+    <TooltipV2 text="Add" type="label">
+      <IconButton as="a" icon={PlusIcon} aria-label="Add" href="https://primer.style" />
+    </TooltipV2>
+    <TooltipV2 text="Subtract" type="label">
+      <IconButton as="a" icon={DashIcon} aria-label="Subtract" href="https://primer.style" />
+    </TooltipV2>
+  </ButtonGroup>
+)
+
+export const ButtonGroupWithTooltip2 = () => (
+  <ButtonGroup>
+    <TooltipV2 text="Add">
+      <Button>Add</Button>
+    </TooltipV2>
+    <TooltipV2 text="Subtract">
+      <Button>Subtract</Button>
+    </TooltipV2>
+  </ButtonGroup>
+)

packages/react/src/ButtonGroup/ButtonGroup.tsx

@@ -8,28 +8,32 @@ const ButtonGroup = styled.div`
   vertical-align: middle;
   isolation: isolate;
 
-  && > * {
+  && > [type='button'],
+  /* this specific selection is required to cover for tooltip. Because the buttons are wrapped with the tooltip span */
+  && > span[role='tooltip'] > [type='button'] {
     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;
     }
   }
 
+  && > [type='button']:first-child,
+  && > span[role='tooltip']:first-of-type > [type='button'] {
+    border-top-left-radius: ${get('radii.2')};
+    border-bottom-left-radius: ${get('radii.2')};
+  }
+
+  && > [type='button']:last-of-type,
+  && > span[role='tooltip']:last-of-type > [type='button'] {
+    border-top-right-radius: ${get('radii.2')};
+    border-bottom-right-radius: ${get('radii.2')};
+  }
+
   ${sx};
 `
 

script/generate-e2e-tests.js

@@ -273,6 +273,31 @@ const components = new Map([
           id: 'components-buttongroup-features--icon-buttons',
           name: 'Icon Buttons',
         },
+        {
+          id: 'components-buttongroup-features--button-group-with-tooltip',
+          name: 'Button Group With Tooltip',
+        },
+
+        {
+          id: 'components-buttongroup-features--button-group-with-tooltip-2',
+          name: 'Button Group With Tooltip 2',
+        },
+        {
+          id: 'components-buttongroup-features--icon-buttons-with-tooltip',
+          name: 'Icon Buttons With Tooltip',
+        },
+        {
+          id: 'components-buttongroup-features--icon-buttons-with-tooltip-2',
+          name: 'Icon Buttons With Tooltip 2',
+        },
+        {
+          id: 'components-buttongroup-features--links-with-tooltip',
+          name: 'Links With Tooltip',
+        },
+        {
+          id: 'components-buttongroup-features--links-with-tooltip-2',
+          name: 'Links With Tooltip 2',
+        },
       ],
     },
   ],