Skip to content

Revert "[a11y] When aria-disabled is set on SegmentedControl, don't allow action" #6986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 13, 2025
Merged

Revert "[a11y] When aria-disabled is set on SegmentedControl, don't allow action" #6986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .changeset/strong-mangos-rest.md
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions packages/react/src/FeatureFlags/DefaultFeatureFlags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({
primer_react_action_list_item_as_button: false,
primer_react_breadcrumbs_overflow_menu: false,
primer_react_overlay_overflow: false,
primer_react_segmented_control_tooltip: false,
primer_react_select_panel_fullscreen_on_narrow: false,
primer_react_select_panel_order_selected_at_top: false,
primer_react_select_panel_remove_active_descendant: false,
Expand Down
76 changes: 0 additions & 76 deletions packages/react/src/SegmentedControl/SegmentedControl.dev.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,82 +13,6 @@ export default {
parameters: {controls: {exclude: excludedControlKeys}},
} as Meta<typeof SegmentedControl>

export const WithAriaDisabled = () => {
const handleOnClick = () => {
alert('Button clicked!')
}

return (
<SegmentedControl aria-label="File view" className="testCustomClassnameMono">
<SegmentedControl.IconButton
onClick={handleOnClick}
aria-label={'Preview'}
aria-disabled={true}
icon={EyeIcon}
className="testCustomClassnameColor"
>
Preview
</SegmentedControl.IconButton>
<SegmentedControl.IconButton
aria-disabled={true}
onClick={handleOnClick}
aria-label={'Raw'}
icon={FileCodeIcon}
className="testCustomClassnameColor"
>
Raw
</SegmentedControl.IconButton>
<SegmentedControl.IconButton
aria-disabled={true}
onClick={handleOnClick}
aria-label={'Blame'}
icon={PeopleIcon}
className="testCustomClassnameColor"
>
Blame
</SegmentedControl.IconButton>
</SegmentedControl>
)
}

export const WithDisabled = () => {
const handleOnClick = () => {
alert('Button clicked!')
}

return (
<SegmentedControl aria-label="File view" className="testCustomClassnameMono">
<SegmentedControl.IconButton
onClick={handleOnClick}
aria-label={'Preview'}
disabled={true}
icon={EyeIcon}
className="testCustomClassnameColor"
>
Preview
</SegmentedControl.IconButton>
<SegmentedControl.IconButton
disabled={true}
onClick={handleOnClick}
aria-label={'Raw'}
icon={FileCodeIcon}
className="testCustomClassnameColor"
>
Raw
</SegmentedControl.IconButton>
<SegmentedControl.IconButton
disabled={true}
onClick={handleOnClick}
aria-label={'Blame'}
icon={PeopleIcon}
className="testCustomClassnameColor"
>
Blame
</SegmentedControl.IconButton>
</SegmentedControl>
)
}

export const WithCss = () => (
<SegmentedControl aria-label="File view" className="testCustomClassnameMono">
<SegmentedControl.Button
Expand Down
22 changes: 0 additions & 22 deletions packages/react/src/SegmentedControl/SegmentedControl.examples.stories.tsx
View file
Open in desktop

This file was deleted.

13 changes: 1 addition & 12 deletions packages/react/src/SegmentedControl/SegmentedControl.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,6 @@
width: 0;
}

&[aria-disabled='true']:not([aria-current='true']) {
cursor: not-allowed;
color: var(--fgColor-disabled);
background-color: transparent;

& svg {
fill: var(--fgColor-disabled);
color: var(--fgColor-disabled);
}
}

@media (pointer: coarse) {
&::before {
position: absolute;
Expand Down Expand Up @@ -194,7 +183,7 @@
}
}

.Button:not([aria-current='true'], [aria-disabled='true']) {
.Button:not([aria-current='true']) {
&:hover .Content {
background-color: var(--controlTrack-bgColor-hover);
}
Expand Down
52 changes: 40 additions & 12 deletions packages/react/src/SegmentedControl/SegmentedControl.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {describe, expect, it, vi} from 'vitest'
import BaseStyles from '../BaseStyles'
import theme from '../theme'
import ThemeProvider from '../ThemeProvider'
import {FeatureFlags} from '../FeatureFlags'
import {SegmentedControl} from '../SegmentedControl'

const segmentData = [
Expand Down Expand Up @@ -143,13 +144,19 @@ describe('SegmentedControl', () => {
}
})

it('renders icon button with tooltip as label', () => {
it('renders icon button with tooltip as label when feature flag is enabled', () => {
const {getByRole, getByText} = render(
<SegmentedControl aria-label="File view">
{segmentData.map(({label, icon}) => (
<SegmentedControl.IconButton icon={icon} aria-label={label} key={label} />
))}
</SegmentedControl>,
<FeatureFlags
flags={{
primer_react_segmented_control_tooltip: true,
}}
>
<SegmentedControl aria-label="File view">
{segmentData.map(({label, icon}) => (
<SegmentedControl.IconButton icon={icon} aria-label={label} key={label} />
))}
</SegmentedControl>
</FeatureFlags>,
)

for (const datum of segmentData) {
Expand All @@ -160,13 +167,19 @@ describe('SegmentedControl', () => {
}
})

it('renders icon button with tooltip description', () => {
it('renders icon button with tooltip description when feature flag is enabled', () => {
const {getByRole, getByText} = render(
<SegmentedControl aria-label="File view">
{segmentData.map(({label, icon, description}) => (
<SegmentedControl.IconButton icon={icon} aria-label={label} description={description} key={label} />
))}
</SegmentedControl>,
<FeatureFlags
flags={{
primer_react_segmented_control_tooltip: true,
}}
>
<SegmentedControl aria-label="File view">
{segmentData.map(({label, icon, description}) => (
<SegmentedControl.IconButton icon={icon} aria-label={label} description={description} key={label} />
))}
</SegmentedControl>
</FeatureFlags>,
)

for (const datum of segmentData) {
Expand All @@ -178,6 +191,21 @@ describe('SegmentedControl', () => {
}
})

it('renders icon button with aria-label and no tooltip', () => {
const {getByRole} = render(
<SegmentedControl aria-label="File view">
{segmentData.map(({label, icon}) => (
<SegmentedControl.IconButton icon={icon} aria-label={label} key={label} />
))}
</SegmentedControl>,
)

for (const datum of segmentData) {
const labelledButton = getByRole('button', {name: datum.label})
expect(labelledButton).toHaveAttribute('aria-label', datum.label)
}
})

it('calls onChange with index of clicked segment button', async () => {
const user = userEvent.setup()
const handleChange = vi.fn()
Expand Down
16 changes: 5 additions & 11 deletions packages/react/src/SegmentedControl/SegmentedControl.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,19 +164,13 @@ const Root: React.FC<React.PropsWithChildren<SegmentedControlProps>> = ({
const sharedChildProps = {
onClick: onChange
? (event: React.MouseEvent<HTMLButtonElement>) => {
const isDisabled = child.props.disabled === true || child.props['aria-disabled'] === true
if (!isDisabled) {
onChange(index)
isUncontrolled && setSelectedIndexInternalState(index)
child.props.onClick && child.props.onClick(event)
}
onChange(index)
isUncontrolled && setSelectedIndexInternalState(index)
child.props.onClick && child.props.onClick(event)
Comment on lines 166 to +169
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing closing brace: The conditional block starting at line 166 is missing its closing brace, making the code structure unclear and potentially causing syntax errors.

Copilot uses AI. Check for mistakes.
}
: (event: React.MouseEvent<HTMLButtonElement>) => {
const isDisabled = child.props.disabled === true || child.props['aria-disabled'] === true
if (!isDisabled) {
child.props.onClick && child.props.onClick(event)
isUncontrolled && setSelectedIndexInternalState(index)
}
child.props.onClick && child.props.onClick(event)
isUncontrolled && setSelectedIndexInternalState(index)
},
selected: index === selectedIndex,
style: {
Expand Down
14 changes: 1 addition & 13 deletions packages/react/src/SegmentedControl/SegmentedControlButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ export type SegmentedControlButtonProps = {
defaultSelected?: boolean
/** The leading icon comes before item label */
leadingIcon?: React.FunctionComponent<React.PropsWithChildren<IconProps>> | React.ReactElement
/** Applies `aria-disabled` to the button. This will disable certain functionality, such as `onClick` events. */
disabled?: boolean
/** Applies `aria-disabled` to the button. This will disable certain functionality, such as `onClick` events. */
'aria-disabled'?: boolean
/** Optional counter to display on the right side of the button */
count?: number | string
} & ButtonHTMLAttributes<HTMLButtonElement | HTMLLIElement>
Expand All @@ -30,22 +26,14 @@ const SegmentedControlButton: FCWithSlotMarker<React.PropsWithChildren<Segmented
leadingIcon: LeadingIcon,
selected,
className,
disabled,
'aria-disabled': ariaDisabled,
// Note: this value is read in the `SegmentedControl` component to determine which button is selected but we do not need to apply it to an underlying element
defaultSelected: _defaultSelected,
count,
...rest
}) => {
return (
<li className={clsx(classes.Item)} data-selected={selected ? '' : undefined}>
<button
aria-current={selected}
aria-disabled={disabled || ariaDisabled || undefined}
className={clsx(classes.Button, className)}
type="button"
{...rest}
>
<button aria-current={selected} className={clsx(classes.Button, className)} type="button" {...rest}>
<span className={clsx(classes.Content, 'segmentedControl-content')}>
{LeadingIcon && (
<div className={classes.LeadingIcon}>{isElement(LeadingIcon) ? LeadingIcon : <LeadingIcon />}</div>
Expand Down
8 changes: 0 additions & 8 deletions packages/react/src/SegmentedControl/SegmentedControlIconButton.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export default {
icon: FileCodeIcon,
selected: false,
defaultSelected: false,
disabled: false,
'aria-disabled': false,
},
argTypes: {
icon: {
Expand All @@ -28,12 +26,6 @@ export default {
defaultSelected: {
type: 'boolean',
},
disabled: {
type: 'boolean',
},
'aria-disabled': {
type: 'boolean',
},
},
decorators: [
Story => {
Expand Down
52 changes: 33 additions & 19 deletions packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {ButtonHTMLAttributes} from 'react'
import type React from 'react'
import type {IconProps} from '@primer/octicons-react'
import {isElement} from 'react-is'
import {useFeatureFlag} from '../FeatureFlags'
import type {TooltipDirection} from '../TooltipV2'
import classes from './SegmentedControl.module.css'
import {clsx} from 'clsx'
Expand All @@ -20,10 +21,6 @@ export type SegmentedControlIconButtonProps = {
description?: string
/** The direction for the tooltip.*/
tooltipDirection?: TooltipDirection
/** Whether the button is disabled. */
disabled?: boolean
/** Whether the button is aria-disabled. */
'aria-disabled'?: boolean
} & ButtonHTMLAttributes<HTMLButtonElement | HTMLLIElement>

export const SegmentedControlIconButton: FCWithSlotMarker<React.PropsWithChildren<SegmentedControlIconButtonProps>> = ({
Expand All @@ -33,31 +30,48 @@ export const SegmentedControlIconButton: FCWithSlotMarker<React.PropsWithChildre
className,
description,
tooltipDirection,
disabled,
'aria-disabled': ariaDisabled,
...rest
}) => {
return (
<li className={clsx(classes.Item, className)} data-selected={selected || undefined}>
<Tooltip
type={description ? undefined : 'label'}
text={description ? description : ariaLabel}
direction={tooltipDirection}
>
const tooltipFlagEnabled = useFeatureFlag('primer_react_segmented_control_tooltip')
if (tooltipFlagEnabled) {
return (
<li className={clsx(classes.Item, className)} data-selected={selected || undefined}>
<Tooltip
type={description ? undefined : 'label'}
text={description ? description : ariaLabel}
direction={tooltipDirection}
>
<button
type="button"
aria-current={selected}
// If description is provided, we will use the tooltip to describe the button, so we need to keep the aria-label to label the button.
aria-label={description ? ariaLabel : undefined}
className={clsx(classes.Button, classes.IconButton)}
{...rest}
>
<span className={clsx(classes.Content, 'segmentedControl-content')}>
{isElement(Icon) ? Icon : <Icon />}
</span>
Comment on lines +52 to +54
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code duplication: The same span with identical structure and content exists on line 70. Consider extracting this into a reusable component or variable to reduce duplication.

Copilot uses AI. Check for mistakes.
</button>
</Tooltip>
</li>
)
} else {
// This can be removed when primer_react_segmented_control_tooltip feature flag is GA-ed.
return (
<li className={clsx(classes.Item, className)} data-selected={selected || undefined}>
<button
type="button"
aria-label={ariaLabel}
aria-current={selected}
// If description is provided, we will use the tooltip to describe the button, so we need to keep the aria-label to label the button.
aria-label={description ? ariaLabel : undefined}
aria-disabled={disabled || ariaDisabled || undefined}
className={clsx(classes.Button, classes.IconButton)}
{...rest}
>
<span className={clsx(classes.Content, 'segmentedControl-content')}>{isElement(Icon) ? Icon : <Icon />}</span>
</button>
</Tooltip>
</li>
)
</li>
)
}
}

SegmentedControlIconButton.__SLOT__ = Symbol('SegmentedControl.IconButton')
Expand Down
Loading