Skip to content

Commit

Permalink
[EuiCollapsibleNavBeta] Improve collapsed button icon screen reader UX (
Browse files Browse the repository at this point in the history
  • Loading branch information
cee-chen authored Aug 11, 2023
1 parent 1a10ca9 commit 4b95e45
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ exports[`EuiCollapsibleNavItem renders a collapsed button icon when in a collaps
data-test-subj="test subject string"
>
<button
aria-label="Item"
class="euiButtonIcon euiCollapsedNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsedNavButton"
data-test-subj="euiCollapsedNavButton"
type="button"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ exports[`EuiCollapsedNavButton renders a tooltip around the icon button 1`] = `
>
<button
aria-describedby="generated-id"
aria-label="Nav item"
class="euiButtonIcon euiCollapsedNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsedNavButton"
data-test-subj="euiCollapsedNavButton"
type="button"
Expand Down Expand Up @@ -54,6 +55,7 @@ exports[`EuiCollapsedNavButton renders isSelected 1`] = `
class="euiToolTipAnchor emotion-euiToolTipAnchor-block"
>
<a
aria-label="Nav item"
class="euiButtonIcon euiCollapsedNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsedNavButton-isSelected"
data-test-subj="euiCollapsedNavButton"
href="#"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ exports[`EuiCollapsedNavItem renders a popover if items exist 1`] = `
class="euiToolTipAnchor emotion-euiToolTipAnchor-block"
>
<button
aria-label="Item"
class="euiButtonIcon euiCollapsedNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsedNavButton"
data-test-subj="euiCollapsedNavButton"
type="button"
Expand All @@ -35,6 +36,7 @@ exports[`EuiCollapsedNavItem renders an icon button/link if items are missing or
data-test-subj="test subject string"
>
<a
aria-label="Item"
class="euiButtonIcon euiCollapsedNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsedNavButton"
data-test-subj="euiCollapsedNavButton"
href="#"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ exports[`EuiCollapsedNavPopover renders the title and sub-items within the popov
class="euiToolTipAnchor emotion-euiToolTipAnchor-block"
>
<button
aria-label="Item"
class="euiButtonIcon euiCollapsedNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsedNavButton"
data-test-subj="euiCollapsedNavButton"
type="button"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const EuiCollapsedNavButton: FunctionComponent<
color="text"
href={href}
onClick={onClick}
aria-label={title}
{...(linkProps as EuiButtonIconPropsForAnchor)} // Exclusive union shenanigans
className={buttonClassName}
css={buttonCssStyles}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,7 @@
* Side Public License, v 1.
*/

import React, {
FunctionComponent,
ReactNode,
HTMLAttributes,
useContext,
} from 'react';
import React, { FunctionComponent, HTMLAttributes, useContext } from 'react';
import classNames from 'classnames';

import { useEuiTheme } from '../../../services';
Expand Down Expand Up @@ -65,11 +60,11 @@ export type _SharedEuiCollapsibleNavItemProps = HTMLAttributes<HTMLElement> &

export type EuiCollapsibleNavItemProps = {
/**
* ReactNode to render as this component's title
* Required text to render as the nav item title
*/
title: ReactNode;
title: string;
/**
* Allows customizing title's element.
* Allows customizing the title element.
* Consider using a heading element for better accessibility.
* Defaults to an unsemantic `span` or `div`, depending on context.
*/
Expand Down
32 changes: 32 additions & 0 deletions src/components/tool_tip/tool_tip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ describe('EuiToolTip', () => {
).not.toBeNull();

jest.useRealTimers();

component.unmount(); // Enzyme's portals stay in the DOM otherwise
});

test('display prop renders block', () => {
Expand Down Expand Up @@ -150,6 +152,36 @@ describe('EuiToolTip', () => {
expect(removeEventSpy).toHaveBeenCalledWith('scroll', repositionFn, true);
});

describe('aria-describedby', () => {
it('by default, sets an `aria-describedby` on the anchor when the tooltip is visible', async () => {
const { getByTestSubject } = render(
<EuiToolTip content="Tooltip content" id="toolTipId">
<button data-test-subj="anchor" />
</EuiToolTip>
);
fireEvent.mouseOver(getByTestSubject('anchor'));
await waitForEuiToolTipVisible();

expect(
getByTestSubject('anchor').getAttribute('aria-describedby')
).toEqual('toolTipId');
});

it('merges with custom consumer `aria-describedby`s', async () => {
const { getByTestSubject } = render(
<EuiToolTip content="Tooltip content" id="toolTipId">
<button data-test-subj="anchor" aria-describedby="customId" />
</EuiToolTip>
);
fireEvent.mouseOver(getByTestSubject('anchor'));
await waitForEuiToolTipVisible();

expect(
getByTestSubject('anchor').getAttribute('aria-describedby')
).toEqual('toolTipId customId');
});
});

describe('ref methods', () => {
// Although we don't publicly recommend it, consumers may need to reach into EuiToolTip
// class methods to manually control visibility state via `show/hideToolTip`.
Expand Down
22 changes: 5 additions & 17 deletions src/components/tool_tip/tool_tip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import React, {
} from 'react';
import classNames from 'classnames';

import { CommonProps, keysOf } from '../common';
import { CommonProps } from '../common';
import { findPopoverPosition, htmlIdGenerator } from '../../services';
import { enqueueStateChange } from '../../services/react';
import { EuiResizeObserver } from '../observer/resize_observer';
Expand All @@ -26,14 +26,8 @@ import { EuiToolTipAnchor } from './tool_tip_anchor';
import { EuiToolTipArrow } from './tool_tip_arrow';
import { toolTipManager } from './tool_tip_manager';

const positionsToClassNameMap: { [key in ToolTipPositions]: string } = {
top: 'euiToolTip--top',
right: 'euiToolTip--right',
bottom: 'euiToolTip--bottom',
left: 'euiToolTip--left',
};

export const POSITIONS = keysOf(positionsToClassNameMap);
export const POSITIONS = ['top', 'right', 'bottom', 'left'] as const;
const DISPLAYS = ['inlineBlock', 'block'] as const;

export type ToolTipDelay = 'regular' | 'long';

Expand All @@ -50,11 +44,6 @@ interface ToolTipStyles {
visibility?: 'hidden';
}

const displayToClassNameMap = {
inlineBlock: undefined,
block: 'euiToolTipAnchor--displayBlock',
};

const DEFAULT_TOOLTIP_STYLES: ToolTipStyles = {
// position the tooltip content near the top-left
// corner of the window so it can't create scrollbars
Expand Down Expand Up @@ -92,7 +81,7 @@ export interface EuiToolTipProps extends CommonProps {
/**
* Common display alternatives for the anchor wrapper
*/
display?: keyof typeof displayToClassNameMap;
display?: (typeof DISPLAYS)[number];
/**
* Delay before showing tooltip. Good for repeatable items.
*/
Expand All @@ -116,7 +105,6 @@ export interface EuiToolTipProps extends CommonProps {
* When nesting an `EuiTooltip` in a scrollable container, `repositionOnScroll` should be `true`
*/
repositionOnScroll?: boolean;

/**
* If supplied, called when mouse movement causes the tool tip to be
* hidden.
Expand Down Expand Up @@ -337,7 +325,7 @@ export class EuiToolTip extends Component<EuiToolTipProps, State> {
onFocus={this.onFocus}
onMouseOver={this.showToolTip}
onMouseOut={this.onMouseOut}
id={this.state.id}
id={id}
className={anchorClasses}
display={display!}
isVisible={visible}
Expand Down
32 changes: 14 additions & 18 deletions src/components/tool_tip/tool_tip_anchor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,21 @@
* Side Public License, v 1.
*/

import React, {
cloneElement,
HTMLAttributes,
ReactElement,
forwardRef,
} from 'react';
import React, { cloneElement, HTMLAttributes, forwardRef } from 'react';
import classNames from 'classnames';

import type { EuiToolTipProps } from './tool_tip';
import { euiToolTipAnchorStyles } from './tool_tip.styles';

export interface EuiToolTipAnchorProps
extends Omit<
HTMLAttributes<HTMLSpanElement>,
'onBlur' | 'onFocus' | 'children'
> {
onBlur: () => void;
onFocus: () => void;
children: ReactElement;
isVisible: boolean;
display: 'block' | 'inlineBlock';
}
export type EuiToolTipAnchorProps = Omit<
HTMLAttributes<HTMLSpanElement>,
'onBlur' | 'onFocus' | 'children'
> &
Required<Pick<EuiToolTipProps, 'display' | 'children'>> & {
onBlur: () => void;
onFocus: () => void;
isVisible: boolean;
};

export const EuiToolTipAnchor = forwardRef<
HTMLSpanElement,
Expand Down Expand Up @@ -79,7 +73,9 @@ export const EuiToolTipAnchor = forwardRef<
onBlur();
children.props.onBlur && children.props.onBlur(e);
},
...(isVisible && { 'aria-describedby': id }),
'aria-describedby': isVisible
? classNames(id, children.props['aria-describedby'])
: children.props['aria-describedby'],
})}
</span>
);
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/7055.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed `EuiToolTip` overriding instead of merging its `aria-describedby` tooltip ID with any existing `aria-describedby`s

0 comments on commit 4b95e45

Please sign in to comment.