From 878ce35e6460b9f5317a4efe29b01c982d744995 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 25 Jan 2023 11:43:00 -0500 Subject: [PATCH 01/11] allows FormControl.Label to render as a span or legend and updates the SegmentedControl example to render the label as a span --- docs/content/SegmentedControl.mdx | 4 +++- src/FormControl/FormControl.docs.json | 6 ++++++ src/FormControl/_FormControlLabel.tsx | 13 +++++-------- src/_InputLabel.tsx | 8 ++++---- src/utils/story-helpers.tsx | 2 +- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/content/SegmentedControl.mdx b/docs/content/SegmentedControl.mdx index 9db26a21813..d5121d2224a 100644 --- a/docs/content/SegmentedControl.mdx +++ b/docs/content/SegmentedControl.mdx @@ -133,7 +133,9 @@ render(Controlled) ```jsx live - File view + + File view + Preview Raw diff --git a/src/FormControl/FormControl.docs.json b/src/FormControl/FormControl.docs.json index 25b0cf834cb..1ee9f06913d 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) => ( > = ({ children, diff --git a/src/utils/story-helpers.tsx b/src/utils/story-helpers.tsx index e86349cf240..633949393e6 100644 --- a/src/utils/story-helpers.tsx +++ b/src/utils/story-helpers.tsx @@ -29,7 +29,7 @@ export type CheckboxOrRadioGroupArgs = CheckboxOrRadioGroupWrapperArgs & CheckboxOrRadioGroupValidationMessageArgs type FormControlParentArgs = Pick, 'required' | 'disabled'> -type FormControlLabelArgs = ComponentProps & {labelChildren?: React.ReactNode} +type FormControlLabelArgs = Omit, 'as'> & {labelChildren?: React.ReactNode} type FormControlCaptionArgs = ComponentProps & {captionChildren?: React.ReactNode} type FormControlValidationMessageArgs = ComponentProps & { validationChildren?: React.ReactNode From 656821a70058a41a9457f27c148f00ac84c53092 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 25 Jan 2023 13:19:32 -0500 Subject: [PATCH 02/11] changes SegmentedControl.IconButton to use aria-current instead of aria-pressed --- src/SegmentedControl/SegmentedControlIconButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SegmentedControl/SegmentedControlIconButton.tsx b/src/SegmentedControl/SegmentedControlIconButton.tsx index 3ba844b6f67..ad506696ac4 100644 --- a/src/SegmentedControl/SegmentedControlIconButton.tsx +++ b/src/SegmentedControl/SegmentedControlIconButton.tsx @@ -47,7 +47,7 @@ export const SegmentedControlIconButton: React.FC From ef9b17ccd8d85cf3bff806eb0a3c240c4b95182e Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 25 Jan 2023 13:43:22 -0500 Subject: [PATCH 03/11] nicer small screen experience for the SegmentedControl story that shows it associated with a label and caption --- .../SegmentedControlFeatures.stories.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/SegmentedControl/SegmentedControlFeatures.stories.tsx b/src/SegmentedControl/SegmentedControlFeatures.stories.tsx index 58cc79629bf..b2969cea172 100644 --- a/src/SegmentedControl/SegmentedControlFeatures.stories.tsx +++ b/src/SegmentedControl/SegmentedControlFeatures.stories.tsx @@ -129,10 +129,17 @@ 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 ` ``` -### 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/src/FormControl/FormControl.docs.json b/src/FormControl/FormControl.docs.json index 1ee9f06913d..8f5aac8fa21 100644 --- a/src/FormControl/FormControl.docs.json +++ b/src/FormControl/FormControl.docs.json @@ -54,7 +54,7 @@ "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" From b2d52f719240139a47dc3ab59121364b1c22226a Mon Sep 17 00:00:00 2001 From: mperrotti Date: Wed, 25 Jan 2023 21:45:05 +0000 Subject: [PATCH 07/11] Update generated/components.json --- generated/components.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/generated/components.json b/generated/components.json index 70b8b9b7889..4842ed255c6 100644 --- a/generated/components.json +++ b/generated/components.json @@ -2717,6 +2717,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" From b76a4629472777144b355e264fa1609cd0838a5d Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 25 Jan 2023 16:48:53 -0500 Subject: [PATCH 08/11] adds changeset --- .changeset/eleven-jokes-think.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eleven-jokes-think.md 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. From ebe5623af895172389e0c818fe7053da29802736 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 27 Jan 2023 17:16:31 -0500 Subject: [PATCH 09/11] fixes bug that broke selected segment test --- src/SegmentedControl/SegmentedControl.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx index 29916e736de..8eecf06b710 100644 --- a/src/SegmentedControl/SegmentedControl.tsx +++ b/src/SegmentedControl/SegmentedControl.tsx @@ -71,7 +71,7 @@ const Root: React.FC> = ({ const selectedSegments = React.Children.toArray(children).map( child => React.isValidElement(child) && - (isUncontrolled ? child.props.defaultSelected : child.props.selected), + (child.props.defaultSelected || child.props.selected), ) const hasSelectedButton = selectedSegments.some(isSelected => isSelected) const selectedIndexExternal = hasSelectedButton ? selectedSegments.indexOf(true) : 0 From 6c29939802d1550081c4fb1e7fb3b1eab8c74f24 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 27 Jan 2023 17:17:31 -0500 Subject: [PATCH 10/11] fixes styling issue where segment separation border was appearing on top of the outline --- src/SegmentedControl/SegmentedControl.tsx | 3 --- .../__snapshots__/SegmentedControl.test.tsx.snap | 8 ++++++++ src/SegmentedControl/getSegmentedControlStyles.ts | 4 ++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx index 8eecf06b710..1bac47b3d1d 100644 --- a/src/SegmentedControl/SegmentedControl.tsx +++ b/src/SegmentedControl/SegmentedControl.tsx @@ -9,8 +9,6 @@ import {ResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' import {ViewportRangeKeys} from '../utils/types/ViewportRangeKeys' import styled from 'styled-components' import {defaultSxProp} from '../utils/defaultSxProp' -import VisuallyHidden from '../_VisuallyHidden' -import {useId} from '../hooks/useId' type WidthOnlyViewportRangeKeys = Exclude @@ -60,7 +58,6 @@ const Root: React.FC> = ({ }) => { const segmentedControlContainerRef = useRef(null) const {theme} = useTheme() - const labelId = useId() const isUncontrolled = onChange === undefined || React.Children.toArray(children).some( diff --git a/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap b/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap index 46e1ec3c4f8..c0bb18bff45 100644 --- a/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap +++ b/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap @@ -27,6 +27,10 @@ exports[`SegmentedControl renders consistently 1`] = ` width: 1px; } +.c1:focus-within:has(:focus-visible) { + --separator-color: transparent; +} + .c1:first-child { margin-left: -1px; } @@ -61,6 +65,10 @@ exports[`SegmentedControl renders consistently 1`] = ` width: 1px; } +.c3:focus-within:has(:focus-visible) { + --separator-color: transparent; +} + .c3:first-child { margin-left: -1px; } diff --git a/src/SegmentedControl/getSegmentedControlStyles.ts b/src/SegmentedControl/getSegmentedControlStyles.ts index 1c0651ca4ea..95c7a19bf6e 100644 --- a/src/SegmentedControl/getSegmentedControlStyles.ts +++ b/src/SegmentedControl/getSegmentedControlStyles.ts @@ -115,5 +115,9 @@ export const getSegmentedControlListItemStyles = () => ({ marginTop: '-1px', marginBottom: '-1px', ':not(:last-child)': borderedSegment, + // Needed to hide the segment border when the button is focused. Without this, the segment border overlaps the focus outline. + ':focus-within:has(:focus-visible)': { + '--separator-color': 'transparent', + }, ...directChildLayoutAdjustments, }) From f01f4376ff2d0c9ea61a0f9287ebe657e7f27c64 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 3 Feb 2023 19:42:09 -0500 Subject: [PATCH 11/11] fixes linting issue --- src/SegmentedControl/SegmentedControlFeatures.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SegmentedControl/SegmentedControlFeatures.stories.tsx b/src/SegmentedControl/SegmentedControlFeatures.stories.tsx index a59038e52ea..b244058aec4 100644 --- a/src/SegmentedControl/SegmentedControlFeatures.stories.tsx +++ b/src/SegmentedControl/SegmentedControlFeatures.stories.tsx @@ -132,7 +132,7 @@ IconOnly.storyName = 'Icon only' export const AssociatedWithALabelAndCaption = () => ( ({ + sx={theme => ({ flexDirection: 'column', gap: theme.space[1], [`@media screen and (min-width: ${theme.breakpoints[1]})`]: {