diff --git a/.changeset/old-experts-applaud.md b/.changeset/old-experts-applaud.md
new file mode 100644
index 00000000000..12c5b68ed33
--- /dev/null
+++ b/.changeset/old-experts-applaud.md
@@ -0,0 +1,7 @@
+---
+'@primer/react': patch
+---
+
+- Fixes `role` and keyboard behavior for SegmentedControl.
+- Fixes a bug where icon-only SegmentedControl buttons did not fill the parent width when the `fullWidth` prop was set
+- Fixes a bug where click handlers were not passed correctly when the responsive variant was set to `'hideLabels'`
diff --git a/src/SegmentedControl/SegmentedControl.test.tsx b/src/SegmentedControl/SegmentedControl.test.tsx
index 3a0a094b008..1129507e80e 100644
--- a/src/SegmentedControl/SegmentedControl.test.tsx
+++ b/src/SegmentedControl/SegmentedControl.test.tsx
@@ -197,55 +197,6 @@ describe('SegmentedControl', () => {
expect(handleClick).toHaveBeenCalled()
})
- it('focuses the selected button first', async () => {
- const user = userEvent.setup()
- const {getByRole} = render(
- <>
-
-
- {segmentData.map(({label, id}, index) => (
-
- {label}
-
- ))}
-
- >
- )
- const initialFocusButtonNode = getByRole('button', {name: segmentData[1].label})
-
- expect(document.activeElement?.id).not.toEqual(initialFocusButtonNode.id)
-
- await user.tab() // focus the button before the segmented control
- await user.tab() // move focus into the segmented control
-
- expect(document.activeElement?.id).toEqual(initialFocusButtonNode.id)
- })
-
- it('focuses the previous button when keying ArrowLeft, and the next button when keying ArrowRight', () => {
- const {getByRole} = render(
-
- {segmentData.map(({label, id}, index) => (
-
- {label}
-
- ))}
-
- )
- const initialFocusButtonNode = getByRole('button', {name: segmentData[1].label})
- const nextFocusButtonNode = getByRole('button', {name: segmentData[0].label})
-
- expect(document.activeElement?.id).not.toEqual(nextFocusButtonNode.id)
-
- fireEvent.focus(initialFocusButtonNode)
- fireEvent.keyDown(initialFocusButtonNode, {key: 'ArrowLeft'})
-
- expect(document.activeElement?.id).toEqual(nextFocusButtonNode.id)
-
- fireEvent.keyDown(initialFocusButtonNode, {key: 'ArrowRight'})
-
- expect(document.activeElement?.id).toEqual(initialFocusButtonNode.id)
- })
-
it('calls onChange with index of clicked segment button when using the dropdown variant', async () => {
act(() => {
matchMedia.useMediaQuery(viewportRanges.narrow)
diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx
index 9336c0097dc..2a7e054336e 100644
--- a/src/SegmentedControl/SegmentedControl.tsx
+++ b/src/SegmentedControl/SegmentedControl.tsx
@@ -1,14 +1,19 @@
-import React, {RefObject, useRef} from 'react'
+import React, {useRef} from 'react'
import Button, {SegmentedControlButtonProps} from './SegmentedControlButton'
import SegmentedControlIconButton, {SegmentedControlIconButtonProps} from './SegmentedControlIconButton'
-import {ActionList, ActionMenu, Box, useTheme} from '..'
-import {merge, SxProp} from '../sx'
+import {ActionList, ActionMenu, useTheme} from '..'
+import sx, {merge, SxProp} from '../sx'
import {ResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue'
import {ViewportRangeKeys} from '../utils/types/ViewportRangeKeys'
-import {FocusKeys, FocusZoneHookSettings, useFocusZone} from '../hooks/useFocusZone'
+import styled from 'styled-components'
type WidthOnlyViewportRangeKeys = Exclude
+// Needed because passing a ref to `Box` causes a type error
+const SegmentedControlList = styled.ul`
+ ${sx};
+`
+
type SegmentedControlProps = {
'aria-label'?: string
'aria-labelledby'?: string
@@ -28,7 +33,10 @@ const getSegmentedControlStyles = (isFullWidth?: boolean) => ({
borderStyle: 'solid',
borderWidth: 1,
display: isFullWidth ? 'flex' : 'inline-flex',
- height: '32px' // TODO: use primitive `control.medium.size` when it is available
+ height: '32px', // TODO: use primitive `control.medium.size` when it is available
+ margin: 0,
+ padding: 0,
+ width: isFullWidth ? '100%' : undefined
})
const Root: React.FC> = ({
@@ -41,7 +49,7 @@ const Root: React.FC> = ({
variant,
...rest
}) => {
- const segmentedControlContainerRef = useRef(null)
+ const segmentedControlContainerRef = useRef(null)
const {theme} = useTheme()
const responsiveVariant = useResponsiveValue(variant, 'default')
const isFullWidth = useResponsiveValue(fullWidth, false)
@@ -74,26 +82,7 @@ const Root: React.FC> = ({
return React.isValidElement(childArg) ? childArg.props['aria-label'] : null
}
- const sx = merge(getSegmentedControlStyles(isFullWidth), sxProp as SxProp)
-
- const focusInStrategy: FocusZoneHookSettings['focusInStrategy'] = () => {
- if (segmentedControlContainerRef.current) {
- // we need to use type assertion because querySelector returns "Element", not "HTMLElement"
- type SelectedButton = HTMLButtonElement | undefined
-
- const selectedButton = segmentedControlContainerRef.current.querySelector(
- 'button[aria-current="true"]'
- ) as SelectedButton
-
- return selectedButton
- }
- }
-
- useFocusZone({
- containerRef: segmentedControlContainerRef,
- bindKeys: FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd,
- focusInStrategy
- })
+ const listSx = merge(getSegmentedControlStyles(isFullWidth), sxProp as SxProp)
if (!ariaLabel && !ariaLabelledby) {
// eslint-disable-next-line no-console
@@ -141,12 +130,11 @@ const Root: React.FC> = ({
) : (
// Render a segmented control
- }
+ ref={segmentedControlContainerRef}
{...rest}
>
{React.Children.map(children, (child, index) => {
@@ -180,6 +168,7 @@ const Root: React.FC> = ({
children: childPropsChildren,
...restChildProps
} = child.props
+ const {sx: sharedSxProp, ...restSharedChildProps} = sharedChildProps
if (!leadingIcon) {
// eslint-disable-next-line no-console
console.warn('A `leadingIcon` prop is required when hiding visible labels')
@@ -190,12 +179,12 @@ const Root: React.FC> = ({
icon={leadingIcon}
sx={
{
- '--separator-color':
- index === selectedIndex || index === selectedIndex - 1
- ? 'transparent'
- : theme?.colors.border.default
+ ...sharedSxProp,
+ // setting width here avoids having to pass `isFullWidth` directly to child components
+ width: !isFullWidth ? '32px' : '100%' // TODO: use primitive `control.medium.size` when it is available instead of '32px'
} as React.CSSProperties
}
+ {...restSharedChildProps}
{...restChildProps}
/>
)
@@ -205,7 +194,7 @@ const Root: React.FC> = ({
// Render the children as-is and add the shared child props
return React.cloneElement(child, sharedChildProps)
})}
-
+
)
}
diff --git a/src/SegmentedControl/SegmentedControlButton.tsx b/src/SegmentedControl/SegmentedControlButton.tsx
index f2054158625..44ec9716a37 100644
--- a/src/SegmentedControl/SegmentedControlButton.tsx
+++ b/src/SegmentedControl/SegmentedControlButton.tsx
@@ -3,7 +3,7 @@ import {IconProps} from '@primer/octicons-react'
import styled from 'styled-components'
import {Box} from '..'
import sx, {merge, SxProp} from '../sx'
-import {getSegmentedControlButtonStyles} from './getSegmentedControlStyles'
+import {getSegmentedControlButtonStyles, getSegmentedControlListItemStyles} from './getSegmentedControlStyles'
export type SegmentedControlButtonProps = {
/** The visible label rendered in the button */
@@ -26,19 +26,25 @@ const SegmentedControlButton: React.FC {
- const mergedSx = merge(getSegmentedControlButtonStyles({selected, children}), sxProp as SxProp)
+ const mergedSx = merge(getSegmentedControlListItemStyles(), sxProp as SxProp)
return (
-
-
- {LeadingIcon && (
-
-
-
- )}
- {children}
-
-
+
+
+
+ {LeadingIcon && (
+
+
+
+ )}
+ {children}
+
+
+
)
}
diff --git a/src/SegmentedControl/SegmentedControlIconButton.tsx b/src/SegmentedControl/SegmentedControlIconButton.tsx
index 1d617151823..2fd0225c5fc 100644
--- a/src/SegmentedControl/SegmentedControlIconButton.tsx
+++ b/src/SegmentedControl/SegmentedControlIconButton.tsx
@@ -2,12 +2,9 @@ import React, {HTMLAttributes} from 'react'
import {IconProps} from '@primer/octicons-react'
import styled from 'styled-components'
import sx, {merge, SxProp} from '../sx'
-import {
- borderedSegment,
- getSegmentedControlButtonStyles,
- directChildLayoutAdjustments
-} from './getSegmentedControlStyles'
+import {getSegmentedControlButtonStyles, getSegmentedControlListItemStyles} from './getSegmentedControlStyles'
import Tooltip from '../Tooltip'
+import Box from '../Box'
export type SegmentedControlIconButtonProps = {
'aria-label': string
@@ -35,26 +32,28 @@ export const SegmentedControlIconButton: React.FC {
- const mergedSx = merge(getSegmentedControlButtonStyles({selected, isIconOnly: true}), sxProp as SxProp)
+ const mergedSx = merge(
+ {
+ width: '32px', // TODO: use primitive `control.medium.size` when it is available
+ ...getSegmentedControlListItemStyles()
+ },
+ sxProp as SxProp
+ )
return (
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
)
}
diff --git a/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap b/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap
index fd92c0bfb14..0dd5f099183 100644
--- a/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap
+++ b/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap
@@ -1,20 +1,77 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`SegmentedControl renders consistently 1`] = `
-.c0 {
- background-color: #eaeef2;
- border-color: #d0d7de;
- border-radius: 6px;
- border-style: solid;
- border-width: 1px;
- display: -webkit-inline-box;
- display: -webkit-inline-flex;
- display: -ms-inline-flexbox;
- display: inline-flex;
+.c1 {
+ display: block;
+ position: relative;
+ -webkit-box-flex: 1;
+ -webkit-flex-grow: 1;
+ -ms-flex-positive: 1;
+ flex-grow: 1;
+ margin-top: -1px;
+ margin-bottom: -1px;
height: 32px;
+ --separator-color: transparent;
}
-.c1 {
+.c1:not(:last-child) {
+ margin-right: 1px;
+}
+
+.c1:not(:last-child):after {
+ background-color: var(--separator-color);
+ content: "";
+ position: absolute;
+ right: -2px;
+ top: 8px;
+ bottom: 8px;
+ width: 1px;
+}
+
+.c1:first-child {
+ margin-left: -1px;
+}
+
+.c1:last-child {
+ margin-right: -1px;
+}
+
+.c3 {
+ display: block;
+ position: relative;
+ -webkit-box-flex: 1;
+ -webkit-flex-grow: 1;
+ -ms-flex-positive: 1;
+ flex-grow: 1;
+ margin-top: -1px;
+ margin-bottom: -1px;
+ height: 32px;
+ --separator-color: #d0d7de;
+}
+
+.c3:not(:last-child) {
+ margin-right: 1px;
+}
+
+.c3:not(:last-child):after {
+ background-color: var(--separator-color);
+ content: "";
+ position: absolute;
+ right: -2px;
+ top: 8px;
+ bottom: 8px;
+ width: 1px;
+}
+
+.c3:first-child {
+ margin-left: -1px;
+}
+
+.c3:last-child {
+ margin-right: -1px;
+}
+
+.c2 {
--segmented-control-button-inner-padding: 12px;
--segmented-control-button-bg-inset: 4px;
--segmented-control-outer-radius: 6px;
@@ -24,21 +81,15 @@ exports[`SegmentedControl renders consistently 1`] = `
border-width: 0;
color: currentColor;
cursor: pointer;
- -webkit-box-flex: 1;
- -webkit-flex-grow: 1;
- -ms-flex-positive: 1;
- flex-grow: 1;
font-family: inherit;
font-size: 14px;
font-weight: 600;
- margin-top: -1px;
- margin-bottom: -1px;
padding: 0;
- position: relative;
- --separator-color: transparent;
+ height: 100%;
+ width: 100%;
}
-.c1 .segmentedControl-content {
+.c2 .segmentedControl-content {
-webkit-align-items: center;
-webkit-box-align: center;
-ms-flex-align: center;
@@ -61,37 +112,15 @@ exports[`SegmentedControl renders consistently 1`] = `
padding-right: var(--segmented-control-button-inner-padding);
}
-.c1 svg {
+.c2 svg {
fill: #57606a;
}
-.c1:first-child {
- margin-left: -1px;
-}
-
-.c1:last-child {
- margin-right: -1px;
-}
-
-.c1:not(:last-child) {
- margin-right: 1px;
-}
-
-.c1:not(:last-child):after {
- background-color: var(--separator-color);
- content: "";
- position: absolute;
- right: -2px;
- top: 8px;
- bottom: 8px;
- width: 1px;
-}
-
-.c1:focus:focus-visible:not(:last-child):after {
+.c2:focus:focus-visible:not(:last-child):after {
width: 0;
}
-.c1 .segmentedControl-text:after {
+.c2 .segmentedControl-text:after {
content: "Preview";
display: block;
font-weight: 600;
@@ -105,7 +134,7 @@ exports[`SegmentedControl renders consistently 1`] = `
visibility: hidden;
}
-.c2 {
+.c4 {
--segmented-control-button-inner-padding: 12px;
--segmented-control-button-bg-inset: 4px;
--segmented-control-outer-radius: 6px;
@@ -115,21 +144,15 @@ exports[`SegmentedControl renders consistently 1`] = `
border-width: 0;
color: currentColor;
cursor: pointer;
- -webkit-box-flex: 1;
- -webkit-flex-grow: 1;
- -ms-flex-positive: 1;
- flex-grow: 1;
font-family: inherit;
font-size: 14px;
font-weight: 400;
- margin-top: -1px;
- margin-bottom: -1px;
padding: var(--segmented-control-button-bg-inset);
- position: relative;
- --separator-color: #d0d7de;
+ height: 100%;
+ width: 100%;
}
-.c2 .segmentedControl-content {
+.c4 .segmentedControl-content {
-webkit-align-items: center;
-webkit-box-align: center;
-ms-flex-align: center;
@@ -152,45 +175,23 @@ exports[`SegmentedControl renders consistently 1`] = `
padding-right: calc(var(--segmented-control-button-inner-padding) - var(--segmented-control-button-bg-inset));
}
-.c2 svg {
+.c4 svg {
fill: #57606a;
}
-.c2:hover .segmentedControl-content {
+.c4:hover .segmentedControl-content {
background-color: rgba(175,184,193,0.2);
}
-.c2:active .segmentedControl-content {
+.c4:active .segmentedControl-content {
background-color: rgba(175,184,193,0.4);
}
-.c2:first-child {
- margin-left: -1px;
-}
-
-.c2:last-child {
- margin-right: -1px;
-}
-
-.c2:not(:last-child) {
- margin-right: 1px;
-}
-
-.c2:not(:last-child):after {
- background-color: var(--separator-color);
- content: "";
- position: absolute;
- right: -2px;
- top: 8px;
- bottom: 8px;
- width: 1px;
-}
-
-.c2:focus:focus-visible:not(:last-child):after {
+.c4:focus:focus-visible:not(:last-child):after {
width: 0;
}
-.c2 .segmentedControl-text:after {
+.c4 .segmentedControl-text:after {
content: "Raw";
display: block;
font-weight: 600;
@@ -204,7 +205,7 @@ exports[`SegmentedControl renders consistently 1`] = `
visibility: hidden;
}
-.c3 {
+.c5 {
--segmented-control-button-inner-padding: 12px;
--segmented-control-button-bg-inset: 4px;
--segmented-control-outer-radius: 6px;
@@ -214,21 +215,15 @@ exports[`SegmentedControl renders consistently 1`] = `
border-width: 0;
color: currentColor;
cursor: pointer;
- -webkit-box-flex: 1;
- -webkit-flex-grow: 1;
- -ms-flex-positive: 1;
- flex-grow: 1;
font-family: inherit;
font-size: 14px;
font-weight: 400;
- margin-top: -1px;
- margin-bottom: -1px;
padding: var(--segmented-control-button-bg-inset);
- position: relative;
- --separator-color: #d0d7de;
+ height: 100%;
+ width: 100%;
}
-.c3 .segmentedControl-content {
+.c5 .segmentedControl-content {
-webkit-align-items: center;
-webkit-box-align: center;
-ms-flex-align: center;
@@ -251,45 +246,23 @@ exports[`SegmentedControl renders consistently 1`] = `
padding-right: calc(var(--segmented-control-button-inner-padding) - var(--segmented-control-button-bg-inset));
}
-.c3 svg {
+.c5 svg {
fill: #57606a;
}
-.c3:hover .segmentedControl-content {
+.c5:hover .segmentedControl-content {
background-color: rgba(175,184,193,0.2);
}
-.c3:active .segmentedControl-content {
+.c5:active .segmentedControl-content {
background-color: rgba(175,184,193,0.4);
}
-.c3:first-child {
- margin-left: -1px;
-}
-
-.c3:last-child {
- margin-right: -1px;
-}
-
-.c3:not(:last-child) {
- margin-right: 1px;
-}
-
-.c3:not(:last-child):after {
- background-color: var(--separator-color);
- content: "";
- position: absolute;
- right: -2px;
- top: 8px;
- bottom: 8px;
- width: 1px;
-}
-
-.c3:focus:focus-visible:not(:last-child):after {
+.c5:focus:focus-visible:not(:last-child):after {
width: 0;
}
-.c3 .segmentedControl-text:after {
+.c5 .segmentedControl-text:after {
content: "Blame";
display: block;
font-weight: 600;
@@ -303,8 +276,23 @@ exports[`SegmentedControl renders consistently 1`] = `
visibility: hidden;
}
+.c0 {
+ background-color: #eaeef2;
+ border-color: #d0d7de;
+ border-radius: 6px;
+ border-style: solid;
+ border-width: 1px;
+ display: -webkit-inline-box;
+ display: -webkit-inline-flex;
+ display: -ms-inline-flexbox;
+ display: inline-flex;
+ height: 32px;
+ margin: 0;
+ padding: 0;
+}
+
@media (pointer:coarse) {
- .c1:before {
+ .c2:before {
content: "";
position: absolute;
left: 0;
@@ -318,7 +306,7 @@ exports[`SegmentedControl renders consistently 1`] = `
}
@media (pointer:coarse) {
- .c2:before {
+ .c4:before {
content: "";
position: absolute;
left: 0;
@@ -332,7 +320,7 @@ exports[`SegmentedControl renders consistently 1`] = `
}
@media (pointer:coarse) {
- .c3:before {
+ .c5:before {
content: "";
position: absolute;
left: 0;
@@ -345,52 +333,63 @@ exports[`SegmentedControl renders consistently 1`] = `
}
}
-
-
-
+
+
+
+
-
-
- Raw
-
-
-
-
+
+
-
-
- Blame
-
-
-
-
+
+ Blame
+
+
+
+
+
`;
diff --git a/src/SegmentedControl/getSegmentedControlStyles.ts b/src/SegmentedControl/getSegmentedControlStyles.ts
index bac77d60a1d..35b8f5a3445 100644
--- a/src/SegmentedControl/getSegmentedControlStyles.ts
+++ b/src/SegmentedControl/getSegmentedControlStyles.ts
@@ -36,18 +36,12 @@ export const getSegmentedControlButtonStyles = (
borderWidth: 0,
color: 'currentColor',
cursor: 'pointer',
- flexGrow: 1,
fontFamily: 'inherit',
fontSize: 1,
fontWeight: props?.selected ? 'bold' : 'normal',
- marginTop: '-1px',
- marginBottom: '-1px',
padding: props?.selected ? 0 : 'var(--segmented-control-button-bg-inset)',
- position: 'relative',
- ...(props?.isIconOnly && {
- height: '32px', // TODO: use primitive `control.medium.size` when it is available
- width: '32px' // TODO: use primitive `control.medium.size` when it is available
- }),
+ height: '100%',
+ width: '100%',
'.segmentedControl-content': {
alignItems: 'center',
@@ -83,13 +77,6 @@ export const getSegmentedControlButtonStyles = (
backgroundColor: props?.selected ? undefined : 'segmentedControl.button.active.bg'
},
- // Icon-only buttons render the button inside of an element rendered by the Tooltip component.
- // This breaks `:first-child` and `:last-child` selectors on buttons because the button is the only child inside of the element rendered by the Tooltip component.
- ...(!props?.isIconOnly && {
- ...directChildLayoutAdjustments,
- ':not(:last-child)': borderedSegment
- }),
-
// fixes an issue where the focus outline shows over the pseudo-element
':focus:focus-visible:not(:last-child):after': {
width: 0
@@ -120,3 +107,14 @@ export const getSegmentedControlButtonStyles = (
}
}
})
+
+export const getSegmentedControlListItemStyles = () => ({
+ display: 'block',
+ position: 'relative',
+ flexGrow: 1,
+ marginTop: '-1px',
+ marginBottom: '-1px',
+ height: '32px', // TODO: use primitive `control.medium.size` when it is available
+ ':not(:last-child)': borderedSegment,
+ ...directChildLayoutAdjustments
+})