Skip to content

Commit 7d69bb4

Browse files
joshuaellisjoshuaellis
authored
Merge pull request #746 from strapi/enhancement/make-icon-buttons-more-accessible
2 parents 7b6e6eb + 95bf489 commit 7d69bb4

File tree

10 files changed

+183
-49
lines changed

10 files changed

+183
-49
lines changed

‎packages/strapi-design-system/.eslintrc.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
module.exports = {
2+
env: {
3+
jest: true,
4+
browser: true,
5+
node: true,
6+
},
27
parserOptions: {
38
ecmaVersion: 2020, // Allows for the parsing of modern ECMAScript features
49
sourceType: 'module', // Allows for the use of imports
@@ -19,6 +24,7 @@ module.exports = {
1924
'react/sort-prop-types': 1,
2025
'default-case': 'error',
2126
'no-unused-vars': ['error', { vars: 'all', args: 'after-used', ignoreRestSiblings: false }],
27+
'no-undef': 'error',
2228
},
2329
overrides: [
2430
{

‎packages/strapi-design-system/src/BaseCheckbox/BaseCheckbox.stories.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ The `BaseCheckbox` can have an indeterminate state.
4141
<Canvas>
4242
<Story name="indeterminate">
4343
{() => {
44-
const [checkedItems, setCheckedItems] = React.useState([true, false]);
44+
const [checkedItems, setCheckedItems] = useState([true, false]);
4545
const allChecked = checkedItems.every(Boolean);
4646
const isIndeterminate = checkedItems.some(Boolean) && !allChecked;
4747
return (

‎packages/strapi-design-system/src/Checkbox/Checkbox.stories.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import { Checkbox } from '@strapi/design-system/Checkbox';
5555
}}
5656
>
5757
{() => {
58-
const [checkedItems, setCheckedItems] = React.useState([true, false]);
58+
const [checkedItems, setCheckedItems] = useState([true, false]);
5959
const allChecked = checkedItems.every(Boolean);
6060
const isIndeterminate = checkedItems.some(Boolean) && !allChecked;
6161
return (

‎packages/strapi-design-system/src/DatePicker/__tests__/DatePicker.spec.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('DatePicker', () => {
1616
locale={locale}
1717
selectedDateLabel={(formattedDate) => `Date picker, current is ${formattedDate}`}
1818
selectedDate={new Date('Tue Sep 06 2022')}
19+
onChange={() => null}
1920
/>
2021
</ThemeProvider>,
2122
);

‎packages/strapi-design-system/src/DismissibleLayer/__test__/DimissibleLayer.spec.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import { DismissibleLayer } from '../DismissibleLayer';
55
describe('DismissibleLayer', () => {
66
it('should fire onEscapeKeyDown when the ESCAPE key is pressed', () => {
77
const onEscapeKeyDownMock = jest.fn();
8+
const onPointerDownOutsideMock = jest.fn();
89
render(
9-
<DismissibleLayer onEscapeKeyDown={onEscapeKeyDownMock}>
10+
<DismissibleLayer onEscapeKeyDown={onEscapeKeyDownMock} onPointerDownOutside={onPointerDownOutsideMock}>
1011
<h1>hello world</h1>
1112
</DismissibleLayer>,
1213
);
@@ -18,9 +19,10 @@ describe('DismissibleLayer', () => {
1819

1920
it('should fire onPointerDownOutside when the user clicks outside the DismissibleLayer', () => {
2021
const onPointerDownOutsideMock = jest.fn();
22+
const onEscapeKeyDownMock = jest.fn();
2123
render(
2224
<>
23-
<DismissibleLayer onPointerDownOutside={onPointerDownOutsideMock}>
25+
<DismissibleLayer onPointerDownOutside={onPointerDownOutsideMock} onEscapeKeyDown={onEscapeKeyDownMock}>
2426
<h1>hello world</h1>
2527
</DismissibleLayer>
2628
<button>Click me</button>

‎packages/strapi-design-system/src/IconButton/IconButton.js

Lines changed: 62 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import React from 'react';
1+
import React, { cloneElement } from 'react';
22
import PropTypes from 'prop-types';
33
import styled from 'styled-components';
44
import { Tooltip } from '../Tooltip';
55
import { BaseButton } from '../BaseButton';
66
import { Flex } from '../Flex';
7+
import { VisuallyHidden } from '../VisuallyHidden';
78

89
const IconButtonWrapper = styled(BaseButton)`
910
display: flex;
@@ -94,29 +95,42 @@ export const IconButtonGroup = styled(Flex)`
9495
}
9596
`;
9697

97-
export const IconButton = React.forwardRef(({ label, noBorder, icon, disabled, onClick, ...props }, ref) => {
98-
const handleClick = (e) => {
99-
if (!disabled && onClick) {
100-
onClick(e);
98+
export const IconButton = React.forwardRef(
99+
({ label, noBorder, children, icon, disabled, onClick, ['aria-label']: ariaLabel, ...restProps }, ref) => {
100+
/**
101+
* @type {React.MouseEventHandler<HTMLButtonElement>}
102+
*/
103+
const handleClick = (e) => {
104+
if (!disabled && onClick) {
105+
onClick(e);
106+
}
107+
};
108+
109+
if (!label) {
110+
return (
111+
<IconButtonWrapper {...restProps} ref={ref} noBorder={noBorder} onClick={handleClick} aria-disabled={disabled}>
112+
<VisuallyHidden as="span">{ariaLabel}</VisuallyHidden>
113+
{cloneElement(children ?? icon, {
114+
'aria-hidden': true,
115+
focusable: false, // See: https://allyjs.io/tutorials/focusing-in-svg.html#making-svg-elements-focusable
116+
})}
117+
</IconButtonWrapper>
118+
);
101119
}
102-
};
103120

104-
if (!label) {
105121
return (
106-
<IconButtonWrapper {...props} ref={ref} noBorder={noBorder} onClick={handleClick} aria-disabled={disabled}>
107-
{icon}
108-
</IconButtonWrapper>
122+
<Tooltip label={label}>
123+
<IconButtonWrapper {...restProps} ref={ref} noBorder={noBorder} onClick={handleClick} aria-disabled={disabled}>
124+
<VisuallyHidden as="span">{label}</VisuallyHidden>
125+
{cloneElement(children ?? icon, {
126+
'aria-hidden': true,
127+
focusable: false, // See: https://allyjs.io/tutorials/focusing-in-svg.html#making-svg-elements-focusable
128+
})}
129+
</IconButtonWrapper>
130+
</Tooltip>
109131
);
110-
}
111-
112-
return (
113-
<Tooltip label={label}>
114-
<IconButtonWrapper {...props} ref={ref} noBorder={noBorder} onClick={handleClick} aria-disabled={disabled}>
115-
{icon}
116-
</IconButtonWrapper>
117-
</Tooltip>
118-
);
119-
});
132+
},
133+
);
120134

121135
IconButton.displayName = 'IconButton';
122136

@@ -126,10 +140,36 @@ IconButton.defaultProps = {
126140
disabled: false,
127141
onClick: undefined,
128142
};
143+
144+
/**
145+
* @type {(otherProps: string[]) => (props: Record<string, unknown>, propName: string) => Error | undefined}
146+
*/
147+
const throwPropErrorIfNoneAreDefined = (otherProps, propType) => (props, propName) => {
148+
if (!props[propName] && otherProps.every((otherProp) => !props[otherProp])) {
149+
return new Error(`One of the following props is required: ${propName}, ${otherProps.join(', ')}`);
150+
}
151+
152+
PropTypes.checkPropTypes({ [propName]: PropTypes[propType] }, props, 'prop', 'IconButton');
153+
};
154+
129155
IconButton.propTypes = {
156+
/**
157+
* `PropTypes.string` – must be defined if `label` is not defined.
158+
*/
159+
['aria-label']: throwPropErrorIfNoneAreDefined(['label'], 'string'),
160+
/**
161+
* `PropTypes.node` – must be defined if `icon` is not defined.
162+
*/
163+
children: throwPropErrorIfNoneAreDefined(['icon'], 'node'),
130164
disabled: PropTypes.bool,
131-
icon: PropTypes.element.isRequired,
132-
label: PropTypes.string,
165+
/**
166+
* `PropTypes.node` – must be defined if `children` is not defined.
167+
*/
168+
icon: throwPropErrorIfNoneAreDefined(['children'], 'node'),
169+
/**
170+
* `PropTypes.string` – must be defined if `aria-label` is not defined.
171+
*/
172+
label: throwPropErrorIfNoneAreDefined(['aria-label'], 'string'),
133173
noBorder: PropTypes.bool,
134174
onClick: PropTypes.func,
135175
};

‎packages/strapi-design-system/src/IconButton/IconButton.stories.mdx

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22

33
import { Meta, Story, Canvas } from '@storybook/addon-docs';
44
import { ArgsTable } from '@storybook/addon-docs';
5+
import { useState } from 'react';
6+
57
import Pencil from '@strapi/icons/Pencil';
68
import Play from '@strapi/icons/Play';
79
import Trash from '@strapi/icons/Trash';
810
import Plus from '@strapi/icons/Plus';
11+
912
import { IconButton, IconButtonGroup } from './IconButton';
1013
import { Box } from '../Box';
1114
import { Flex } from '../Flex';
15+
import { Typography } from '../Typography';
1216

1317
<Meta title="Design System/Components/IconButton" component={IconButton} />
1418

@@ -38,14 +42,22 @@ Icon Buttons are used to perform actions within a page, a table or another objec
3842

3943
<Canvas>
4044
<Story name="base">
41-
<Box padding={7}>
42-
<Flex>
43-
<IconButton onClick={() => console.log('edit')} label="Edit" icon={<Pencil />} />
44-
<IconButton onClick={() => console.log('Create')} label="Create" icon={<Plus />} />
45-
<IconButton onClick={() => console.log('Delete')} label="Delete" icon={<Trash />} />
46-
<IconButton onClick={() => console.log('Publish')} label="Publish" icon={<Play />} />
47-
</Flex>
48-
</Box>
45+
{() => {
46+
const [currentAction, setCurrentAction] = useState('None Selected');
47+
return (
48+
<Box padding={7}>
49+
<Box paddingBottom={3}>
50+
<Typography>{currentAction}</Typography>
51+
</Box>
52+
<Flex>
53+
<IconButton onClick={() => setCurrentAction('edit')} label="Edit" icon={<Pencil />} />
54+
<IconButton onClick={() => setCurrentAction('Create')} label="Create" icon={<Plus />} />
55+
<IconButton onClick={() => setCurrentAction('Delete')} label="Delete" icon={<Trash />} />
56+
<IconButton onClick={() => setCurrentAction('Publish')} label="Publish" icon={<Play />} />
57+
</Flex>
58+
</Box>
59+
);
60+
}}
4961
</Story>
5062
</Canvas>
5163

@@ -55,14 +67,22 @@ Depending on the status of an action or the permissions, an IconButton can be un
5567

5668
<Canvas>
5769
<Story name="disabled">
58-
<Box padding={7}>
59-
<Flex>
60-
<IconButton disabled onClick={() => console.log('edit')} label="Edit" icon={<Pencil />} />
61-
<IconButton disabled onClick={() => console.log('Create')} label="Create" icon={<Plus />} />
62-
<IconButton disabled onClick={() => console.log('Delete')} label="Delete" icon={<Trash />} />
63-
<IconButton disabled onClick={() => console.log('Publish')} label="Publish" icon={<Play />} />
64-
</Flex>
65-
</Box>
70+
{() => {
71+
const [currentAction, setCurrentAction] = useState('None Selected');
72+
return (
73+
<Box padding={7}>
74+
<Box paddingBottom={3}>
75+
<Typography>{currentAction}</Typography>
76+
</Box>
77+
<Flex>
78+
<IconButton disabled onClick={() => setCurrentAction('edit')} label="Edit" icon={<Pencil />} />
79+
<IconButton disabled onClick={() => setCurrentAction('Create')} label="Create" icon={<Plus />} />
80+
<IconButton disabled onClick={() => setCurrentAction('Delete')} label="Delete" icon={<Trash />} />
81+
<IconButton disabled onClick={() => setCurrentAction('Publish')} label="Publish" icon={<Play />} />
82+
</Flex>
83+
</Box>
84+
);
85+
}}
6686
</Story>
6787
</Canvas>
6888

@@ -95,6 +115,39 @@ IconButtons can be used within another component is a Group shape.
95115
</Story>
96116
</Canvas>
97117

118+
### Icons as Children & aria-label
119+
120+
IconButtons can take Icons as their children and can be fully accessible without a tooltip.
121+
122+
<Canvas>
123+
<Story name="children">
124+
{() => {
125+
const [currentAction, setCurrentAction] = useState('None Selected');
126+
return (
127+
<Box padding={7}>
128+
<Box paddingBottom={3}>
129+
<Typography>{currentAction}</Typography>
130+
</Box>
131+
<Flex>
132+
<IconButton onClick={() => setCurrentAction('Edit')} aria-label={'Edit'}>
133+
<Pencil />
134+
</IconButton>
135+
<IconButton onClick={() => setCurrentAction('Create')} aria-label="Create">
136+
<Plus />
137+
</IconButton>
138+
<IconButton onClick={() => setCurrentAction('Delete')} aria-label="Delete">
139+
<Trash />
140+
</IconButton>
141+
<IconButton onClick={() => setCurrentAction('Publish')} aria-label="Publish">
142+
<Play />
143+
</IconButton>
144+
</Flex>
145+
</Box>
146+
);
147+
}}
148+
</Story>
149+
</Canvas>
150+
98151
## Props
99152

100153
<ArgsTable of={IconButton} />

‎packages/strapi-design-system/src/ModalLayout/__tests__/ModalLayout.spec.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,12 +506,18 @@ describe('ModalLayout', () => {
506506
Modal Title
507507
<button
508508
aria-disabled="false"
509-
aria-label="Close the modal"
510509
class="c8 c9"
511510
type="button"
512511
>
512+
<span
513+
class="c0"
514+
>
515+
Close the modal
516+
</span>
513517
<svg
518+
aria-hidden="true"
514519
fill="none"
520+
focusable="false"
515521
height="1em"
516522
viewBox="0 0 24 24"
517523
width="1em"

‎packages/strapi-design-system/src/Status/Status.stories.mdx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,25 @@ Use a Status sparingly within forms to give an important visual indication. Stat
4949
</Story>
5050
</Canvas>
5151

52-
5352
### Size
5453

5554
<Canvas>
5655
<Story name="size S">
5756
<Stack spacing={3}>
5857
<Status variant="success" size="S" showBullet={false}>
59-
<Typography fontWeight="bold" textColor="success700">Published</Typography>
58+
<Typography fontWeight="bold" textColor="success700">
59+
Published
60+
</Typography>
6061
</Status>
6162
<Status variant="secondary" size="S" showBullet={false}>
62-
<Typography fontWeight="bold" textColor="secondary700">Draft</Typography>
63+
<Typography fontWeight="bold" textColor="secondary700">
64+
Draft
65+
</Typography>
6366
</Status>
6467
<Status variant="alternative" size="S" showBullet={false}>
65-
<Typography fontWeight="bold" textColor="alternative700">Updated</Typography>
68+
<Typography fontWeight="bold" textColor="alternative700">
69+
Updated
70+
</Typography>
6671
</Status>
6772
</Stack>
6873
</Story>

‎packages/strapi-design-system/src/Switch/Switch.stories.mdx

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<!--- Switch.stories.mdx --->
22

3+
import { useState } from 'react';
34
import { Meta, Story, Canvas } from '@storybook/addon-docs';
45
import { ArgsTable } from '@storybook/addon-docs';
56
import { Switch } from './Switch';
@@ -36,7 +37,17 @@ Activated state is one of two of them available for a Switch.
3637

3738
<Canvas>
3839
<Story name="activated">
39-
<Switch label="Activate the microphone" selected={true} onChange={() => setChecked((s) => !s)} visibleLabels />
40+
{() => {
41+
const [activated, setActivated] = useState(true);
42+
return (
43+
<Switch
44+
label="Activate the microphone"
45+
selected={activated}
46+
onChange={() => setActivated((s) => !s)}
47+
visibleLabels
48+
/>
49+
);
50+
}}
4051
</Story>
4152
</Canvas>
4253

@@ -46,7 +57,17 @@ Deactivated state is one of two of them available for a Switch.
4657

4758
<Canvas>
4859
<Story name="not-activated">
49-
<Switch label="Activate the microphone" selected={false} onChange={() => setChecked((s) => !s)} visibleLabels />
60+
{() => {
61+
const [activated, setActivated] = useState(false);
62+
return (
63+
<Switch
64+
label="Activate the microphone"
65+
selected={activated}
66+
onChange={() => setActivated((s) => !s)}
67+
visibleLabels
68+
/>
69+
);
70+
}}
5071
</Story>
5172
</Canvas>
5273

0 commit comments

Comments
 (0)