Skip to content

Commit e9f6ccc

Browse files
iansan5653iansan5653
authored
Merge branch 'main' into button-group-isolation
2 parents 7817e9c + 21d1b86 commit e9f6ccc

18 files changed

+172
-111
lines changed

‎.changeset/eleven-jokes-think.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': minor
3+
---
4+
5+
Addresses feedback from the accessibility team about our SegmentedControl component. These changes include an update to ActionMenu that allows u to specify the ID of the DOM node that labels the menu.

‎contributor-docs/adrs/adr-010-drafts-are-experimental.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Status
44

5-
Draft
5+
Approved 2023-02-16
66

77
## Context
88

‎docs/content/SegmentedControl.mdx‎

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,48 @@ storybook: '/react/storybook/?path=/story/components-segmentedcontrol'
99

1010
import data from '../../src/SegmentedControl/SegmentedControl.docs.json'
1111

12+
<Note variant="warning">
13+
14+
A `SegmentedControl` should not be used in a form as a replacement for something like a [RadioGroup](/RadioGroup) or [Select](/Select). See the [Accessibility section](https://primer.style/design/components/segmented-control#accessibility) of the SegmentedControl interface guidelines for more details.
15+
16+
</Note>
17+
1218
## Examples
1319

14-
### Uncontrolled (default)
20+
### With a label above and caption below
1521

1622
```jsx live
17-
<SegmentedControl aria-label="File view">
18-
<SegmentedControl.Button defaultSelected>Preview</SegmentedControl.Button>
19-
<SegmentedControl.Button>Raw</SegmentedControl.Button>
20-
<SegmentedControl.Button>Blame</SegmentedControl.Button>
21-
</SegmentedControl>
23+
<FormControl>
24+
<FormControl.Label id="scLabel-horiz" as="span">
25+
File view
26+
</FormControl.Label>
27+
<SegmentedControl aria-labelledby="scLabel-horiz" aria-describedby="scCaption-horiz">
28+
<SegmentedControl.Button defaultSelected>Preview</SegmentedControl.Button>
29+
<SegmentedControl.Button>Raw</SegmentedControl.Button>
30+
<SegmentedControl.Button>Blame</SegmentedControl.Button>
31+
</SegmentedControl>
32+
<FormControl.Caption id="scCaption-horiz">Change the way the file is viewed</FormControl.Caption>
33+
</FormControl>
34+
```
35+
36+
### With a label and caption on the left
37+
38+
```jsx live
39+
<Box display="flex">
40+
<Box flexGrow={1}>
41+
<Text fontSize={2} fontWeight="bold" id="scLabel-vert" display="block">
42+
File view
43+
</Text>
44+
<Text color="fg.subtle" fontSize={1} id="scCaption-vert" display="block">
45+
Change the way the file is viewed
46+
</Text>
47+
</Box>
48+
<SegmentedControl aria-labelledby="scLabel-vert" aria-describedby="scCaption-vert">
49+
<SegmentedControl.Button defaultSelected>Preview</SegmentedControl.Button>
50+
<SegmentedControl.Button>Raw</SegmentedControl.Button>
51+
<SegmentedControl.Button>Blame</SegmentedControl.Button>
52+
</SegmentedControl>
53+
</Box>
2254
```
2355

2456
### Controlled
@@ -109,40 +141,6 @@ render(Controlled)
109141
</SegmentedControl>
110142
```
111143

112-
### With a label and caption on the left
113-
114-
```jsx live
115-
<Box display="flex">
116-
<Box flexGrow={1}>
117-
<Text fontSize={2} fontWeight="bold" id="scLabel-vert" display="block">
118-
File view
119-
</Text>
120-
<Text color="fg.subtle" fontSize={1} id="scCaption-vert" display="block">
121-
Change the way the file is viewed
122-
</Text>
123-
</Box>
124-
<SegmentedControl aria-labelledby="scLabel-vert" aria-describedby="scCaption-vert">
125-
<SegmentedControl.Button defaultSelected>Preview</SegmentedControl.Button>
126-
<SegmentedControl.Button>Raw</SegmentedControl.Button>
127-
<SegmentedControl.Button>Blame</SegmentedControl.Button>
128-
</SegmentedControl>
129-
</Box>
130-
```
131-
132-
### With a label above and caption below
133-
134-
```jsx live
135-
<FormControl>
136-
<FormControl.Label id="scLabel-horiz">File view</FormControl.Label>
137-
<SegmentedControl aria-labelledby="scLabel-horiz" aria-describedby="scCaption-horiz">
138-
<SegmentedControl.Button defaultSelected>Preview</SegmentedControl.Button>
139-
<SegmentedControl.Button>Raw</SegmentedControl.Button>
140-
<SegmentedControl.Button>Blame</SegmentedControl.Button>
141-
</SegmentedControl>
142-
<FormControl.Caption id="scCaption-horiz">Change the way the file is viewed</FormControl.Caption>
143-
</FormControl>
144-
```
145-
146144
### With something besides the first option selected
147145

148146
```jsx live

‎generated/components.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2692,6 +2692,12 @@
26922692
"defaultValue": "false",
26932693
"description": "Whether the label should be visually hidden"
26942694
},
2695+
{
2696+
"name": "as",
2697+
"type": "'label' | 'legend' | 'span'",
2698+
"defaultValue": "'label'",
2699+
"description": "The label element can be changed to a 'legend' when it's being used to label a fieldset, or a 'span' when it's being used to label an element that is not a form input. For example: when using a FormControl to render a labeled SegementedControl, the label should be a 'span'"
2700+
},
26952701
{
26962702
"name": "sx",
26972703
"type": "SystemStyleObject"

‎src/ActionMenu.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,12 @@ type MenuOverlayProps = Partial<OverlayProps> &
8989
*/
9090
children: React.ReactNode
9191
}
92-
const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({children, align = 'start', ...overlayProps}) => {
92+
const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({
93+
children,
94+
align = 'start',
95+
'aria-labelledby': ariaLabelledby,
96+
...overlayProps
97+
}) => {
9398
// we typecast anchorRef as required instead of optional
9499
// because we know that we're setting it in context in Menu
95100
const {anchorRef, renderAnchor, anchorId, open, onOpen, onClose} = React.useContext(MenuContext) as MandateProps<
@@ -117,7 +122,7 @@ const Overlay: React.FC<React.PropsWithChildren<MenuOverlayProps>> = ({children,
117122
value={{
118123
container: 'ActionMenu',
119124
listRole: 'menu',
120-
listLabelledBy: anchorId,
125+
listLabelledBy: ariaLabelledby || anchorId,
121126
selectionAttribute: 'aria-checked', // Should this be here?
122127
afterSelect: onClose,
123128
}}

‎src/FormControl/FormControl.docs.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@
4949
"defaultValue": "false",
5050
"description": "Whether the label should be visually hidden"
5151
},
52+
{
53+
"name": "as",
54+
"type": "'label' | 'legend' | 'span'",
55+
"defaultValue": "'label'",
56+
"description": "The label element can be changed to a 'legend' when it's being used to label a fieldset, or a 'span' when it's being used to label an element that is not a form input. For example: when using a FormControl to render a labeled SegementedControl, the label should be a 'span'"
57+
},
5258
{
5359
"name": "sx",
5460
"type": "SystemStyleObject"

‎src/FormControl/_FormControlLabel.tsx‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react'
22
import {SxProp} from '../sx'
3-
import InputLabel from '../_InputLabel'
3+
import InputLabel, {LegendOrSpanProps, LabelProps} from '../_InputLabel'
44
import {FormControlContext} from './FormControl'
55
import {Slot} from './slots'
66

@@ -9,15 +9,12 @@ export type Props = {
99
* Whether the label should be visually hidden
1010
*/
1111
visuallyHidden?: boolean
12+
id?: string
1213
} & SxProp
1314

14-
const FormControlLabel: React.FC<React.PropsWithChildren<{htmlFor?: string; id?: string} & Props>> = ({
15-
children,
16-
htmlFor,
17-
id,
18-
visuallyHidden,
19-
sx,
20-
}) => (
15+
const FormControlLabel: React.FC<
16+
React.PropsWithChildren<{htmlFor?: string} & (LegendOrSpanProps | LabelProps) & Props>
17+
> = ({children, htmlFor, id, visuallyHidden, sx}) => (
2118
<Slot name="Label">
2219
{({disabled, id: formControlId, required}: FormControlContext) => (
2320
<InputLabel

‎src/SegmentedControl/SegmentedControl.stories.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export const Playground: Story<Args> = args => (
112112
variant={parseVariantFromArgs(args)}
113113
size={args.size}
114114
>
115-
<SegmentedControl.Button selected aria-label={'Preview'} leadingIcon={EyeIcon}>
115+
<SegmentedControl.Button defaultSelected aria-label={'Preview'} leadingIcon={EyeIcon}>
116116
Preview
117117
</SegmentedControl.Button>
118118
<SegmentedControl.Button aria-label={'Raw'} leadingIcon={FileCodeIcon}>

‎src/SegmentedControl/SegmentedControl.test.tsx‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ describe('SegmentedControl', () => {
4545
SegmentedControl,
4646
})
4747

48-
it('renders with a selected segment', () => {
48+
it('renders with a selected segment - controlled', () => {
4949
const {getByText} = render(
5050
<SegmentedControl aria-label="File view">
5151
{segmentData.map(({label}, index) => (
@@ -61,6 +61,22 @@ describe('SegmentedControl', () => {
6161
expect(selectedButton?.getAttribute('aria-current')).toBe('true')
6262
})
6363

64+
it('renders with a selected segment - uncontrolled', () => {
65+
const {getByText} = render(
66+
<SegmentedControl aria-label="File view">
67+
{segmentData.map(({label}, index) => (
68+
<SegmentedControl.Button defaultSelected={index === 1} key={label}>
69+
{label}
70+
</SegmentedControl.Button>
71+
))}
72+
</SegmentedControl>,
73+
)
74+
75+
const selectedButton = getByText('Raw').closest('button')
76+
77+
expect(selectedButton?.getAttribute('aria-current')).toBe('true')
78+
})
79+
6480
it('renders the dropdown variant', () => {
6581
act(() => {
6682
matchMedia.useMediaQuery(viewportRanges.narrow)

‎src/SegmentedControl/SegmentedControl.tsx‎

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ const Root: React.FC<React.PropsWithChildren<SegmentedControlProps>> = ({
6868
const selectedSegments = React.Children.toArray(children).map(
6969
child =>
7070
React.isValidElement<SegmentedControlButtonProps | SegmentedControlIconButtonProps>(child) &&
71-
child.props.selected,
71+
(child.props.defaultSelected || child.props.selected),
7272
)
7373
const hasSelectedButton = selectedSegments.some(isSelected => isSelected)
7474
const selectedIndexExternal = hasSelectedButton ? selectedSegments.indexOf(true) : 0
@@ -102,40 +102,48 @@ const Root: React.FC<React.PropsWithChildren<SegmentedControlProps>> = ({
102102
if (!ariaLabel && !ariaLabelledby) {
103103
// eslint-disable-next-line no-console
104104
console.warn(
105-
'Use the `aria-label` or `aria-labelledby` prop to provide an accessible label for assistive technology',
105+
'Use the `aria-label` or `aria-labelledby` prop to provide an accessible label for assistive technologies',
106106
)
107107
}
108108

109109
return responsiveVariant === 'dropdown' ? (
110110
// Render the 'dropdown' variant of the SegmentedControlButton or SegmentedControlIconButton
111-
<ActionMenu>
112-
<ActionMenu.Button leadingIcon={getChildIcon(selectedChild)}>{getChildText(selectedChild)}</ActionMenu.Button>
113-
<ActionMenu.Overlay>
114-
<ActionList selectionVariant="single">
115-
{React.Children.map(children, (child, index) => {
116-
const ChildIcon = getChildIcon(child)
117-
// Not a valid child element - skip rendering
118-
if (!React.isValidElement<SegmentedControlButtonProps | SegmentedControlIconButtonProps>(child)) {
119-
return null
120-
}
111+
<>
112+
<ActionMenu>
113+
{/*
114+
The aria-label is only provided as a backup when the designer or engineer neglects to show a label for the SegmentedControl.
115+
The best thing to do is to have a visual label who's id is referenced using the `aria-labelledby` prop.
116+
*/}
117+
<ActionMenu.Button aria-label={ariaLabel} leadingIcon={getChildIcon(selectedChild)}>
118+
{getChildText(selectedChild)}
119+
</ActionMenu.Button>
120+
<ActionMenu.Overlay aria-labelledby={ariaLabelledby}>
121+
<ActionList selectionVariant="single">
122+
{React.Children.map(children, (child, index) => {
123+
const ChildIcon = getChildIcon(child)
124+
// Not a valid child element - skip rendering
125+
if (!React.isValidElement<SegmentedControlButtonProps | SegmentedControlIconButtonProps>(child)) {
126+
return null
127+
}
121128

122-
return (
123-
<ActionList.Item
124-
key={`segmented-control-action-btn-${index}`}
125-
selected={index === selectedIndex}
126-
onSelect={(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
127-
isUncontrolled && setSelectedIndexInternalState(index)
128-
onChange && onChange(index)
129-
child.props.onClick && child.props.onClick(event as React.MouseEvent<HTMLLIElement>)
130-
}}
131-
>
132-
{ChildIcon && <ChildIcon />} {getChildText(child)}
133-
</ActionList.Item>
134-
)
135-
})}
136-
</ActionList>
137-
</ActionMenu.Overlay>
138-
</ActionMenu>
129+
return (
130+
<ActionList.Item
131+
key={`segmented-control-action-btn-${index}`}
132+
selected={index === selectedIndex}
133+
onSelect={(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
134+
isUncontrolled && setSelectedIndexInternalState(index)
135+
onChange && onChange(index)
136+
child.props.onClick && child.props.onClick(event as React.MouseEvent<HTMLLIElement>)
137+
}}
138+
>
139+
{ChildIcon && <ChildIcon />} {getChildText(child)}
140+
</ActionList.Item>
141+
)
142+
})}
143+
</ActionList>
144+
</ActionMenu.Overlay>
145+
</ActionMenu>
146+
</>
139147
) : (
140148
// Render a segmented control
141149
<SegmentedControlList

0 commit comments

Comments
 (0)