Skip to content

Commit 9fb1934

Browse files
wuyq0808wuyq0808
authored
[BD-9484][BpkInfoBannerExpandable] Fix an a11y issue (#3751)
* Fix a11y issue: element with button role can not be operable via keyboard * remove role from button * move view more label into the toggle button; add aria-controls; change section to div and remove role; remove deprecated comments * fix test * add type button for good practice * remove aria-controls * replace getByText by getByRole --------- Co-authored-by: Yongqi Wu <[email protected]>
1 parent 222ea31 commit 9fb1934

9 files changed

+180
-212
lines changed

‎packages/bpk-component-info-banner/src/BpkInfoBanner-test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ describe('BpkInfoBanner', () => {
5555
message={message}
5656
/>,
5757
);
58-
expect(screen.getByRole('presentation')).toHaveClass('custom-banner-class-name');
58+
expect(screen.getByText(message).parentElement?.parentElement).toHaveClass(
59+
'custom-banner-class-name',
60+
);
5961
});
6062
});

‎packages/bpk-component-info-banner/src/BpkInfoBanner.module.scss

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@
4242
align-items: flex-start;
4343

4444
&--expandable {
45+
width: 100%;
46+
padding: 0;
47+
border: none;
48+
background-color: transparent;
49+
text-align: start;
4550
cursor: pointer;
4651
}
4752
}
@@ -78,9 +83,6 @@
7883
background-color: transparent;
7984
cursor: pointer;
8085
appearance: none;
81-
}
82-
83-
&__expand-icon {
8486
fill: tokens.$bpk-text-secondary-day;
8587
}
8688

‎packages/bpk-component-info-banner/src/BpkInfoBannerExpandable-test.tsx

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import { render, screen } from '@testing-library/react';
2020
import '@testing-library/jest-dom';
21+
import { axe } from 'jest-axe';
2122

2223
import BpkInfoBannerExpandable from './BpkInfoBannerExpandable';
2324
import { ALERT_TYPES } from './common-types';
@@ -28,15 +29,17 @@ const innerMessage =
2829
'Quisque sagittis sagittis purus, id blandit ipsum. Pellentesque nec diam nec erat condimentum dapibus.' +
2930
'Nunc diam augue, egestas id egestas ut, facilisis nec mi.';
3031

