From 03c1b090594a0a928e44822f5003285a97dc1b5a Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 10 Feb 2022 13:49:08 -0500 Subject: [PATCH 01/21] adds ChoiceGroup components and Storybook stories --- src/ChoiceFieldset/ChoiceFieldsetLegend.tsx | 1 - src/ChoiceGroup/ChoiceGroup.tsx | 178 +++++++++++++++ src/ChoiceGroup/_ChoiceGroupCaption.tsx | 16 ++ src/ChoiceGroup/_ChoiceGroupContext.tsx | 7 + src/ChoiceGroup/_ChoiceGroupLabel.tsx | 39 ++++ src/ChoiceGroup/_ChoiceGroupValidation.tsx | 22 ++ src/ChoiceGroup/index.ts | 2 + src/ChoiceGroup/slots.ts | 3 + src/FormControl/FormControl.tsx | 11 +- src/_InputValidation.tsx | 2 +- src/_ValidationAnimationContainer.tsx | 4 +- src/_VisuallyHidden.tsx | 2 +- src/expectedInputComponents.ts | 3 + src/stories/ChoiceGroup.stories.tsx | 228 ++++++++++++++++++++ 14 files changed, 509 insertions(+), 9 deletions(-) create mode 100644 src/ChoiceGroup/ChoiceGroup.tsx create mode 100644 src/ChoiceGroup/_ChoiceGroupCaption.tsx create mode 100644 src/ChoiceGroup/_ChoiceGroupContext.tsx create mode 100644 src/ChoiceGroup/_ChoiceGroupLabel.tsx create mode 100644 src/ChoiceGroup/_ChoiceGroupValidation.tsx create mode 100644 src/ChoiceGroup/index.ts create mode 100644 src/ChoiceGroup/slots.ts create mode 100644 src/expectedInputComponents.ts create mode 100644 src/stories/ChoiceGroup.stories.tsx diff --git a/src/ChoiceFieldset/ChoiceFieldsetLegend.tsx b/src/ChoiceFieldset/ChoiceFieldsetLegend.tsx index ce5ec7077aa..68341c889e7 100644 --- a/src/ChoiceFieldset/ChoiceFieldsetLegend.tsx +++ b/src/ChoiceFieldset/ChoiceFieldsetLegend.tsx @@ -14,7 +14,6 @@ const ChoiceFieldsetLegend: React.FC = ({children, vi {({required, disabled}: ChoiceFieldsetContext) => ( void + /** + * Whether this field must have a value for the user to complete their task + */ + required?: boolean + /** + * TODO: if we remove onSelect, also remove this + * The selected values + */ + selected?: string[] +} + +export type ChoiceGroupContext = { + validationMessageId: string +} & ChoiceGroupProps + +const Body = styled.div` + display: flex; + flex-direction: column; + list-style: none; + margin: 0; + padding: 0; + + > * + * { + margin-top: ${get('space.2')}; + } +` + +const ChoiceGroup = ({ + 'aria-labelledby': ariaLabelledby, + children, + disabled, + id, + name, + onSelect, + required, + selected +}: ChoiceGroupProps) => { + const fieldsetId = useSSRSafeId(id) + const validationChild = React.Children.toArray(children).find(child => + React.isValidElement(child) && child.type === ChoiceGroupValidation ? child : null + ) + const validationMessageId = validationChild ? `${fieldsetId}-validationMsg` : '' + const checkIfOnlyContainsChoiceInputs = () => { + const formControlComponentChildren = React.Children.toArray(children) + .filter(child => React.isValidElement(child) && child.type === FormControl) + .map(formControlComponent => + React.isValidElement(formControlComponent) ? formControlComponent.props.children : [] + ) + .flat() + + return React.Children.toArray(formControlComponentChildren) + .filter(child => + expectedInputComponents.some(inputComponent => React.isValidElement(child) && child.type === inputComponent) + ) + .every( + inputComponent => + React.isValidElement(inputComponent) && (inputComponent.type === Checkbox || inputComponent.type === Radio) + ) + } + + if (!checkIfOnlyContainsChoiceInputs()) { + // eslint-disable-next-line no-console + console.warn('Only `Checkbox` and `Radio` form controls should be used in a `ChoiceGroup`.') + } + + return ( + + {slots => { + const isLegendVisible = React.isValidElement(slots.Label) && slots.Label.props.isVisible + const hasLegendContent = slots.Label || slots.Caption || slots.Validation + + if (!slots.Label || !ariaLabelledby) { + // eslint-disable-next-line no-console + console.warn( + 'A choice group must be labelled using a `ChoiceGroup.Label` child, or by passing `aria-labelledby` to the ChoiceGroup component.' + ) + } + + return ( + +
+ + {hasLegendContent && ( + /* + Placing the caption text and validation text in the provides a better user experience for more screenreaders + Reference: https://blog.tenon.io/accessible-validation-of-checkbox-and-radiobutton-groups/ + */ + + {slots.Label} + {slots.Caption} + {slots.Validation} + + )} + + {React.Children.toArray(children).filter(child => React.isValidElement(child))} + + {validationChild && ( + + )} +
+ + ) + }} + + ) +} + +export type InputFieldComponentProps = ComponentProps +export type {ChoiceGroupLabelProps} from './_ChoiceGroupLabel' +export default Object.assign(ChoiceGroup, { + Caption: ChoiceGroupCaption, + Label: ChoiceGroupLabel, + Validation: ChoiceGroupValidation +}) diff --git a/src/ChoiceGroup/_ChoiceGroupCaption.tsx b/src/ChoiceGroup/_ChoiceGroupCaption.tsx new file mode 100644 index 00000000000..6443c7e7d3b --- /dev/null +++ b/src/ChoiceGroup/_ChoiceGroupCaption.tsx @@ -0,0 +1,16 @@ +import React from 'react' +import {Text} from '..' +import {ChoiceGroupContext} from './ChoiceGroup' +import {Slot} from './slots' + +const ChoiceGroupCaption: React.FC = ({children}) => ( + + {({disabled}: ChoiceGroupContext) => ( + + {children} + + )} + +) + +export default ChoiceGroupCaption diff --git a/src/ChoiceGroup/_ChoiceGroupContext.tsx b/src/ChoiceGroup/_ChoiceGroupContext.tsx new file mode 100644 index 00000000000..1bff0515e7b --- /dev/null +++ b/src/ChoiceGroup/_ChoiceGroupContext.tsx @@ -0,0 +1,7 @@ +import {createContext} from 'react' + +const ChoiceGroupContext = createContext<{ + disabled?: boolean +} | null>(null) + +export default ChoiceGroupContext diff --git a/src/ChoiceGroup/_ChoiceGroupLabel.tsx b/src/ChoiceGroup/_ChoiceGroupLabel.tsx new file mode 100644 index 00000000000..873dcf4418b --- /dev/null +++ b/src/ChoiceGroup/_ChoiceGroupLabel.tsx @@ -0,0 +1,39 @@ +import React from 'react' +import {Box} from '..' +import VisuallyHidden from '../_VisuallyHidden' +import {ChoiceGroupContext} from './ChoiceGroup' +import {Slot} from './slots' + +export interface ChoiceGroupLabelProps { + /** + * Whether to visually hide the fieldset legend + */ + visuallyHidden?: boolean +} + +const ChoiceGroupLabel: React.FC = ({children, visuallyHidden}) => ( + + {({required, disabled}: ChoiceGroupContext) => ( + + {required ? ( + + {children} + * + + ) : ( + children + )} + + )} + +) + +export default ChoiceGroupLabel diff --git a/src/ChoiceGroup/_ChoiceGroupValidation.tsx b/src/ChoiceGroup/_ChoiceGroupValidation.tsx new file mode 100644 index 00000000000..7a51a62537d --- /dev/null +++ b/src/ChoiceGroup/_ChoiceGroupValidation.tsx @@ -0,0 +1,22 @@ +import React from 'react' +import InputValidation from '../_InputValidation' +import {SxProp} from '../sx' +import {FormValidationStatus} from '../utils/types/FormValidationStatus' +import {ChoiceGroupContext} from './ChoiceGroup' +import {Slot} from './slots' + +export type ChoiceGroupValidationProps = { + variant: FormValidationStatus +} & SxProp + +const ChoiceGroupValidation: React.FC = ({children, variant, sx}) => ( + + {({validationMessageId}: ChoiceGroupContext) => ( + + {children} + + )} + +) + +export default ChoiceGroupValidation diff --git a/src/ChoiceGroup/index.ts b/src/ChoiceGroup/index.ts new file mode 100644 index 00000000000..2acbeb23bf4 --- /dev/null +++ b/src/ChoiceGroup/index.ts @@ -0,0 +1,2 @@ +export {default} from './ChoiceGroup' +export type {ChoiceGroupProps} from './ChoiceGroup' diff --git a/src/ChoiceGroup/slots.ts b/src/ChoiceGroup/slots.ts new file mode 100644 index 00000000000..75ba8a8171b --- /dev/null +++ b/src/ChoiceGroup/slots.ts @@ -0,0 +1,3 @@ +import createSlots from '../utils/create-slots' + +export const {Slots, Slot} = createSlots(['Caption', 'Label', 'Validation']) diff --git a/src/FormControl/FormControl.tsx b/src/FormControl/FormControl.tsx index 0091fc65a16..d888af8ed6d 100644 --- a/src/FormControl/FormControl.tsx +++ b/src/FormControl/FormControl.tsx @@ -1,5 +1,5 @@ -import React from 'react' -import {Autocomplete, Box, Checkbox, Radio, Select, Textarea, TextInput, TextInputWithTokens, useSSRSafeId} from '..' +import React, {useContext} from 'react' +import {Box, Checkbox, Radio, useSSRSafeId} from '..' import FormControlCaption from './_FormControlCaption' import FormControlLabel from './_FormControlLabel' import FormControlValidation from './_FormControlValidation' @@ -8,6 +8,8 @@ import ValidationAnimationContainer from '../_ValidationAnimationContainer' import {get} from '../constants' import FormControlLeadingVisual from './_FormControlLeadingVisual' import {SxProp} from '../sx' +import ChoiceGroupContext from '../ChoiceGroup/_ChoiceGroupContext' +import {expectedInputComponents} from '../expectedInputComponents' export type FormControlProps = { children?: React.ReactNode @@ -30,8 +32,9 @@ export interface FormControlContext extends Pick { - const expectedInputComponents = [Autocomplete, Checkbox, Radio, Select, TextInput, TextInputWithTokens, Textarea] +const FormControl = ({children, disabled: disabledProp, id: idProp, required, sx}: FormControlProps) => { + const choiceGroupContext = useContext(ChoiceGroupContext) + const disabled = choiceGroupContext?.disabled || disabledProp const id = useSSRSafeId(idProp) const validationChild = React.Children.toArray(children).find(child => React.isValidElement(child) && child.type === FormControlValidation ? child : null diff --git a/src/_InputValidation.tsx b/src/_InputValidation.tsx index 20aa0fd30f6..9b29e04798f 100644 --- a/src/_InputValidation.tsx +++ b/src/_InputValidation.tsx @@ -40,7 +40,7 @@ const InputValidation: React.FC = ({children, id, validationStatus, sx}) }} > {IconComponent && ( - + )} diff --git a/src/_ValidationAnimationContainer.tsx b/src/_ValidationAnimationContainer.tsx index c4206d3658a..591a8c9c745 100644 --- a/src/_ValidationAnimationContainer.tsx +++ b/src/_ValidationAnimationContainer.tsx @@ -1,8 +1,8 @@ -import React, {useEffect, useState} from 'react' +import React, {HTMLProps, useEffect, useState} from 'react' import styled, {keyframes, css} from 'styled-components' import {Box} from '.' -interface Props { +interface Props extends HTMLProps { show?: boolean } diff --git a/src/_VisuallyHidden.tsx b/src/_VisuallyHidden.tsx index 9237f91a086..abb75c85abf 100644 --- a/src/_VisuallyHidden.tsx +++ b/src/_VisuallyHidden.tsx @@ -26,7 +26,7 @@ const VisuallyHidden = styled.span` ` VisuallyHidden.defaultProps = { - isVisible: true + isVisible: false } export default VisuallyHidden diff --git a/src/expectedInputComponents.ts b/src/expectedInputComponents.ts new file mode 100644 index 00000000000..1da60ebb368 --- /dev/null +++ b/src/expectedInputComponents.ts @@ -0,0 +1,3 @@ +import {Autocomplete, Checkbox, Radio, Select, Textarea, TextInput, TextInputWithTokens} from './' + +export const expectedInputComponents = [Autocomplete, Checkbox, Radio, Select, TextInput, TextInputWithTokens, Textarea] diff --git a/src/stories/ChoiceGroup.stories.tsx b/src/stories/ChoiceGroup.stories.tsx new file mode 100644 index 00000000000..90e9bf1354e --- /dev/null +++ b/src/stories/ChoiceGroup.stories.tsx @@ -0,0 +1,228 @@ +import React, {ChangeEventHandler} from 'react' +import {Meta} from '@storybook/react' +import {BaseStyles, Box, Checkbox, FormControl, Radio, ThemeProvider} from '..' +import ChoiceGroup from '../ChoiceGroup' +import {ComponentProps} from '../utils/types' +import {useState} from '@storybook/addons' + +type Args = ComponentProps + +export default { + title: 'Forms/ChoiceGroup', + component: ChoiceGroup, + argTypes: { + disabled: { + defaultValue: false + }, + required: { + defaultValue: false + } + }, + parameters: {controls: {exclude: ['id', 'validationMap', 'validationResult', 'name', 'onSelect']}}, + decorators: [ + Story => { + return ( + + + + + + ) + } + ] +} as Meta + +export const RadioChoiceGroup = (args: Args) => ( + + Choices + + + Choice one + + + + Choice two + + + + Choice three + + +) +RadioChoiceGroup.storyName = 'Radio group (default)' + +export const CheckboxChoiceGroup = (args: Args) => ( + + Choices + + + Choice one + + + + Choice two + + + + Choice three + + +) +CheckboxChoiceGroup.parameters = {controls: {exclude: ['id', 'selectionVariant']}} +CheckboxChoiceGroup.storyName = 'Checkbox group' + +export const WithCaption = (args: Args) => ( + + Choices + You can pick any or all of these choices + + + Choice one + + + + Choice two + + + + Choice three + + +) + +export const WithExternalLabel = (args: Args) => ( + <> + + Choices + + + + + Choice one + + + + Choice two + + + + Choice three + + + +) + +export const WithHiddenLabel = (args: Args) => ( + + Choices + + + Choice one + + + + Choice two + + + + Choice three + + +) + +export const WithValidation = (args: Args) => ( + + Choices + + + Choice one + + + + Choice two + + + + Choice three + + Your choices are wrong + +) + +export const WithOnChangeHandlers = (args: Args) => { + const [selectedCheckboxValues, setSelectedCheckboxValues] = useState([]) + const [selectedRadioValue, setSelectedRadioValue] = useState('') + + const handleCheckboxChange: ChangeEventHandler = e => { + const {value, checked} = e.currentTarget + + if (checked) { + setSelectedCheckboxValues([...selectedCheckboxValues, value]) + return + } + + setSelectedCheckboxValues(selectedCheckboxValues.filter(selectedValue => selectedValue !== value)) + } + + const handleRadioChange: ChangeEventHandler = e => { + const {value, checked} = e.currentTarget + + if (checked) { + setSelectedRadioValue(value) + } + } + + return ( + + + + Checkboxes + + + Checkbox one + + + + Checkbox two + + + + Checkbox three + + + + {Boolean(selectedCheckboxValues.length) && ( +
The selected checkbox values are {selectedCheckboxValues.join(', ')}
+ )} + + + + + Radios + + + Radio one + + + + Radio two + + + + Radio three + + + + {selectedRadioValue &&
The selected radio value is {selectedRadioValue}
} + + + ) +} From b7c17dc6f73491f9ff2efbf730ee6624b3b59c94 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 10 Feb 2022 15:05:39 -0500 Subject: [PATCH 02/21] removes unnecessary code from ChoiceGroup --- src/ChoiceGroup/ChoiceGroup.tsx | 38 ++------------------------------- 1 file changed, 2 insertions(+), 36 deletions(-) diff --git a/src/ChoiceGroup/ChoiceGroup.tsx b/src/ChoiceGroup/ChoiceGroup.tsx index 255dc03686f..576736db2ec 100644 --- a/src/ChoiceGroup/ChoiceGroup.tsx +++ b/src/ChoiceGroup/ChoiceGroup.tsx @@ -16,35 +16,14 @@ export type ChoiceGroupProps = { * Used when associating the choice group with a label other than `ChoiceGroup.Label` */ ['aria-labelledby']?: string - children?: React.ReactNode /** * Whether the fieldset is NOT ready for user input */ disabled?: boolean - /** - * The unique identifier for this fieldset. Used to associate the validation text with the fieldset - * If an ID is not passed, one will be automatically generated - */ - id?: string - /** - * The unique identifier used to associate radio inputs with eachother - * If a name is not passed and the fieldset renders radio inputs, a name will be automatically generated - */ - name?: string - /** - * TODO: consider removing this - * The callback that is called when a user toggles a choice on or off - */ - onSelect?: (selectedValues: string[]) => void /** * Whether this field must have a value for the user to complete their task */ required?: boolean - /** - * TODO: if we remove onSelect, also remove this - * The selected values - */ - selected?: string[] } export type ChoiceGroupContext = { @@ -63,21 +42,11 @@ const Body = styled.div` } ` -const ChoiceGroup = ({ - 'aria-labelledby': ariaLabelledby, - children, - disabled, - id, - name, - onSelect, - required, - selected -}: ChoiceGroupProps) => { - const fieldsetId = useSSRSafeId(id) +const ChoiceGroup: React.FC = ({'aria-labelledby': ariaLabelledby, children, disabled, required}) => { const validationChild = React.Children.toArray(children).find(child => React.isValidElement(child) && child.type === ChoiceGroupValidation ? child : null ) - const validationMessageId = validationChild ? `${fieldsetId}-validationMsg` : '' + const validationMessageId = useSSRSafeId() const checkIfOnlyContainsChoiceInputs = () => { const formControlComponentChildren = React.Children.toArray(children) .filter(child => React.isValidElement(child) && child.type === FormControl) @@ -105,10 +74,7 @@ const ChoiceGroup = ({ From cd22868a2adbd7f272741a8e3b9a94da14aa5f74 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 10 Feb 2022 19:29:54 -0500 Subject: [PATCH 03/21] adds tests and fixes issues discovered during testing --- src/ChoiceGroup/ChoiceGroup.tsx | 93 +++++--- src/ChoiceGroup/_ChoiceGroupCaption.tsx | 4 +- src/FormControl/FormControl.tsx | 14 +- src/__tests__/ChoiceGroup.test.tsx | 273 ++++++++++++++++++++++++ src/expectedInputComponents.ts | 13 +- src/index.ts | 1 + 6 files changed, 359 insertions(+), 39 deletions(-) create mode 100644 src/__tests__/ChoiceGroup.test.tsx diff --git a/src/ChoiceGroup/ChoiceGroup.tsx b/src/ChoiceGroup/ChoiceGroup.tsx index 576736db2ec..6db0750741e 100644 --- a/src/ChoiceGroup/ChoiceGroup.tsx +++ b/src/ChoiceGroup/ChoiceGroup.tsx @@ -1,5 +1,5 @@ import React, {ComponentProps} from 'react' -import {Box, Checkbox, FormControl, Radio, useSSRSafeId} from '..' +import {Box, Checkbox, FormControl, Radio, useSSRSafeId} from '..' // Checkbox, FormControl, Radio, import ValidationAnimationContainer from '../_ValidationAnimationContainer' import ChoiceGroupCaption from './_ChoiceGroupCaption' import ChoiceGroupLabel from './_ChoiceGroupLabel' @@ -13,13 +13,18 @@ import VisuallyHidden from '../_VisuallyHidden' export type ChoiceGroupProps = { /** - * Used when associating the choice group with a label other than `ChoiceGroup.Label` + * Used when associating the input group with a label other than `ChoiceGroup.Label` */ ['aria-labelledby']?: string /** - * Whether the fieldset is NOT ready for user input + * Whether the input group allows user input */ disabled?: boolean + /** + * The unique identifier for this input group. Used to associate the label, validation text, and caption text. + * You may want a custom ID to make it easier to select elements in integration tests. + */ + id?: string /** * Whether this field must have a value for the user to complete their task */ @@ -27,7 +32,8 @@ export type ChoiceGroupProps = { } export type ChoiceGroupContext = { - validationMessageId: string + validationMessageId?: string + captionId?: string } & ChoiceGroupProps const Body = styled.div` @@ -42,11 +48,25 @@ const Body = styled.div` } ` -const ChoiceGroup: React.FC = ({'aria-labelledby': ariaLabelledby, children, disabled, required}) => { +const ChoiceGroup: React.FC = ({ + 'aria-labelledby': ariaLabelledby, + children, + disabled, + id: idProp, + required +}) => { + const labelChild = React.Children.toArray(children).find( + child => React.isValidElement(child) && child.type === ChoiceGroupLabel + ) const validationChild = React.Children.toArray(children).find(child => React.isValidElement(child) && child.type === ChoiceGroupValidation ? child : null ) - const validationMessageId = useSSRSafeId() + const captionChild = React.Children.toArray(children).find(child => + React.isValidElement(child) && child.type === ChoiceGroupCaption ? child : null + ) + const id = useSSRSafeId(idProp) + const validationMessageId = validationChild && `${id}-validationMessage` + const captionId = captionChild && `${id}-caption` const checkIfOnlyContainsChoiceInputs = () => { const formControlComponentChildren = React.Children.toArray(children) .filter(child => React.isValidElement(child) && child.type === FormControl) @@ -70,24 +90,24 @@ const ChoiceGroup: React.FC = ({'aria-labelledby': ariaLabelle console.warn('Only `Checkbox` and `Radio` form controls should be used in a `ChoiceGroup`.') } + if (!labelChild && !ariaLabelledby) { + // eslint-disable-next-line no-console + console.warn( + 'A choice group must be labelled using a `ChoiceGroup.Label` child, or by passing `aria-labelledby` to the ChoiceGroup component.' + ) + } + return ( {slots => { - const isLegendVisible = React.isValidElement(slots.Label) && slots.Label.props.isVisible - const hasLegendContent = slots.Label || slots.Caption || slots.Validation - - if (!slots.Label || !ariaLabelledby) { - // eslint-disable-next-line no-console - console.warn( - 'A choice group must be labelled using a `ChoiceGroup.Label` child, or by passing `aria-labelledby` to the ChoiceGroup component.' - ) - } + const isLegendVisible = React.isValidElement(labelChild) && labelChild.props.isVisible return ( @@ -97,20 +117,16 @@ const ChoiceGroup: React.FC = ({'aria-labelledby': ariaLabelle margin={0} mb={validationChild ? 2 : undefined} padding={0} - {...(hasLegendContent - ? { - as: 'fieldset', - disabled - } - : { - ['aria-describedby']: validationMessageId, - as: 'div', - role: 'group' - })} + {...(labelChild && { + as: 'fieldset', + disabled + })} > - {hasLegendContent && ( + {labelChild ? ( /* - Placing the caption text and validation text in the provides a better user experience for more screenreaders + Placing the caption text and validation text in the provides a better user + experience for more screenreaders. + Reference: https://blog.tenon.io/accessible-validation-of-checkbox-and-radiobutton-groups/ */ @@ -118,12 +134,31 @@ const ChoiceGroup: React.FC = ({'aria-labelledby': ariaLabelle {slots.Caption} {slots.Validation} + ) : ( + /* + If ChoiceGroup.Label wasn't passed as a child, we don't render a + but we still want to render a caption + */ + slots.Caption )} - {React.Children.toArray(children).filter(child => React.isValidElement(child))} + + {React.Children.toArray(children).filter(child => React.isValidElement(child))} + {validationChild && ( -