From 48b8350f6f8b3fe71d0710d25cc6b62bf20077c0 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 20 Mar 2024 11:31:28 -0700 Subject: [PATCH 1/3] Do not apply `aria-hidden` to empty icons - primarily affects loading icons - they may still have perfectly valid `aria-label`s that should be accessible to screen readers --- src/components/icon/icon.test.tsx | 65 ++++++++++++++++++++++++------- src/components/icon/icon.tsx | 16 ++++---- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/src/components/icon/icon.test.tsx b/src/components/icon/icon.test.tsx index 5080fade0fb..fb58c69822d 100644 --- a/src/components/icon/icon.test.tsx +++ b/src/components/icon/icon.test.tsx @@ -30,19 +30,29 @@ jest.mock('./icon', () => { beforeEach(() => clearIconComponentCache()); -const testIcon = (props: PropsOf) => async () => { - act(() => { - render(); - }); - await waitFor( - () => { - const icon = document.querySelector(`[data-icon-type=${props.type}]`); - expect(icon).toHaveAttribute('data-is-loaded', 'true'); - expect(icon).toMatchSnapshot(); - }, - { timeout: 3000 } // CI will sometimes time out if the icon doesn't load fast enough - ); -}; +const testIcon = + ( + props: PropsOf, + assertion?: (icon: Element | null) => void + ) => + async () => { + act(() => { + render(); + }); + await waitFor( + () => { + const icon = document.querySelector(`[data-icon-type=${props.type}]`); + expect(icon).toHaveAttribute('data-is-loaded', 'true'); + + if (assertion) { + assertion(icon); + } else { + expect(icon).toMatchSnapshot(); + } + }, + { timeout: 3000 } // CI will sometimes time out if the icon doesn't load fast enough + ); + }; describe('EuiIcon', () => { test('is rendered', testIcon({ type: 'search', ...requiredProps })); @@ -130,6 +140,35 @@ describe('EuiIcon', () => { testIcon({ type: 'search', tabIndex: 0 }) ); }); + + describe('aria-hidden', () => { + it( + 'enforces aria-hidden if no title or label has been passed', + testIcon({ type: 'empty', 'aria-hidden': false }, (icon) => { + expect(icon).toHaveAttribute('aria-hidden', 'true'); + }) + ); + + it( + 'does not set aria-hidden if a title/label is passed', + testIcon( + { type: 'empty', title: 'Anything', 'aria-label': 'Anything' }, + (icon) => { + expect(icon).not.toHaveAttribute('aria-hidden'); + } + ) + ); + + it( + 'allows consumers to override aria-hidden even if a title/label exists', + testIcon( + { type: 'empty', title: 'Anything', 'aria-hidden': true }, + (icon) => { + expect(icon).toHaveAttribute('aria-hidden', 'true'); + } + ) + ); + }); }); it('renders custom components', () => { diff --git a/src/components/icon/icon.tsx b/src/components/icon/icon.tsx index a8569a461f8..3f33450ec1e 100644 --- a/src/components/icon/icon.tsx +++ b/src/components/icon/icon.tsx @@ -296,14 +296,12 @@ export class EuiIconClass extends PureComponent< } else { const Svg = icon; - // If it's an empty icon, or if there is no aria-label, aria-labelledby, or title it gets aria-hidden true - const isAriaHidden = - icon === empty || - !( - this.props['aria-label'] || - this.props['aria-labelledby'] || - this.props.title - ); + // If there is no aria-label, aria-labelledby, or title it gets aria-hidden true + const isAriaHidden = !( + this.props['aria-label'] || + this.props['aria-labelledby'] || + this.props.title + ); // If no aria-label or aria-labelledby is provided but there's a title, a titleId is generated // The svg aria-labelledby attribute gets this titleId @@ -326,7 +324,7 @@ export class EuiIconClass extends PureComponent< data-is-loaded={isLoaded || undefined} data-is-loading={isLoading || undefined} {...rest} - aria-hidden={isAriaHidden || undefined} + aria-hidden={isAriaHidden || rest['aria-hidden']} /> ); } From 65f24f82c5a054a14ec3bcea127c1a8767955ab6 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 20 Mar 2024 11:56:26 -0700 Subject: [PATCH 2/3] changelog --- changelogs/upcoming/7606.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/upcoming/7606.md diff --git a/changelogs/upcoming/7606.md b/changelogs/upcoming/7606.md new file mode 100644 index 00000000000..94d8aceac57 --- /dev/null +++ b/changelogs/upcoming/7606.md @@ -0,0 +1,3 @@ +**Accessibility** + +- `EuiIcons` no longer apply `aria-hidden` to empty icons, as long as a valid title or label is provided to the icon. In particular, this is intended to improve the accessibility of loading `EuiIconTip`s. From a2816f7032cc8577235f84f13f6d3948a7bd2c95 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 20 Mar 2024 11:57:47 -0700 Subject: [PATCH 3/3] [EuiIconTip] Fix non-i18n'd aria-label --- changelogs/upcoming/7606.md | 4 ++++ src/components/tool_tip/icon_tip.tsx | 31 ++++++++++++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/changelogs/upcoming/7606.md b/changelogs/upcoming/7606.md index 94d8aceac57..ba11d1f23d9 100644 --- a/changelogs/upcoming/7606.md +++ b/changelogs/upcoming/7606.md @@ -1,3 +1,7 @@ +**Bug fixes** + +- Fixed `EuiIconTip`'s default `aria-label` text to be i18n tokenizable + **Accessibility** - `EuiIcons` no longer apply `aria-hidden` to empty icons, as long as a valid title or label is provided to the icon. In particular, this is intended to improve the accessibility of loading `EuiIconTip`s. diff --git a/src/components/tool_tip/icon_tip.tsx b/src/components/tool_tip/icon_tip.tsx index 1493357372b..b8fcd876e6a 100644 --- a/src/components/tool_tip/icon_tip.tsx +++ b/src/components/tool_tip/icon_tip.tsx @@ -9,6 +9,7 @@ import React, { FunctionComponent } from 'react'; import { PropsOf } from '../common'; +import { useEuiI18n } from '../i18n'; import { EuiIcon, IconSize, IconType } from '../icon'; import { EuiToolTip, EuiToolTipProps } from './tool_tip'; @@ -53,22 +54,26 @@ type Props = Omit & export const EuiIconTip: FunctionComponent = ({ type = 'questionInCircle', - 'aria-label': ariaLabel = 'Info', + 'aria-label': ariaLabel, color, size, iconProps, position = 'top', delay = 'regular', ...rest -}) => ( - - - -); +}) => { + const defaultAriaLabel = useEuiI18n('euiIconTip.defaultAriaLabel', 'Info'); + + return ( + + + + ); +};