Skip to content

Commit 930b401

Browse files
Revert "[a11y] When aria-disabled is set on SegmentedControl, don't allow action" (#6986)
1 parent 62df166 commit 930b401

10 files changed

+81
-179
lines changed

.changeset/strong-mangos-rest.md

Lines changed: 0 additions & 6 deletions
This file was deleted.

packages/react/src/FeatureFlags/DefaultFeatureFlags.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({
44
primer_react_action_list_item_as_button: false,
55
primer_react_breadcrumbs_overflow_menu: false,
66
primer_react_overlay_overflow: false,
7+
primer_react_segmented_control_tooltip: false,
78
primer_react_select_panel_fullscreen_on_narrow: false,
89
primer_react_select_panel_order_selected_at_top: false,
910
primer_react_select_panel_remove_active_descendant: false,

packages/react/src/SegmentedControl/SegmentedControl.dev.stories.tsx

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -13,82 +13,6 @@ export default {
1313
parameters: {controls: {exclude: excludedControlKeys}},
1414
} as Meta<typeof SegmentedControl>
1515

16-
export const WithAriaDisabled = () => {
17-
const handleOnClick = () => {
18-
alert('Button clicked!')
19-
}
20-
21-
return (
22-
<SegmentedControl aria-label="File view" className="testCustomClassnameMono">
23-
<SegmentedControl.IconButton
24-
onClick={handleOnClick}
25-
aria-label={'Preview'}
26-
aria-disabled={true}
27-
icon={EyeIcon}
28-
className="testCustomClassnameColor"
29-
>
30-
Preview
31-
</SegmentedControl.IconButton>
32-
<SegmentedControl.IconButton
33-
aria-disabled={true}
34-
onClick={handleOnClick}
35-
aria-label={'Raw'}
36-
icon={FileCodeIcon}
37-
className="testCustomClassnameColor"
38-
>
39-
Raw
40-
</SegmentedControl.IconButton>
41-
<SegmentedControl.IconButton
42-
aria-disabled={true}
43-
onClick={handleOnClick}
44-
aria-label={'Blame'}
45-
icon={PeopleIcon}
46-
className="testCustomClassnameColor"
47-
>
48-
Blame
49-
</SegmentedControl.IconButton>
50-
</SegmentedControl>
51-
)
52-
}
53-
54-
export const WithDisabled = () => {
55-
const handleOnClick = () => {
56-
alert('Button clicked!')
57-
}
58-
59-
return (
60-
<SegmentedControl aria-label="File view" className="testCustomClassnameMono">
61-
<SegmentedControl.IconButton
62-
onClick={handleOnClick}
63-
aria-label={'Preview'}
64-
disabled={true}
65-
icon={EyeIcon}
66-
className="testCustomClassnameColor"
67-
>
68-
Preview
69-
</SegmentedControl.IconButton>
70-
<SegmentedControl.IconButton
71-
disabled={true}
72-
onClick={handleOnClick}
73-
aria-label={'Raw'}
74-
icon={FileCodeIcon}
75-
className="testCustomClassnameColor"
76-
>
77-
Raw
78-
</SegmentedControl.IconButton>
79-
<SegmentedControl.IconButton
80-
disabled={true}
81-
onClick={handleOnClick}
82-
aria-label={'Blame'}
83-
icon={PeopleIcon}
84-
className="testCustomClassnameColor"
85-
>
86-
Blame
87-
</SegmentedControl.IconButton>
88-
</SegmentedControl>
89-
)
90-
}
91-
9216
export const WithCss = () => (
9317
<SegmentedControl aria-label="File view" className="testCustomClassnameMono">
9418
<SegmentedControl.Button

packages/react/src/SegmentedControl/SegmentedControl.examples.stories.tsx

Lines changed: 0 additions & 22 deletions
This file was deleted.

packages/react/src/SegmentedControl/SegmentedControl.module.css

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,6 @@
124124
width: 0;
125125
}
126126

127-
&[aria-disabled='true']:not([aria-current='true']) {
128-
cursor: not-allowed;
129-
color: var(--fgColor-disabled);
130-
background-color: transparent;
131-
132-
& svg {
133-
fill: var(--fgColor-disabled);
134-
color: var(--fgColor-disabled);
135-
}
136-
}
137-
138127
@media (pointer: coarse) {
139128
&::before {
140129
position: absolute;
@@ -194,7 +183,7 @@
194183
}
195184
}
196185

197-
.Button:not([aria-current='true'], [aria-disabled='true']) {
186+
.Button:not([aria-current='true']) {
198187
&:hover .Content {
199188
background-color: var(--controlTrack-bgColor-hover);
200189
}

packages/react/src/SegmentedControl/SegmentedControl.test.tsx

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {describe, expect, it, vi} from 'vitest'
55
import BaseStyles from '../BaseStyles'
66
import theme from '../theme'
77
import ThemeProvider from '../ThemeProvider'
8+
import {FeatureFlags} from '../FeatureFlags'
89
import {SegmentedControl} from '../SegmentedControl'
910

1011
const segmentData = [
@@ -143,13 +144,19 @@ describe('SegmentedControl', () => {
143144
}
144145
})
145146

146-
it('renders icon button with tooltip as label', () => {
147+
it('renders icon button with tooltip as label when feature flag is enabled', () => {
147148
const {getByRole, getByText} = render(
148-
<SegmentedControl aria-label="File view">
149-
{segmentData.map(({label, icon}) => (
150-
<SegmentedControl.IconButton icon={icon} aria-label={label} key={label} />
151-
))}
152-
</SegmentedControl>,
149+
<FeatureFlags
150+
flags={{
151+
primer_react_segmented_control_tooltip: true,
152+
}}
153+
>
154+
<SegmentedControl aria-label="File view">
155+
{segmentData.map(({label, icon}) => (
156+
<SegmentedControl.IconButton icon={icon} aria-label={label} key={label} />
157+
))}
158+
</SegmentedControl>
159+
</FeatureFlags>,
153160
)
154161

155162
for (const datum of segmentData) {
@@ -160,13 +167,19 @@ describe('SegmentedControl', () => {
160167
}
161168
})
162169

163-
it('renders icon button with tooltip description', () => {
170+
it('renders icon button with tooltip description when feature flag is enabled', () => {
164171
const {getByRole, getByText} = render(
165-
<SegmentedControl aria-label="File view">
166-
{segmentData.map(({label, icon, description}) => (
167-
<SegmentedControl.IconButton icon={icon} aria-label={label} description={description} key={label} />
168-
))}
169-
</SegmentedControl>,
172+
<FeatureFlags
173+
flags={{
174+
primer_react_segmented_control_tooltip: true,
175+
}}
176+
>
177+
<SegmentedControl aria-label="File view">
178+
{segmentData.map(({label, icon, description}) => (
179+
<SegmentedControl.IconButton icon={icon} aria-label={label} description={description} key={label} />
180+
))}
181+
</SegmentedControl>
182+
</FeatureFlags>,
170183
)
171184

172185
for (const datum of segmentData) {
@@ -178,6 +191,21 @@ describe('SegmentedControl', () => {
178191
}
179192
})
180193

194+
it('renders icon button with aria-label and no tooltip', () => {
195+
const {getByRole} = render(
196+
<SegmentedControl aria-label="File view">
197+
{segmentData.map(({label, icon}) => (
198+
<SegmentedControl.IconButton icon={icon} aria-label={label} key={label} />
199+
))}
200+
</SegmentedControl>,
201+
)
202+
203+
for (const datum of segmentData) {
204+
const labelledButton = getByRole('button', {name: datum.label})
205+
expect(labelledButton).toHaveAttribute('aria-label', datum.label)
206+
}
207+
})
208+
181209
it('calls onChange with index of clicked segment button', async () => {
182210
const user = userEvent.setup()
183211
const handleChange = vi.fn()

packages/react/src/SegmentedControl/SegmentedControl.tsx

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -164,19 +164,13 @@ const Root: React.FC<React.PropsWithChildren<SegmentedControlProps>> = ({
164164
const sharedChildProps = {
165165
onClick: onChange
166166
? (event: React.MouseEvent<HTMLButtonElement>) => {
167-
const isDisabled = child.props.disabled === true || child.props['aria-disabled'] === true
168-
if (!isDisabled) {
169-
onChange(index)
170-
isUncontrolled && setSelectedIndexInternalState(index)
171-
child.props.onClick && child.props.onClick(event)
172-
}
167+
onChange(index)
168+
isUncontrolled && setSelectedIndexInternalState(index)
169+
child.props.onClick && child.props.onClick(event)
173170
}
174171
: (event: React.MouseEvent<HTMLButtonElement>) => {
175-
const isDisabled = child.props.disabled === true || child.props['aria-disabled'] === true
176-
if (!isDisabled) {
177-
child.props.onClick && child.props.onClick(event)
178-
isUncontrolled && setSelectedIndexInternalState(index)
179-
}
172+
child.props.onClick && child.props.onClick(event)
173+
isUncontrolled && setSelectedIndexInternalState(index)
180174
},
181175
selected: index === selectedIndex,
182176
style: {

packages/react/src/SegmentedControl/SegmentedControlButton.tsx

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ export type SegmentedControlButtonProps = {
1717
defaultSelected?: boolean
1818
/** The leading icon comes before item label */
1919
leadingIcon?: React.FunctionComponent<React.PropsWithChildren<IconProps>> | React.ReactElement
20-
/** Applies `aria-disabled` to the button. This will disable certain functionality, such as `onClick` events. */
21-
disabled?: boolean
22-
/** Applies `aria-disabled` to the button. This will disable certain functionality, such as `onClick` events. */
23-
'aria-disabled'?: boolean
2420
/** Optional counter to display on the right side of the button */
2521
count?: number | string
2622
} & ButtonHTMLAttributes<HTMLButtonElement | HTMLLIElement>
@@ -30,22 +26,14 @@ const SegmentedControlButton: FCWithSlotMarker<React.PropsWithChildren<Segmented
3026
leadingIcon: LeadingIcon,
3127
selected,
3228
className,
33-
disabled,
34-
'aria-disabled': ariaDisabled,
3529
// 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
3630
defaultSelected: _defaultSelected,
3731
count,
3832
...rest
3933
}) => {
4034
return (
4135
<li className={clsx(classes.Item)} data-selected={selected ? '' : undefined}>
42-
<button
43-
aria-current={selected}
44-
aria-disabled={disabled || ariaDisabled || undefined}
45-
className={clsx(classes.Button, className)}
46-
type="button"
47-
{...rest}
48-
>
36+
<button aria-current={selected} className={clsx(classes.Button, className)} type="button" {...rest}>
4937
<span className={clsx(classes.Content, 'segmentedControl-content')}>
5038
{LeadingIcon && (
5139
<div className={classes.LeadingIcon}>{isElement(LeadingIcon) ? LeadingIcon : <LeadingIcon />}</div>

packages/react/src/SegmentedControl/SegmentedControlIconButton.stories.tsx

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ export default {
1313
icon: FileCodeIcon,
1414
selected: false,
1515
defaultSelected: false,
16-
disabled: false,
17-
'aria-disabled': false,
1816
},
1917
argTypes: {
2018
icon: {
@@ -28,12 +26,6 @@ export default {
2826
defaultSelected: {
2927
type: 'boolean',
3028
},
31-
disabled: {
32-
type: 'boolean',
33-
},
34-
'aria-disabled': {
35-
type: 'boolean',
36-
},
3729
},
3830
decorators: [
3931
Story => {

packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {ButtonHTMLAttributes} from 'react'
22
import type React from 'react'
33
import type {IconProps} from '@primer/octicons-react'
44
import {isElement} from 'react-is'
5+
import {useFeatureFlag} from '../FeatureFlags'
56
import type {TooltipDirection} from '../TooltipV2'
67
import classes from './SegmentedControl.module.css'
78
import {clsx} from 'clsx'
@@ -20,10 +21,6 @@ export type SegmentedControlIconButtonProps = {
2021
description?: string
2122
/** The direction for the tooltip.*/
2223
tooltipDirection?: TooltipDirection
23-
/** Whether the button is disabled. */
24-
disabled?: boolean
25-
/** Whether the button is aria-disabled. */
26-
'aria-disabled'?: boolean
2724
} & ButtonHTMLAttributes<HTMLButtonElement | HTMLLIElement>
2825

2926
export const SegmentedControlIconButton: FCWithSlotMarker<React.PropsWithChildren<SegmentedControlIconButtonProps>> = ({
@@ -33,31 +30,48 @@ export const SegmentedControlIconButton: FCWithSlotMarker<React.PropsWithChildre
3330
className,
3431
description,
3532
tooltipDirection,
36-
disabled,
37-
'aria-disabled': ariaDisabled,
3833
...rest
3934
}) => {
40-
return (
41-
<li className={clsx(classes.Item, className)} data-selected={selected || undefined}>
42-
<Tooltip
43-
type={description ? undefined : 'label'}
44-
text={description ? description : ariaLabel}
45-
direction={tooltipDirection}
46-
>
35+
const tooltipFlagEnabled = useFeatureFlag('primer_react_segmented_control_tooltip')
36+
if (tooltipFlagEnabled) {
37+
return (
38+
<li className={clsx(classes.Item, className)} data-selected={selected || undefined}>
39+
<Tooltip
40+
type={description ? undefined : 'label'}
41+
text={description ? description : ariaLabel}
42+
direction={tooltipDirection}
43+
>
44+
<button
45+
type="button"
46+
aria-current={selected}
47+
// 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.
48+
aria-label={description ? ariaLabel : undefined}
49+
className={clsx(classes.Button, classes.IconButton)}
50+
{...rest}
51+
>
52+
<span className={clsx(classes.Content, 'segmentedControl-content')}>
53+
{isElement(Icon) ? Icon : <Icon />}
54+
</span>
55+
</button>
56+
</Tooltip>
57+
</li>
58+
)
59+
} else {
60+
// This can be removed when primer_react_segmented_control_tooltip feature flag is GA-ed.
61+
return (
62+
<li className={clsx(classes.Item, className)} data-selected={selected || undefined}>
4763
<button
4864
type="button"
65+
aria-label={ariaLabel}
4966
aria-current={selected}
50-
// 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.
51-
aria-label={description ? ariaLabel : undefined}
52-
aria-disabled={disabled || ariaDisabled || undefined}
5367
className={clsx(classes.Button, classes.IconButton)}
5468
{...rest}
5569
>
5670
<span className={clsx(classes.Content, 'segmentedControl-content')}>{isElement(Icon) ? Icon : <Icon />}</span>
5771
</button>
58-
</Tooltip>
59-
</li>
60-
)
72+
</li>
73+
)
74+
}
6175
}
6276

6377
SegmentedControlIconButton.__SLOT__ = Symbol('SegmentedControl.IconButton')

0 commit comments

Comments
 (0)