diff --git a/.changeset/eleven-jokes-think.md b/.changeset/eleven-jokes-think.md new file mode 100644 index 00000000000..5ac7c24582a --- /dev/null +++ b/.changeset/eleven-jokes-think.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +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. diff --git a/docs/content/SegmentedControl.mdx b/docs/content/SegmentedControl.mdx index 9db26a21813..d6c56925d65 100644 --- a/docs/content/SegmentedControl.mdx +++ b/docs/content/SegmentedControl.mdx @@ -9,16 +9,48 @@ storybook: '/react/storybook/?path=/story/components-segmentedcontrol' import data from '../../src/SegmentedControl/SegmentedControl.docs.json' + + +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. + + + ## Examples -### Uncontrolled (default) +### With a label above and caption below ```jsx live - - Preview - Raw - Blame - + + + File view + + + Preview + Raw + Blame + + Change the way the file is viewed + +``` + +### With a label and caption on the left + +```jsx live + + + + File view + + + Change the way the file is viewed + + + + Preview + Raw + Blame + + ``` ### Controlled @@ -109,40 +141,6 @@ render(Controlled) ``` -### With a label and caption on the left - -```jsx live - - - - File view - - - Change the way the file is viewed - - - - Preview - Raw - Blame - - -``` - -### With a label above and caption below - -```jsx live - - File view - - Preview - Raw - Blame - - Change the way the file is viewed - -``` - ### With something besides the first option selected ```jsx live diff --git a/generated/components.json b/generated/components.json index 60287ca45e2..a81c15c7097 100644 --- a/generated/components.json +++ b/generated/components.json @@ -2692,6 +2692,12 @@ "defaultValue": "false", "description": "Whether the label should be visually hidden" }, + { + "name": "as", + "type": "'label' | 'legend' | 'span'", + "defaultValue": "'label'", + "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'" + }, { "name": "sx", "type": "SystemStyleObject" diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index bcf1179eb87..a9568da86f8 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -89,7 +89,12 @@ type MenuOverlayProps = Partial & */ children: React.ReactNode } -const Overlay: React.FC> = ({children, align = 'start', ...overlayProps}) => { +const Overlay: React.FC> = ({ + children, + align = 'start', + 'aria-labelledby': ariaLabelledby, + ...overlayProps +}) => { // we typecast anchorRef as required instead of optional // because we know that we're setting it in context in Menu const {anchorRef, renderAnchor, anchorId, open, onOpen, onClose} = React.useContext(MenuContext) as MandateProps< @@ -117,7 +122,7 @@ const Overlay: React.FC> = ({children, value={{ container: 'ActionMenu', listRole: 'menu', - listLabelledBy: anchorId, + listLabelledBy: ariaLabelledby || anchorId, selectionAttribute: 'aria-checked', // Should this be here? afterSelect: onClose, }} diff --git a/src/FormControl/FormControl.docs.json b/src/FormControl/FormControl.docs.json index 25b0cf834cb..8f5aac8fa21 100644 --- a/src/FormControl/FormControl.docs.json +++ b/src/FormControl/FormControl.docs.json @@ -49,6 +49,12 @@ "defaultValue": "false", "description": "Whether the label should be visually hidden" }, + { + "name": "as", + "type": "'label' | 'legend' | 'span'", + "defaultValue": "'label'", + "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'" + }, { "name": "sx", "type": "SystemStyleObject" diff --git a/src/FormControl/_FormControlLabel.tsx b/src/FormControl/_FormControlLabel.tsx index 323b1081248..9ae1e7d3bf4 100644 --- a/src/FormControl/_FormControlLabel.tsx +++ b/src/FormControl/_FormControlLabel.tsx @@ -1,6 +1,6 @@ import React from 'react' import {SxProp} from '../sx' -import InputLabel from '../_InputLabel' +import InputLabel, {LegendOrSpanProps, LabelProps} from '../_InputLabel' import {FormControlContext} from './FormControl' import {Slot} from './slots' @@ -9,15 +9,12 @@ export type Props = { * Whether the label should be visually hidden */ visuallyHidden?: boolean + id?: string } & SxProp -const FormControlLabel: React.FC> = ({ - children, - htmlFor, - id, - visuallyHidden, - sx, -}) => ( +const FormControlLabel: React.FC< + React.PropsWithChildren<{htmlFor?: string} & (LegendOrSpanProps | LabelProps) & Props> +> = ({children, htmlFor, id, visuallyHidden, sx}) => ( {({disabled, id: formControlId, required}: FormControlContext) => ( = args => ( variant={parseVariantFromArgs(args)} size={args.size} > - + Preview diff --git a/src/SegmentedControl/SegmentedControl.test.tsx b/src/SegmentedControl/SegmentedControl.test.tsx index 76e7a077d3d..ea95e54d70b 100644 --- a/src/SegmentedControl/SegmentedControl.test.tsx +++ b/src/SegmentedControl/SegmentedControl.test.tsx @@ -45,7 +45,7 @@ describe('SegmentedControl', () => { SegmentedControl, }) - it('renders with a selected segment', () => { + it('renders with a selected segment - controlled', () => { const {getByText} = render( {segmentData.map(({label}, index) => ( @@ -61,6 +61,22 @@ describe('SegmentedControl', () => { expect(selectedButton?.getAttribute('aria-current')).toBe('true') }) + it('renders with a selected segment - uncontrolled', () => { + const {getByText} = render( + + {segmentData.map(({label}, index) => ( + + {label} + + ))} + , + ) + + const selectedButton = getByText('Raw').closest('button') + + expect(selectedButton?.getAttribute('aria-current')).toBe('true') + }) + it('renders the dropdown variant', () => { act(() => { matchMedia.useMediaQuery(viewportRanges.narrow) diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx index c22d3d2cd4f..1bac47b3d1d 100644 --- a/src/SegmentedControl/SegmentedControl.tsx +++ b/src/SegmentedControl/SegmentedControl.tsx @@ -68,7 +68,7 @@ const Root: React.FC> = ({ const selectedSegments = React.Children.toArray(children).map( child => React.isValidElement(child) && - child.props.selected, + (child.props.defaultSelected || child.props.selected), ) const hasSelectedButton = selectedSegments.some(isSelected => isSelected) const selectedIndexExternal = hasSelectedButton ? selectedSegments.indexOf(true) : 0 @@ -102,40 +102,48 @@ const Root: React.FC> = ({ if (!ariaLabel && !ariaLabelledby) { // eslint-disable-next-line no-console console.warn( - 'Use the `aria-label` or `aria-labelledby` prop to provide an accessible label for assistive technology', + 'Use the `aria-label` or `aria-labelledby` prop to provide an accessible label for assistive technologies', ) } return responsiveVariant === 'dropdown' ? ( // Render the 'dropdown' variant of the SegmentedControlButton or SegmentedControlIconButton - - {getChildText(selectedChild)} - - - {React.Children.map(children, (child, index) => { - const ChildIcon = getChildIcon(child) - // Not a valid child element - skip rendering - if (!React.isValidElement(child)) { - return null - } + <> + + {/* + The aria-label is only provided as a backup when the designer or engineer neglects to show a label for the SegmentedControl. + The best thing to do is to have a visual label who's id is referenced using the `aria-labelledby` prop. + */} + + {getChildText(selectedChild)} + + + + {React.Children.map(children, (child, index) => { + const ChildIcon = getChildIcon(child) + // Not a valid child element - skip rendering + if (!React.isValidElement(child)) { + return null + } - return ( - | React.KeyboardEvent) => { - isUncontrolled && setSelectedIndexInternalState(index) - onChange && onChange(index) - child.props.onClick && child.props.onClick(event as React.MouseEvent) - }} - > - {ChildIcon && } {getChildText(child)} - - ) - })} - - - + return ( + | React.KeyboardEvent) => { + isUncontrolled && setSelectedIndexInternalState(index) + onChange && onChange(index) + child.props.onClick && child.props.onClick(event as React.MouseEvent) + }} + > + {ChildIcon && } {getChildText(child)} + + ) + })} + + + + ) : ( // Render a segmented control ( - Preview + Preview Raw Blame @@ -19,7 +19,7 @@ export const Default = () => ( export const WithIcons = () => ( - + Preview @@ -47,7 +47,7 @@ export const Controlled = () => { export const VariantNarrowHideLabels = () => ( - + Preview @@ -62,7 +62,7 @@ VariantNarrowHideLabels.storyName = '[variant: narrow] Hide labels' export const VariantNarrowActionMenu = () => ( - + Preview @@ -77,7 +77,7 @@ VariantNarrowActionMenu.storyName = '[variant: narrow] Action menu' export const FullwidthNarrow = () => ( - + Preview @@ -92,7 +92,7 @@ FullwidthNarrow.storyName = '[fullWidth: narrow]' export const FullwidthRegular = () => ( - + Preview @@ -107,7 +107,7 @@ FullwidthRegular.storyName = '[fullWidth: regular]' export const FullwidthAll = () => ( - + Preview @@ -122,17 +122,24 @@ FullwidthAll.storyName = 'Full width' export const IconOnly = () => ( - + ) IconOnly.storyName = 'Icon only' -// TODO: make it possible to use FormControl as a wrapper for SegmentedControl -// - FormControl.Label needs to accept a prop that lets it render an element that isn't a `