Skip to content

Commit

Permalink
Add screen reader instructions on interacting with the flyout
Browse files Browse the repository at this point in the history
- allow for custom `role`s that consumers pass in that aren't `dialog`s (likely very rare, may need more testing)

- remove `closeButtonAriaLabel`. Consumers can use `closeButtonProps['aria-label']` instead

+ misc cleanup - remove `Fragment`

TODO: write tests
  • Loading branch information
cee-chen committed Feb 2, 2023
1 parent e79cd0d commit 6e0c98c
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 47 deletions.
25 changes: 1 addition & 24 deletions src/components/flyout/flyout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import React from 'react';
import { render, mount } from 'enzyme';
import { mount } from 'enzyme';
import { requiredProps, takeMountedSnapshot } from '../../test';
import { shouldRenderCustomStyles } from '../../test/internal';

Expand Down Expand Up @@ -76,29 +76,6 @@ describe('EuiFlyout', () => {
).toMatchSnapshot();
});

describe('closeButtonAriaLabel', () => {
test('has a default label for the close button', () => {
const component = render(<EuiFlyout onClose={() => {}} />);
const label = component
.find('[data-test-subj="euiFlyoutCloseButton"]')
.prop('aria-label');
expect(label).toBe('Close this dialog');
});

test('sets a custom label for the close button', () => {
const component = render(
<EuiFlyout
onClose={() => {}}
closeButtonAriaLabel="Closes specific flyout"
/>
);
const label = component
.find('[data-test-subj="euiFlyoutCloseButton"]')
.prop('aria-label');
expect(label).toBe('Closes specific flyout');
});
});

test('accepts div props', () => {
const component = mount(<EuiFlyout onClose={() => {}} id="imaflyout" />);

Expand Down
80 changes: 57 additions & 23 deletions src/components/flyout/flyout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import React, {
ComponentPropsWithRef,
CSSProperties,
ElementType,
Fragment,
FunctionComponent,
MouseEvent as ReactMouseEvent,
MutableRefObject,
Expand All @@ -28,6 +27,7 @@ import {
EuiBreakpointSize,
useIsWithinMinBreakpoint,
useEuiTheme,
useGeneratedHtmlId,
} from '../../services';
import { logicalStyle } from '../../global_styling';

Expand All @@ -38,6 +38,7 @@ import { EuiButtonIcon, EuiButtonIconPropsForButton } from '../button';
import { EuiI18n } from '../i18n';
import { useResizeObserver } from '../observer/resize_observer';
import { EuiPortal } from '../portal';
import { EuiScreenReaderOnly } from '../accessibility';

import { euiFlyoutStyles, euiFlyoutCloseButtonStyles } from './flyout.styles';

Expand Down Expand Up @@ -93,11 +94,6 @@ interface _EuiFlyoutProps {
* @default false
*/
hideCloseButton?: boolean;
/**
* Specify a custom aria-label for the close button of the flyout.
* @default "Close this dialog"
*/
closeButtonAriaLabel?: string;
/**
* Extends EuiButtonIconProps onto the close button
*/
Expand Down Expand Up @@ -180,7 +176,6 @@ export const EuiFlyout = forwardRef(
as,
hideCloseButton = false,
closeButtonProps,
closeButtonAriaLabel,
closeButtonPosition = 'inside',
onClose,
ownFocus = true,
Expand Down Expand Up @@ -305,14 +300,18 @@ export const EuiFlyout = forwardRef(
];

closeButton = (
<EuiI18n token="euiFlyout.closeAriaLabel" default="Close this dialog">
<EuiI18n
token="euiFlyout.closeAriaLabel"
default="Close this {role}"
values={{ role: role || as }}
>
{(closeAriaLabel: string) => (
<EuiButtonIcon
css={closeButtonCssStyles}
display={closeButtonPosition === 'outside' ? 'fill' : 'empty'}
iconType="cross"
color="text"
aria-label={closeButtonAriaLabel || closeAriaLabel}
aria-label={closeAriaLabel}
data-test-subj="euiFlyoutCloseButton"
{...closeButtonProps}
className={closeButtonClasses}
Expand Down Expand Up @@ -355,7 +354,51 @@ export const EuiFlyout = forwardRef(
shards: [...fixedHeaders, ...(_focusTrapProps.shards || [])],
};

/*
* Provide meaningful screen reader instructions/details
*/
const hasOverlayMask = ownFocus && !isPushed;
const descriptionId = useGeneratedHtmlId();

// TODO: We need to re-examine how inaccessibly push flyouts behave
const screenReaderDescription = !isPushed && (
<EuiScreenReaderOnly>
<p id={descriptionId}>
<EuiI18n
token="euiFlyout.screenReaderEscapeToClose"
default="You are in a {role}. To close this {role}, press Escape."
values={{ role: role || as }}
/>{' '}
{hasOverlayMask && (
<EuiI18n
token="euiFlyout.screenReaderTapToClose"
default="Or tap/click outside the {role} on the shadowed overlay to close."
values={{ role: role || as }}
/>
)}{' '}
{fixedHeaders.length > 0 && (
<EuiI18n
token="euiFlyout.screenReaderFixedHeaders"
default="You can still continue tabbing through the page headers in addition to the {role}."
values={{ role: role || as }}
/>
)}
</p>
</EuiScreenReaderOnly>
);

/*
* Trap focus even when `ownFocus={false}`, otherwise closing
* the flyout won't return focus to the originating button.
*
* Set `clickOutsideDisables={true}` when `ownFocus={false}`
* to allow non-keyboard users the ability to interact with
* elements outside the flyout.
*
* Set `onClickOutside={onClose}` when `ownFocus` and `type` are the defaults,
* or if `outsideClickCloses={true}` to close on clicks that target
* (both mousedown and mouseup) the overlay mask.
*/
const onClickOutside = (event: MouseEvent | TouchEvent) => {
// Do not close the flyout for any external click
if (outsideClickCloses === false) return undefined;
Expand All @@ -369,18 +412,7 @@ export const EuiFlyout = forwardRef(
// Otherwise if ownFocus is false and outsideClickCloses is undefined, outside clicks should not close the flyout
return undefined;
};
/*
* Trap focus even when `ownFocus={false}`, otherwise closing
* the flyout won't return focus to the originating button.
*
* Set `clickOutsideDisables={true}` when `ownFocus={false}`
* to allow non-keyboard users the ability to interact with
* elements outside the flyout.
*
* Set `onClickOutside={onClose}` when `ownFocus` and `type` are the defaults,
* or if `outsideClickCloses={true}` to close on clicks that target
* (both mousedown and mouseup) the overlay mask.
*/

let flyout = (
<EuiFocusTrap
disabled={isPushed}
Expand All @@ -395,9 +427,11 @@ export const EuiFlyout = forwardRef(
className={classes}
tabIndex={0}
data-autofocus
aria-describedby={descriptionId}
style={newStyle}
ref={setRef}
>
{screenReaderDescription}
{closeButton}
{children}
</Element>
Expand All @@ -421,10 +455,10 @@ export const EuiFlyout = forwardRef(
}

return (
<Fragment>
<>
<EuiWindowEvent event="keydown" handler={onKeyDown} />
{flyout}
</Fragment>
</>
);
}
// React.forwardRef interferes with the inferred element type
Expand Down

0 comments on commit 6e0c98c

Please sign in to comment.