From 5589a1a27c6aac277399f0a1f5e56a8715a66289 Mon Sep 17 00:00:00 2001
From: Constance Chen <constance.chen.3@gmail.com>
Date: Wed, 1 Feb 2023 16:45:39 -0800
Subject: [PATCH] Add screen reader instructions on interacting with the flyout

- 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
---
 src/components/flyout/flyout.test.tsx | 25 +--------
 src/components/flyout/flyout.tsx      | 80 +++++++++++++++++++--------
 2 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/src/components/flyout/flyout.test.tsx b/src/components/flyout/flyout.test.tsx
index 90c5e6fb7d42..0010ac2e2680 100644
--- a/src/components/flyout/flyout.test.tsx
+++ b/src/components/flyout/flyout.test.tsx
@@ -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';
 
@@ -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" />);
 
diff --git a/src/components/flyout/flyout.tsx b/src/components/flyout/flyout.tsx
index 90f05ad2ae37..d2eab98a43f1 100644
--- a/src/components/flyout/flyout.tsx
+++ b/src/components/flyout/flyout.tsx
@@ -14,7 +14,6 @@ import React, {
   ComponentPropsWithRef,
   CSSProperties,
   ElementType,
-  Fragment,
   FunctionComponent,
   MouseEvent as ReactMouseEvent,
   MutableRefObject,
@@ -28,6 +27,7 @@ import {
   EuiBreakpointSize,
   useIsWithinMinBreakpoint,
   useEuiTheme,
+  useGeneratedHtmlId,
 } from '../../services';
 import { logicalStyle } from '../../global_styling';
 
@@ -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';
 
@@ -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
    */
@@ -180,7 +176,6 @@ export const EuiFlyout = forwardRef(
       as,
       hideCloseButton = false,
       closeButtonProps,
-      closeButtonAriaLabel,
       closeButtonPosition = 'inside',
       onClose,
       ownFocus = true,
@@ -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}
@@ -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;
@@ -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}
@@ -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>
@@ -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