-
Notifications
You must be signed in to change notification settings - Fork 646
[a11y] When aria-disabled is set on SegmentedControl, don't allow action #7047
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
27b3a27
94c4b25
46f8beb
57f988d
c2aad6e
af99208
307289d
088b051
528e05a
3b6eefe
9acdcc4
d99fc85
3984849
3b95781
9ff45d6
af884b0
c31274e
154deb0
6eeecc9
fe20f4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@primer/react": minor | ||
| --- | ||
|
|
||
| Remove the feature flag for `primer_react_segmented_control_tooltip` and GA tooltip by default behavior. | ||
| - Ensure that when `disabled` is applied, the tooltip is still triggered. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import type {Meta} from '@storybook/react-vite' | ||
| import {SegmentedControl} from '.' | ||
| import {EyeIcon, FileCodeIcon, PeopleIcon} from '@primer/octicons-react' | ||
|
|
||
| export default { | ||
| title: 'Components/SegmentedControl/Examples', | ||
| component: SegmentedControl, | ||
| } as Meta<typeof SegmentedControl> | ||
|
|
||
| export const WithDisabledButtons = () => ( | ||
| <SegmentedControl aria-label="File view"> | ||
| <SegmentedControl.Button defaultSelected aria-label={'Preview'} leadingIcon={EyeIcon} disabled> | ||
| Preview | ||
| </SegmentedControl.Button> | ||
| <SegmentedControl.Button aria-label={'Raw'} leadingIcon={FileCodeIcon}> | ||
| Raw | ||
| </SegmentedControl.Button> | ||
| <SegmentedControl.Button aria-label={'Blame'} leadingIcon={PeopleIcon} disabled> | ||
| Blame | ||
| </SegmentedControl.Button> | ||
| </SegmentedControl> | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,13 +167,19 @@ const Root: React.FC<React.PropsWithChildren<SegmentedControlProps>> = ({ | |
| const sharedChildProps = { | ||
| onClick: onChange | ||
| ? (event: React.MouseEvent<HTMLButtonElement>) => { | ||
| onChange(index) | ||
| isUncontrolled && setSelectedIndexInternalState(index) | ||
| child.props.onClick && child.props.onClick(event) | ||
| 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) | ||
| } | ||
| } | ||
| : (event: React.MouseEvent<HTMLButtonElement>) => { | ||
| child.props.onClick && child.props.onClick(event) | ||
| isUncontrolled && setSelectedIndexInternalState(index) | ||
| const isDisabled = child.props.disabled === true || child.props['aria-disabled'] === true | ||
| if (!isDisabled) { | ||
| child.props.onClick && child.props.onClick(event) | ||
| isUncontrolled && setSelectedIndexInternalState(index) | ||
| } | ||
| }, | ||
| selected: index === selectedIndex, | ||
| style: { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,10 @@ 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. */ | ||||||||||||||||||||||||||
|
Comment on lines
+20
to
+22
|
||||||||||||||||||||||||||
| /** 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. */ | |
| /** | |
| * Disables the button, preventing click functionality and applying the `aria-disabled` attribute. | |
| * Use this prop to make the button non-interactive. | |
| */ | |
| disabled?: boolean | |
| /** | |
| * Applies the `aria-disabled` attribute to the button for accessibility purposes, but does not disable click functionality. | |
| * Use this prop if you want the button to appear disabled to assistive technologies but remain interactive. | |
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im confused on what the difference between these two props is.
Also, does the *.docs.json need to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to be backwards compatible from what I recall. Ideally we'll remove one or the other once we reduce usage in dotcom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we deprecate one now then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need VRT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! I'll add one.