Skip to content

Commit

Permalink
[EuiIcon] Remove logic that applies aria-hidden to empty/loading ic…
Browse files Browse the repository at this point in the history
…ons; [EuiIconTip] Fix non-i18n'd default `aria-label` (#7606)
  • Loading branch information
cee-chen authored Mar 20, 2024
1 parent 23c99c3 commit 18dc7ec
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 35 deletions.
7 changes: 7 additions & 0 deletions changelogs/upcoming/7606.md
Original file line number Diff line number Diff line change
@@ -0,0 +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.
65 changes: 52 additions & 13 deletions src/components/icon/icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,29 @@ jest.mock('./icon', () => {

beforeEach(() => clearIconComponentCache());

const testIcon = (props: PropsOf<typeof EuiIcon>) => async () => {
act(() => {
render(<EuiIcon {...props} />);
});
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<typeof EuiIcon>,
assertion?: (icon: Element | null) => void
) =>
async () => {
act(() => {
render(<EuiIcon {...props} />);
});
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 }));
Expand Down Expand Up @@ -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', () => {
Expand Down
16 changes: 7 additions & 9 deletions src/components/icon/icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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']}
/>
);
}
Expand Down
31 changes: 18 additions & 13 deletions src/components/tool_tip/icon_tip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -53,22 +54,26 @@ type Props = Omit<EuiToolTipProps, 'children' | 'delay' | 'position'> &

export const EuiIconTip: FunctionComponent<Props> = ({
type = 'questionInCircle',
'aria-label': ariaLabel = 'Info',
'aria-label': ariaLabel,
color,
size,
iconProps,
position = 'top',
delay = 'regular',
...rest
}) => (
<EuiToolTip position={position} delay={delay} {...rest}>
<EuiIcon
tabIndex={0}
type={type}
color={color}
size={size}
aria-label={ariaLabel}
{...iconProps}
/>
</EuiToolTip>
);
}) => {
const defaultAriaLabel = useEuiI18n('euiIconTip.defaultAriaLabel', 'Info');

return (
<EuiToolTip position={position} delay={delay} {...rest}>
<EuiIcon
tabIndex={0}
type={type}
color={color}
size={size}
aria-label={ariaLabel || defaultAriaLabel}
{...iconProps}
/>
</EuiToolTip>
);
};

0 comments on commit 18dc7ec

Please sign in to comment.