Skip to content

Commit

Permalink
Merge pull request #545 from department-of-veterans-affairs/feature/5…
Browse files Browse the repository at this point in the history
…32-533-narin-checkbox-accessibility

[Feature] Checkbox/CheckboxGroup: Add heading role and list position
  • Loading branch information
narin authored Oct 28, 2024
2 parents 255f68c + d3a44d0 commit 862011a
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 28 deletions.
49 changes: 49 additions & 0 deletions packages/components/src/components/Checkbox/Checkbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ describe('Checkbox', () => {
onPress: onPressSpy,
}

const labelObject = { text: 'Label text object', a11yLabel: 'Label a11y' }
const descriptionObject = {
text: 'Description text object',
a11yLabel: 'Description a11y',
}

const errorMsg = 'Error text'

describe('Basic tests', () => {
Expand Down Expand Up @@ -75,6 +81,49 @@ describe('Checkbox', () => {
})
})

describe('Accessibility', () => {
it('should have a11y labels when label and description are strings', () => {
render(<Checkbox {...commonProps} />)
expect(
screen.getByLabelText('Label text, Description text'),
).toBeOnTheScreen()
})

it('should include required in a11y label', () => {
render(<Checkbox {...commonProps} required />)
expect(
screen.getByLabelText('Label text, Required, Description text'),
).toBeOnTheScreen()
})

it('should have a11y labels when label and description are TextWithA11y objects', () => {
render(
<Checkbox
{...commonProps}
label={labelObject}
description={descriptionObject}
required
/>,
)
expect(
screen.getByLabelText('Label a11y, Required, Description a11y'),
).toBeOnTheScreen()
})

it('should have a11y labels when label and description are objects without a11y', () => {
render(
<Checkbox
{...commonProps}
label={{ text: 'Label without a11y' }}
description={{ text: 'Description without a11y' }}
/>,
)
expect(
screen.getByLabelText('Label without a11y, Description without a11y'),
).toBeOnTheScreen()
})
})

describe('Styling', () => {
describe('Light mode', () => {
it('icon color (unchecked)', async () => {
Expand Down
15 changes: 14 additions & 1 deletion packages/components/src/components/Checkbox/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
useWindowDimensions,
} from 'react-native'
import { spacing } from '@department-of-veterans-affairs/mobile-tokens'
import { useTranslation } from 'react-i18next'
import React, { FC } from 'react'

import { CheckboxRadioProps, FormElementProps } from '../../types/forms'
Expand All @@ -20,7 +21,7 @@ import {
} from '../shared/FormText'
import { Icon, IconProps } from '../Icon/Icon'
import { Spacer } from '../Spacer/Spacer'
import { useTheme } from '../../utils'
import { getA11yLabel, useTheme } from '../../utils'

export type CheckboxProps = FormElementProps &
CheckboxRadioProps & {
Expand All @@ -31,6 +32,7 @@ export type CheckboxProps = FormElementProps &
}

export const Checkbox: FC<CheckboxProps> = ({
a11yListPosition,
checked,
label,
description,
Expand All @@ -44,6 +46,7 @@ export const Checkbox: FC<CheckboxProps> = ({
tile,
}) => {
const theme = useTheme()
const { t } = useTranslation()
const fontScale = useWindowDimensions().fontScale

/**
Expand Down Expand Up @@ -115,6 +118,14 @@ export const Checkbox: FC<CheckboxProps> = ({
</View>
)

/**
* Combined a11yLabel on Pressable required for Android Talkback
*/
const a11yLabel =
getA11yLabel(label) +
(required ? ', ' + t('required') : '') +
(description ? `, ${getA11yLabel(description)}` : '')

return (
<ComponentWrapper>
<View style={containerStyle} testID={testID}>
Expand All @@ -131,6 +142,8 @@ export const Checkbox: FC<CheckboxProps> = ({
onPress={onPress}
style={tile ? tileStyle : pressableBaseStyle}
aria-checked={indeterminate ? 'mixed' : checked}
aria-valuetext={a11yListPosition}
aria-label={a11yLabel}
role="checkbox">
{_icon}
<Spacer size="xs" horizontal />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ describe('CheckboxGroup', () => {
expect(checkboxes[4].props.accessibilityState.checked).toBe(false)
expect(checkboxes[5].props.accessibilityState.checked).toBe(false)
})

it('should have accessibilityValues on checkboxes', () => {
render(<CheckboxGroup {...commonProps} />)
const checkboxes = screen.queryAllByRole('checkbox')
expect(checkboxes[0].props.accessibilityValue.text).toBe('1 of 6')
expect(checkboxes[1].props.accessibilityValue.text).toBe('2 of 6')
expect(checkboxes[2].props.accessibilityValue.text).toBe('3 of 6')
expect(checkboxes[3].props.accessibilityValue.text).toBe('4 of 6')
expect(checkboxes[4].props.accessibilityValue.text).toBe('5 of 6')
expect(checkboxes[5].props.accessibilityValue.text).toBe('6 of 6')
})
})

describe('onPress behavior', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { View, ViewStyle } from 'react-native'
import { spacing } from '@department-of-veterans-affairs/mobile-tokens'
import { useTranslation } from 'react-i18next'
import React, { FC, Fragment } from 'react'

import { Checkbox } from '../Checkbox/Checkbox'
Expand Down Expand Up @@ -99,6 +100,7 @@ export const CheckboxGroup: FC<CheckboxGroupProps> = ({
tile,
}) => {
const theme = useTheme()
const { t } = useTranslation()

const handleCheckboxChange = (value: string | number) => {
if (selectedItems.includes(value)) {
Expand Down Expand Up @@ -141,11 +143,16 @@ export const CheckboxGroup: FC<CheckboxGroupProps> = ({
{items.map((item, index) => {
const isObject = typeof item === 'object'
const value = isObject ? item.value || item.text : item
const a11yListPosition = t('listPosition', {
position: index + 1,
total: items.length,
})

return (
<Fragment key={`checkbox-group-item-${index}`}>
<Checkbox
label={item}
a11yListPosition={a11yListPosition}
description={isObject ? item.description : undefined}
checked={selectedItems.includes(value)}
onPress={() => handleCheckboxChange(value)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export const SegmentedControl: FC<SegmentedControlProps> = ({
a11yHints,
testIDs,
}) => {
const { t } = useTranslation()
const theme = useTheme()
const { t } = useTranslation()

useEffect(() => {
onChange(selected)
Expand Down Expand Up @@ -83,12 +83,11 @@ export const SegmentedControl: FC<SegmentedControlProps> = ({
const accessibilityLabel = a11yLabels
? a11yLabels[index] || labels[index]
: labels[index]
const accessibilityValue = {
text: t('listPosition', {
position: index + 1,
total: labels.length,
}),
}

const a11yListPosition = t('listPosition', {
position: index + 1,
total: labels.length,
})

// TODO: Replace with typography tokens
const font: TextStyle = {
Expand All @@ -111,8 +110,8 @@ export const SegmentedControl: FC<SegmentedControlProps> = ({
key={index}
widthPct={`${100 / labels.length}%`}
aria-label={accessibilityLabel}
aria-valuetext={a11yListPosition}
accessibilityHint={a11yHints ? a11yHints[index] : ''}
accessibilityValue={accessibilityValue}
role={'tab'}
accessibilityState={{ selected: isSelected }}
style={PressableOpacityStyle()}
Expand Down
11 changes: 3 additions & 8 deletions packages/components/src/components/shared/FormText.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('Form Text', () => {
it('should render text object and a11y label', () => {
render(<Header text={headerObject} />)
expect(screen.root).toHaveTextContent('Header text object')
expect(screen.getByRole('heading')).toBeOnTheScreen()
expect(screen.getByLabelText('Header a11y')).toBeOnTheScreen()
})

Expand Down Expand Up @@ -153,10 +154,9 @@ describe('Form Text', () => {
})

describe('Label', () => {
it('should render text and a11y label', () => {
it('should render text', () => {
render(<Label text={labelObject} />)
expect(screen.root).toHaveTextContent('Label text object')
expect(screen.getByLabelText('Label a11y')).toBeOnTheScreen()
expect(screen.root).toHaveStyle({
fontFamily: 'SourceSansPro-Regular',
})
Expand All @@ -170,7 +170,6 @@ describe('Form Text', () => {
it('should render text object without a11y label', () => {
render(<Label text={{ text: 'Label text object' }} />)
expect(screen.root).toHaveTextContent('Label text object')
expect(screen.getByLabelText('Label text object')).toBeOnTheScreen()
})

it('should render required text', () => {
Expand Down Expand Up @@ -210,10 +209,9 @@ describe('Form Text', () => {
})

describe('Description', () => {
it('should render text and a11y label', () => {
it('should render text', () => {
render(<Description text={descriptionObject} />)
expect(screen.root).toHaveTextContent('Description text object')
expect(screen.getByLabelText(', Description a11y')).toBeOnTheScreen()
})

it('should render simple string', () => {
Expand All @@ -224,9 +222,6 @@ describe('Form Text', () => {
it('should render text object without a11y label', () => {
render(<Description text={{ text: 'Description text object' }} />)
expect(screen.root).toHaveTextContent('Description text object')
expect(
screen.getByLabelText(', Description text object'),
).toBeOnTheScreen()
})

it('should render light mode color', () => {
Expand Down
14 changes: 3 additions & 11 deletions packages/components/src/components/shared/FormText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const Header: FC<HeaderProps> = ({ text, required }) => {
: getA11yLabel(text)

return (
<Text aria-label={ariaLabel} style={textStyle}>
<Text role="heading" aria-label={ariaLabel} style={textStyle}>
{getDisplayText(text)}
{required && <Text style={requiredStyle}>{` (*${t('required')})`}</Text>}
</Text>
Expand Down Expand Up @@ -145,12 +145,8 @@ export const Label: FC<LabelProps> = ({ text, error, required }) => {
color: theme.vadsColorForegroundError,
}

const ariaLabel = required
? getA11yLabel(text) + ', ' + t('required')
: getA11yLabel(text)

return (
<Text aria-label={ariaLabel} style={textStyle}>
<Text style={textStyle}>
{getDisplayText(text)}
{required && <Text style={requiredStyle}>{` (*${t('required')})`}</Text>}
</Text>
Expand All @@ -170,9 +166,5 @@ export const Description: FC<FormTextProps> = ({ text }) => {
color: theme.vadsColorForegroundDefault,
}

return (
<Text aria-label={`, ${getA11yLabel(text)}`} style={textStyle}>
{getDisplayText(text)}
</Text>
)
return <Text style={textStyle}>{getDisplayText(text)}</Text>
}
2 changes: 2 additions & 0 deletions packages/components/src/types/forms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export type CheckboxRadioProps = {
label: StringOrTextWithA11y
/** OnPress logic to alter `checked` state or other behavior associated with the checkbox */
onPress: () => void
/** Textual description of position within list of checkboxes */
a11yListPosition?: string
/** Description that appears below label */
description?: StringOrTextWithA11y
/** True to apply tile styling */
Expand Down

0 comments on commit 862011a

Please sign in to comment.