32+
const defaultTestProps = {
33+
type: ALERT_TYPES.SUCCESS,
34+
message,
35+
toggleButtonLabel: 'Show details',
36+
onExpandToggle: jest.fn(),
37+
};
38+
3139
describe('BpkInfoBannerExpandable', () => {
3240
it('should not show inner message when "expanded" is not set to true', () => {
3341
render(
34-
<BpkInfoBannerExpandable
35-
type={ALERT_TYPES.SUCCESS}
36-
message={message}
37-
toggleButtonLabel="Show details"
38-
onExpandToggle={jest.fn()}
39-
>
42+
<BpkInfoBannerExpandable {...defaultTestProps}>
4043
{innerMessage}
4144
</BpkInfoBannerExpandable>,
4245
);
@@ -45,16 +48,20 @@ describe('BpkInfoBannerExpandable', () => {
4548

4649
it('should show inner message when "expanded" is set to true', () => {
4750
render(
48-
<BpkInfoBannerExpandable
49-
type={ALERT_TYPES.SUCCESS}
50-
message={message}
51-
toggleButtonLabel="Show details"
52-
onExpandToggle={jest.fn()}
53-
expanded
54-
>
51+
<BpkInfoBannerExpandable {...defaultTestProps} expanded>
5552
{innerMessage}
5653
</BpkInfoBannerExpandable>,
5754
);
5855
expect(screen.getByText(innerMessage)).toBeVisible();
5956
});
57+
58+
it('should not have programmatically-detectable accessibility issues', async () => {
59+
const { container } = render(
60+
<BpkInfoBannerExpandable {...defaultTestProps}>
61+
{innerMessage}
62+
</BpkInfoBannerExpandable>,
63+
);
64+
const results = await axe(container);
65+
expect(results).toHaveNoViolations();
66+
});
6067
});

‎packages/bpk-component-info-banner/src/BpkInfoBannerInner.tsx

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -91,23 +91,12 @@ type ToggleButtonProps = {
9191
expanded: boolean;
9292
};
9393

94-
const ToggleButton = (props: ToggleButtonProps) => {
95-
const classNames = getClassName('bpk-info-banner__expand-icon');
96-
97-
return (
98-
<button
99-
type="button"
100-
className={getClassName('bpk-info-banner__toggle-button')}
101-
aria-label={props.label}
102-
aria-expanded={props.expanded}
103-
title={props.label}
104-
>
105-
<div className={classNames}>
106-
{props.expanded ? <CollapseIcon/> : <ExpandIcon/> }
107-
</div>
108-
</button>
109-
);
110-
};
94+
const ToggleButton = (props: ToggleButtonProps) => (
95+
<div className={getClassName('bpk-info-banner__toggle-button')}>
96+
{props.expanded ? <CollapseIcon /> : <ExpandIcon />}
97+
<span className="visually-hidden">{props.label}</span>
98+
</div>
99+
);
111100

112101
type Props = CommonProps & {
113102
action?: ExpandableBannerAction;
@@ -159,7 +148,7 @@ const BpkInfoBannerInner = ({
159148
const dismissable = configuration === CONFIGURATION.DISMISSABLE;
160149
const showChildren = isExpandable && expanded;
161150

162-
const sectionClassNames = getClassName(
151+
const classNames = getClassName(
163152
'bpk-info-banner',
164153
`bpk-info-banner--${type}`,
165154
`bpk-info-banner--style-${style}`,
@@ -175,22 +164,21 @@ const BpkInfoBannerInner = ({
175164
? getClassName('bpk-info-banner__children-container--with-action')
176165
: getClassName('bpk-info-banner__children-container--no-action')
177166

178-
/* eslint-disable
179-
jsx-a11y/no-static-element-interactions,
180-
jsx-a11y/click-events-have-key-events,
181-
*/
182-
// Disabling 'click-events-have-key-events and interactive-supports-focus' because header element is not focusable.
183-
// ToggleButton is focusable and works for this.
167+
const BannerHeader = isExpandable ? 'button' : 'div';
168+
184169
return (
185170
<AnimateAndFade
186171
animateOnEnter={animateOnEnter}
187172
animateOnLeave={dismissable || animateOnLeave}
188173
show={show}
189174
{...rest}
190175
>
191-
<section className={sectionClassNames} role="presentation">
192-
<div
193-
role={isExpandable ? 'button' : undefined}
176+
<div className={classNames}>
177+
<BannerHeader
178+
aria-expanded={isExpandable ? expanded : undefined}
179+
type={isExpandable ? 'button' : undefined}
180+
// BannerHeader is just <button> or <div>, so className should be allowed.
181+
// eslint-disable-next-line @skyscanner/rules/forbid-component-props
194182
className={headerClassNames}
195183
onClick={onBannerExpandToggle}
196184
>
@@ -214,7 +202,7 @@ const BpkInfoBannerInner = ({
214202
/>
215203
</span>
216204
)}
217-
</div>
205+
</BannerHeader>
218206
<BpkAnimateHeight
219207
duration={parseInt(durationSm, 10)}
220208
height={showChildren ? 'auto' : 0}
@@ -230,10 +218,9 @@ const BpkInfoBannerInner = ({
230218
</BpkLink>
231219
)}
232220
</BpkAnimateHeight>
233-
</section>
221+
</div>
234222
</AnimateAndFade>
235223
);
236-
/* eslint-enable */
237224
};
238225

239226
export default BpkInfoBannerInner;

‎packages/bpk-component-info-banner/src/__snapshots__/BpkInfoBanner-test.tsx.snap

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@ exports[`BpkInfoBanner should render correctly 1`] = `
88
>
99
<div>
1010
<div>
11-
<section
11+
<div
1212
class="bpk-info-banner bpk-info-banner--success bpk-info-banner--style-default"
13-
role="presentation"
1413
>
1514
<div
1615
class="bpk-info-banner__header"
@@ -55,7 +54,7 @@ exports[`BpkInfoBanner should render correctly 1`] = `
5554
/>
5655
</div>
5756
</div>
58-
</section>
57+
</div>
5958
</div>
6059
</div>
6160
</div>
@@ -73,9 +72,8 @@ exports[`BpkInfoBanner should render correctly with no type specified 1`] = `
7372
>
7473
<div>
7574
<div>
76-
<section
75+
<div
7776
class="bpk-info-banner bpk-info-banner--info bpk-info-banner--style-default"
78-
role="presentation"
7977
>
8078
<div
8179
class="bpk-info-banner__header"
@@ -120,7 +118,7 @@ exports[`BpkInfoBanner should render correctly with no type specified 1`] = `
120118
/>
121119
</div>
122120
</div>
123-
</section>
121+
</div>
124122
</div>
125123
</div>
126124
</div>

‎packages/bpk-component-info-banner/src/__snapshots__/BpkInfoBannerDismissable-test.tsx.snap

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@ exports[`BpkInfoBannerDismissable should render correctly 1`] = `
88
>
99
<div>
1010
<div>
11-
<section
11+
<div
1212
class="bpk-info-banner bpk-info-banner--success bpk-info-banner--style-default"
13-
role="presentation"
1413
>
1514
<div
1615
class="bpk-info-banner__header"
@@ -81,7 +80,7 @@ exports[`BpkInfoBannerDismissable should render correctly 1`] = `
8180
/>
8281
</div>
8382
</div>
84-
</section>
83+
</div>
8584
</div>
8685
</div>
8786
</div>

0 commit comments

Comments
 (0)