Tooltip: Ensure tooltip text is correctly announced by screenreaders (#76683)

* add aria-describedby, tooltip role and unit tests

Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>

* remove `delayShow` so tooltip text is announced correctly

* adjust IconButton, fix unit tests

* undo tooltip aria-label change

* only set aria-describedby if there's no aria-label

---------

Co-authored-by: L-M-K-B <48948963+L-M-K-B@users.noreply.github.com>
Co-authored-by: joshhunt <josh@trtr.co>
This commit is contained in:
Ashley Harrison 2023-10-18 16:05:20 +01:00 committed by GitHub
parent 5cc3a3f1ed
commit d632dd672c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 42 additions and 14 deletions

View File

@ -32,8 +32,6 @@ const meta: Meta<typeof IconButton> = {
tooltip: 'sample tooltip message', tooltip: 'sample tooltip message',
tooltipPlacement: 'top', tooltipPlacement: 'top',
variant: 'secondary', variant: 'secondary',
ariaLabel: 'this property is deprecated',
['aria-label']: 'sample aria-label content',
}, },
argTypes: { argTypes: {
tooltip: { tooltip: {

View File

@ -39,6 +39,7 @@ describe('Tooltip', () => {
expect(refObj.current).not.toBeNull(); expect(refObj.current).not.toBeNull();
}); });
it('to be shown on hover and be dismissable by pressing Esc key when show is undefined', async () => { it('to be shown on hover and be dismissable by pressing Esc key when show is undefined', async () => {
render( render(
<Tooltip content="Tooltip content"> <Tooltip content="Tooltip content">
@ -50,6 +51,7 @@ describe('Tooltip', () => {
await userEvent.keyboard('{Escape}'); await userEvent.keyboard('{Escape}');
expect(screen.queryByText('Tooltip content')).not.toBeInTheDocument(); expect(screen.queryByText('Tooltip content')).not.toBeInTheDocument();
}); });
it('is always visible when show prop is true', async () => { it('is always visible when show prop is true', async () => {
render( render(
<Tooltip content="Tooltip content" show={true}> <Tooltip content="Tooltip content" show={true}>
@ -61,6 +63,7 @@ describe('Tooltip', () => {
await userEvent.unhover(screen.getByText('On the page')); await userEvent.unhover(screen.getByText('On the page'));
expect(screen.getByText('Tooltip content')).toBeInTheDocument(); expect(screen.getByText('Tooltip content')).toBeInTheDocument();
}); });
it('is never visible when show prop is false', async () => { it('is never visible when show prop is false', async () => {
render( render(
<Tooltip content="Tooltip content" show={false}> <Tooltip content="Tooltip content" show={false}>
@ -70,4 +73,27 @@ describe('Tooltip', () => {
await userEvent.hover(screen.getByText('On the page')); await userEvent.hover(screen.getByText('On the page'));
expect(screen.queryByText('Tooltip content')).not.toBeInTheDocument(); expect(screen.queryByText('Tooltip content')).not.toBeInTheDocument();
}); });
it('exposes the tooltip text to screen readers', async () => {
render(
<Tooltip content="Tooltip content">
<button>On the page</button>
</Tooltip>
);
// if tooltip is not visible, description won't be set
expect(
screen.queryByRole('button', {
description: 'Tooltip content',
})
).not.toBeInTheDocument();
// tab to button to make tooltip visible
await userEvent.keyboard('{tab}');
expect(
await screen.findByRole('button', {
description: 'Tooltip content',
})
).toBeInTheDocument();
});
}); });

View File

@ -1,4 +1,4 @@
import React, { useCallback, useEffect } from 'react'; import React, { useCallback, useEffect, useId, useState } from 'react';
import { usePopperTooltip } from 'react-popper-tooltip'; import { usePopperTooltip } from 'react-popper-tooltip';
import { GrafanaTheme2 } from '@grafana/data'; import { GrafanaTheme2 } from '@grafana/data';
@ -24,7 +24,8 @@ export interface TooltipProps {
export const Tooltip = React.forwardRef<HTMLElement, TooltipProps>( export const Tooltip = React.forwardRef<HTMLElement, TooltipProps>(
({ children, theme, interactive, show, placement, content }, forwardedRef) => { ({ children, theme, interactive, show, placement, content }, forwardedRef) => {
const [controlledVisible, setControlledVisible] = React.useState(show); const [controlledVisible, setControlledVisible] = useState(show);
const tooltipId = useId();
useEffect(() => { useEffect(() => {
if (controlledVisible !== false) { if (controlledVisible !== false) {
@ -44,10 +45,9 @@ export const Tooltip = React.forwardRef<HTMLElement, TooltipProps>(
const { getArrowProps, getTooltipProps, setTooltipRef, setTriggerRef, visible, update } = usePopperTooltip({ const { getArrowProps, getTooltipProps, setTooltipRef, setTriggerRef, visible, update } = usePopperTooltip({
visible: show ?? controlledVisible, visible: show ?? controlledVisible,
placement: placement, placement,
interactive: interactive, interactive,
delayHide: interactive ? 100 : 0, delayHide: interactive ? 100 : 0,
delayShow: 150,
offset: [0, 8], offset: [0, 8],
trigger: ['hover', 'focus'], trigger: ['hover', 'focus'],
onVisibleChange: setControlledVisible, onVisibleChange: setControlledVisible,
@ -69,17 +69,23 @@ export const Tooltip = React.forwardRef<HTMLElement, TooltipProps>(
[forwardedRef, setTriggerRef] [forwardedRef, setTriggerRef]
); );
// if the child has an aria-label, this should take precedence over the tooltip content
const childHasAriaLabel = 'aria-label' in children.props;
return ( return (
<> <>
{React.cloneElement(children, { {React.cloneElement(children, {
ref: handleRef, ref: handleRef,
tabIndex: 0, // tooltip should be keyboard focusable tabIndex: 0, // tooltip trigger should be keyboard focusable
'aria-describedby': !childHasAriaLabel && visible ? tooltipId : undefined,
})} })}
{visible && ( {visible && (
<Portal> <Portal>
<div <div
data-testid={selectors.components.Tooltip.container} data-testid={selectors.components.Tooltip.container}
ref={setTooltipRef} ref={setTooltipRef}
id={tooltipId}
role="tooltip"
{...getTooltipProps({ className: style.container })} {...getTooltipProps({ className: style.container })}
> >
<div {...getArrowProps({ className: style.arrow })} /> <div {...getArrowProps({ className: style.arrow })} />

View File

@ -402,10 +402,8 @@ describe(`Traces Filters`, () => {
], ],
}, },
}; };
const removeLabel = screen.getAllByLabelText(`Remove`); const removeLabel = screen.getAllByLabelText(/Remove/);
await act(async () => {
await userEvent.click(removeLabel[1]); await userEvent.click(removeLabel[1]);
});
rerender( rerender(
<Filters <Filters

View File

@ -269,7 +269,7 @@ describe('AnnoListPanel', () => {
'anno-list-panel-1' 'anno-list-panel-1'
); );
expect(screen.getByText(/filter:/i)).toBeInTheDocument(); expect(screen.getByText(/filter:/i)).toBeInTheDocument();
expect(screen.getByText(/result email/i)).toBeInTheDocument(); expect(screen.getByRole('button', { name: /result email/i })).toBeInTheDocument();
}); });
}); });

View File

@ -86,7 +86,7 @@ const Avatar = ({ onClick, avatarUrl, login, email }: AvatarProps) => {
return ( return (
<Tooltip content={tooltipContent} theme="info" placement="top"> <Tooltip content={tooltipContent} theme="info" placement="top">
<button onClick={onAvatarClick} className={styles.avatar} aria-label={`Created by ${email}`}> <button onClick={onAvatarClick} className={styles.avatar}>
<img src={avatarUrl} alt="avatar icon" /> <img src={avatarUrl} alt="avatar icon" />
</button> </button>
</Tooltip> </Tooltip>