diff --git a/.changeset/clean-camels-train.md b/.changeset/clean-camels-train.md new file mode 100644 index 00000000000..ee88bd04f80 --- /dev/null +++ b/.changeset/clean-camels-train.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Bug fix: ButtonGroup with Tooltip border-radius is incorrect diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-colorblind-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-colorblind-linux.png new file mode 100644 index 00000000000..3731c61b332 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-dimmed-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-dimmed-linux.png new file mode 100644 index 00000000000..251f7e14587 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-high-contrast-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-high-contrast-linux.png new file mode 100644 index 00000000000..9d830ba5a2f Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-linux.png new file mode 100644 index 00000000000..3731c61b332 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-tritanopia-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-tritanopia-linux.png new file mode 100644 index 00000000000..3731c61b332 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-colorblind-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-colorblind-linux.png new file mode 100644 index 00000000000..3c5e07833c7 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-high-contrast-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-high-contrast-linux.png new file mode 100644 index 00000000000..8b6679befe6 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-linux.png new file mode 100644 index 00000000000..3c5e07833c7 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-linux.png differ diff --git a/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-tritanopia-linux.png b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-tritanopia-linux.png new file mode 100644 index 00000000000..3c5e07833c7 Binary files /dev/null and b/.playwright/snapshots/components/ButtonGroup.test.ts-snapshots/ButtonGroup-Overrides-light-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-colorblind-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-colorblind-linux.png index 15c7c89e0fc..d0b8895bc4e 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-colorblind-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-dimmed-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-dimmed-linux.png index 38bb73c0bba..2418e2d7aeb 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-dimmed-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-high-contrast-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-high-contrast-linux.png index 7390ebf1e9c..4bdb6143f3e 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-high-contrast-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-linux.png index bc2921e4ba2..2d52f44b407 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-tritanopia-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-tritanopia-linux.png index 15c7c89e0fc..d0b8895bc4e 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-tritanopia-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-colorblind-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-colorblind-linux.png index 83c6dd9acf8..097370785f2 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-colorblind-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-high-contrast-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-high-contrast-linux.png index 2a7d1ad9849..dc876d486e2 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-high-contrast-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-linux.png index 61e5794f0c7..8acf800b33b 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-tritanopia-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-tritanopia-linux.png index afe9e626188..097370785f2 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-tritanopia-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Memex-Nested-Overlays-light-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-dark-high-contrast-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-dark-high-contrast-linux.png index 6fc363ea5cc..ae8fa337c2b 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-dark-high-contrast-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-light-high-contrast-linux.png b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-light-high-contrast-linux.png index 5de6ccd568d..700fcabb70a 100644 Binary files a/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-light-high-contrast-linux.png and b/.playwright/snapshots/components/Overlay.test.ts-snapshots/Overlay-Nested-Overlays-light-high-contrast-linux.png differ diff --git a/e2e/components/ButtonGroup.test.ts b/e2e/components/ButtonGroup.test.ts index 1a9eab686b9..243f070df7f 100644 --- a/e2e/components/ButtonGroup.test.ts +++ b/e2e/components/ButtonGroup.test.ts @@ -114,4 +114,32 @@ test.describe('ButtonGroup', () => { }) } }) + + test.describe('Overrides', () => { + for (const theme of themes) { + test.describe(theme, () => { + test('default @vrt', async ({page}) => { + await visit(page, { + id: 'components-buttongroup-devonly--link-button-with-icon-buttons', + globals: { + colorScheme: theme, + }, + }) + + // Default state + expect(await page.screenshot()).toMatchSnapshot(`ButtonGroup.Overrides.${theme}.png`) + }) + + test('axe @aat', async ({page}) => { + await visit(page, { + id: 'components-buttongroup-devonly--link-button-with-icon-buttons', + globals: { + colorScheme: theme, + }, + }) + await expect(page).toHaveNoViolations() + }) + }) + } + }) }) diff --git a/packages/react/src/ButtonGroup/ButtonGroup.dev.stories.tsx b/packages/react/src/ButtonGroup/ButtonGroup.dev.stories.tsx new file mode 100644 index 00000000000..c0d2f2645b7 --- /dev/null +++ b/packages/react/src/ButtonGroup/ButtonGroup.dev.stories.tsx @@ -0,0 +1,65 @@ +import React from 'react' +import type {Meta} from '@storybook/react' +import ButtonGroup from './ButtonGroup' +import {Button, IconButton, LinkButton} from '../Button' +import {CopilotIcon} from '@primer/octicons-react' +import {Box, Tooltip, ThemeProvider, BaseStyles} from '..' + +const meta: Meta = { + title: 'Components/ButtonGroup/DevOnly', + component: ButtonGroup, + decorators: [ + Story => { + // Add some padding to the wrapper box to make sure tooltip v1 is always in the viewport + return ( + + + {Story()} + + + ) + }, + ], +} + +export default meta + +export const LinkAndButtonWithTooltip2 = () => ( + + + + + + +) + +export const ButtonAndLinkWithTooltip2 = () => ( + + + + + + +) + +export const ButtonGroupSingleButton = () => ( + + +
+
+) + +export const LinkButtonWithIconButtons = () => ( + + + Small link + + + + + +) diff --git a/packages/react/src/ButtonGroup/ButtonGroup.features.stories.tsx b/packages/react/src/ButtonGroup/ButtonGroup.features.stories.tsx index 211e09ad17e..da2b99ff6e6 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.features.stories.tsx +++ b/packages/react/src/ButtonGroup/ButtonGroup.features.stories.tsx @@ -1,10 +1,11 @@ -import React, {useState} from 'react' +import React from 'react' import type {Meta} from '@storybook/react' import ButtonGroup from './ButtonGroup' import {IconButton, Button} from '../Button' import {PlusIcon, DashIcon, TriangleDownIcon} from '@primer/octicons-react' import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' +import {Tooltip} from '../next' export default { title: 'Components/ButtonGroup/Features', @@ -14,26 +15,84 @@ export default { export const IconButtons = () => ( {/* We can remove these unsafe props after we resolve https://github.com/primer/react/issues/4129 */} - {/* eslint-disable-next-line primer-react/a11y-remove-disable-tooltip */} - - {/* eslint-disable-next-line primer-react/a11y-remove-disable-tooltip */} - + + ) export const LoadingButtons = () => { - const [isLoading, setIsLoading] = useState(false) - - const handleClick = () => { - setIsLoading(true) - } + const handleClick = () => {} return ( - - + + + + + ) +} + +export const IconButtonsWithTooltip = () => ( + + + + +) +export const ButtonAndLink = () => ( + + + + +) + +export const LinksWithTooltip = () => ( + + + + + + + + +) + +export const InactiveButtonsGroup = () => { + const primaryButton = ( + + ) + + const secondaryButton = ( + + ) + + return ( + + + {primaryButton} + + + {}}> + + + {secondaryButton} + + + + Item 1 + Item 2 + + ) } diff --git a/packages/react/src/ButtonGroup/ButtonGroup.module.css b/packages/react/src/ButtonGroup/ButtonGroup.module.css index 56e5b808a2a..7c289b69544 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.module.css +++ b/packages/react/src/ButtonGroup/ButtonGroup.module.css @@ -7,16 +7,27 @@ /* stylelint-disable-next-line primer/spacing */ margin-inline-end: -1px; position: relative; - border-radius: 0; + + /* reset border-radius */ + button, + a { + border-radius: 0; + } &:first-child { - border-top-left-radius: var(--borderRadius-medium); - border-bottom-left-radius: var(--borderRadius-medium); + button, + a { + border-top-left-radius: var(--borderRadius-medium); + border-bottom-left-radius: var(--borderRadius-medium); + } } &:last-child { - border-top-right-radius: var(--borderRadius-medium); - border-bottom-right-radius: var(--borderRadius-medium); + button, + a { + border-top-right-radius: var(--borderRadius-medium); + border-bottom-right-radius: var(--borderRadius-medium); + } } &:focus, @@ -26,8 +37,32 @@ } } + /* this is a workaround until portal based tooltips are fully removed from dotcom */ + &:has(div:last-child:empty) { + button, + a { + border-radius: var(--borderRadius-medium); + } + } + /* if child is loading button */ - [data-loading-wrapper] { + & > *[data-loading-wrapper] { + /* stylelint-disable-next-line primer/spacing */ + margin-inline-end: -1px; + position: relative; + + /* reset border-radius */ + button, + a { + border-radius: 0; + } + + &:focus, + &:active, + &:hover { + z-index: 1; + } + &:first-child { button, a { @@ -44,17 +79,4 @@ } } } - - [data-loading-wrapper] > * { - /* stylelint-disable-next-line primer/spacing */ - margin-inline-end: -1px; - position: relative; - border-radius: 0; - - &:focus, - &:active, - &:hover { - z-index: 1; - } - } } diff --git a/packages/react/src/ButtonGroup/ButtonGroup.tsx b/packages/react/src/ButtonGroup/ButtonGroup.tsx index c901875909b..e3cc3e0632a 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.tsx +++ b/packages/react/src/ButtonGroup/ButtonGroup.tsx @@ -1,6 +1,5 @@ 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' @@ -19,57 +18,79 @@ const StyledButtonGroup = toggleStyledComponent( vertical-align: middle; isolation: isolate; - && > *:not([data-loading-wrapper]) { + & > *:not([data-loading-wrapper]) { + /* stylelint-disable-next-line primer/spacing */ 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')}; + /* reset border-radius */ + button, + a { + border-radius: 0; } - :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 { + &:first-child { button, a { - border-top-left-radius: ${get('radii.2')}; - border-bottom-left-radius: ${get('radii.2')}; + border-top-left-radius: var(--borderRadius-medium); + border-bottom-left-radius: var(--borderRadius-medium); } } - :last-child { + &:last-child { button, a { - border-top-right-radius: ${get('radii.2')}; - border-bottom-right-radius: ${get('radii.2')}; + border-top-right-radius: var(--borderRadius-medium); + border-bottom-right-radius: var(--borderRadius-medium); } } + + &:focus, + &:active, + &:hover { + z-index: 1; + } + } + + /* this is a workaround until portal based tooltips are fully removed from dotcom */ + &:has(div:last-child:empty) { + button, + a { + border-radius: var(--borderRadius-medium); + } } - [data-loading-wrapper] > * { + /* if child is loading button */ + & > *[data-loading-wrapper] { + /* stylelint-disable-next-line primer/spacing */ margin-inline-end: -1px; position: relative; - border-radius: 0; + /* reset border-radius */ + button, + a { + border-radius: 0; + } - :focus, - :active, - :hover { + &:focus, + &:active, + &:hover { z-index: 1; } + &:first-child { + button, + a { + border-top-left-radius: var(--borderRadius-medium); + border-bottom-left-radius: var(--borderRadius-medium); + } + } + + &:last-child { + button, + a { + border-top-right-radius: var(--borderRadius-medium); + border-bottom-right-radius: var(--borderRadius-medium); + } + } } ${sx}; @@ -83,6 +104,7 @@ const ButtonGroup = React.forwardRef(function But forwardRef, ) { const enabled = useFeatureFlag('primer_react_css_modules_ga') + const buttons = React.Children.map(children, (child, index) =>
{child}
) const buttonRef = useProvidedRefOrCreate(forwardRef as React.RefObject) useFocusZone({ @@ -101,7 +123,7 @@ const ButtonGroup = React.forwardRef(function But role={role} {...rest} > - {children} + {buttons} ) }) as PolymorphicForwardRefComponent<'div', ButtonGroupProps> diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index 43644bac051..09e9ac7c35a 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -301,11 +301,17 @@ export const Tooltip = React.forwardRef( // Has trigger element or any of its children interactive elements? const isTriggerInteractive = isInteractive(triggerRef.current) const triggerChildren = triggerRef.current.childNodes - const hasInteractiveChild = Array.from(triggerChildren).some(child => { - return child instanceof HTMLElement && isInteractive(child) + // two levels deep + const hasInteractiveDescendant = Array.from(triggerChildren).some(child => { + return ( + (child instanceof HTMLElement && isInteractive(child)) || + Array.from(child.childNodes).some( + grandChild => grandChild instanceof HTMLElement && isInteractive(grandChild), + ) + ) }) invariant( - isTriggerInteractive || hasInteractiveChild, + isTriggerInteractive || hasInteractiveDescendant, 'The `Tooltip` component expects a single React element that contains interactive content. Consider using a ` + + + + , + ) + + const triggerEL = getByText('Button 1') + expect(triggerEL).toBeInTheDocument() + }) })