[EuiFlyout] Resolve disparity between push flyouts and overlay flyouts

src-docs/src/views/flyout/flyout_example.js

@@ -1,4 +1,4 @@
-import React, { Fragment } from 'react';
+import React from 'react';
 import { Link } from 'react-router-dom';
 
 import { GuideSectionTypes } from '../../components';
@@ -10,6 +10,7 @@ import {
   EuiFlyoutHeader,
   EuiFlyoutFooter,
   EuiCallOut,
+  EuiLink,
 } from '../../../../src/components';
 
 import Flyout from './flyout';
@@ -315,7 +316,7 @@ export const FlyoutExample = {
         },
       ],
       text: (
-        <Fragment>
+        <>
           <p>
             Another way to allow for continued interactions of the page content
             while a flyout is visible, is to change the <EuiCode>type</EuiCode>{' '}
@@ -329,7 +330,29 @@ export const FlyoutExample = {
             window widths. You can adjust this minimum breakpoint with{' '}
             <EuiCode>pushMinBreakpoint</EuiCode>.
           </p>
-        </Fragment>
+          <EuiCallOut
+            iconType="accessibility"
+            title="Push flyouts require manual accessibility management"
+            color="warning"
+          >
+            <p>
+              Push flyouts do not use a focus trap, do not close on Escape
+              keypress, do not inherit a{' '}
+              <EuiLink
+                href="https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/dialog_role"
+                target="_blank"
+              >
+                dialog role
+              </EuiLink>
+              , and do not include any of the default screen reader guidance
+              that overlay flyouts contain out-of-the-box.
+            </p>
+            <p>
+              Please be cautious when using push flyouts, and make sure you are
+              managing your own focus and screen reader UX.
+            </p>
+          </EuiCallOut>
+        </>
       ),
       snippet: flyoutPushedSnippet,
       demo: <FlyoutPush />,
@@ -344,7 +367,7 @@ export const FlyoutExample = {
         },
       ],
       text: (
-        <Fragment>
+        <>
           <p>
             By default, flyouts will continue to grow with the width of the
             window. To stop this growth at an ideal width, set{' '}
@@ -355,7 +378,7 @@ export const FlyoutExample = {
             color="warning"
             title="Note that there are some caveats to providing a maxWidth that is smaller than the minWidth."
           />
-        </Fragment>
+        </>
       ),
       snippet: flyoutMaxWidthSnippet,
       demo: <FlyoutMaxWidth />,

src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap

@@ -274,11 +274,8 @@ exports[`EuiCollapsibleNav props isDocked 1`] = `
   >
     <nav
       class="euiFlyout euiCollapsibleNav emotion-euiFlyout-none-noMaxWidth-push-left-left-euiCollapsibleNav-push"
-      data-autofocus="true"
       id="id"
-      role="dialog"
       style="inline-size: 320px;"
-      tabindex="0"
     />
   </div>
   <div
@@ -357,11 +354,8 @@ exports[`EuiCollapsibleNav props showButtonIfDocked 1`] = `
   >
     <nav
       class="euiFlyout euiCollapsibleNav emotion-euiFlyout-none-noMaxWidth-push-left-left-euiCollapsibleNav-push"
-      data-autofocus="true"
       id="id"
-      role="dialog"
       style="inline-size: 320px;"
-      tabindex="0"
     />
   </div>
   <div

src/components/collapsible_nav_beta/__snapshots__/collapsible_nav_beta.test.tsx.snap

@@ -37,12 +37,9 @@ exports[`EuiCollapsibleNavBeta renders 1`] = `
       <nav
         aria-label="aria-label"
         class="euiFlyout euiCollapsibleNav euiCollapsibleNavBeta testClass1 testClass2 emotion-euiFlyout-none-noMaxWidth-push-left-left-euiCollapsibleNavBeta-left-isPush-euiTestCss"
-        data-autofocus="true"
         data-test-subj="nav"
         id="generated-id_euiCollapsibleNav"
-        role="dialog"
         style="inline-size: 248px;"
-        tabindex="0"
       >
         Nav content
       </nav>
@@ -91,13 +88,11 @@ exports[`EuiCollapsibleNavBeta renders initialIsCollapsed 1`] = `
       data-focus-lock-disabled="disabled"
     >
       <nav
+        aria-label="Site menu"
         class="euiFlyout euiCollapsibleNav euiCollapsibleNavBeta emotion-euiFlyout-none-noMaxWidth-push-left-left-euiCollapsibleNavBeta-left-isPush-isPushCollapsed"
-        data-autofocus="true"
         data-test-subj="nav"
         id="generated-id_euiCollapsibleNav"
-        role="dialog"
         style="inline-size: 48px;"
-        tabindex="0"
       >
         Nav content
       </nav>
@@ -183,6 +178,7 @@ exports[`EuiCollapsibleNavBeta responsive behavior makes the overlay flyout full
     >
       <nav
         aria-describedby="generated-id"
+        aria-label="Site menu"
         class="euiFlyout euiCollapsibleNav euiCollapsibleNavBeta emotion-euiFlyout-none-noMaxWidth-overlay-left-euiCollapsibleNavBeta-left-isOverlayFullWidth"
         data-autofocus="true"
         id="generated-id_euiCollapsibleNav"

src/components/collapsible_nav_beta/collapsible_nav_beta.tsx

@@ -23,6 +23,7 @@ import { mathWithUnits, logicalStyle } from '../../global_styling';
 
 import { CommonProps } from '../common';
 import { EuiFlyout, EuiFlyoutProps } from '../flyout';
+import { useEuiI18n } from '../i18n';
 import { euiHeaderVariables } from '../header/header.styles';
 
 import { EuiCollapsibleNavContext } from './context';
@@ -59,6 +60,16 @@ export type EuiCollapsibleNavBetaProps = CommonProps &
      * take up the full width of the page.
      */
     width?: number;
+    /**
+     * Overlay flyouts are considered dialogs, and dialogs must have a label.
+     * By default, a label of `Site menu` will be applied.
+     *
+     * If your usage of this component is not actually for site-wide navigation,
+     * please set your own `aria-label` or `aria-labelledby`.
+     *
+     * @default 'Site menu'
+     */
+    'aria-label'?: string;
   };
 
 export const EuiCollapsibleNavBeta: FunctionComponent<
@@ -156,6 +167,10 @@ export const EuiCollapsibleNavBeta: FunctionComponent<
     conditionalId: id,
     suffix: 'euiCollapsibleNav',
   });
+  const defaultAriaLabel = useEuiI18n(
+    'euiCollapsibleNavBeta.ariaLabel',
+    'Site menu'
+  );
 
   const buttonRef = useRef<HTMLDivElement | null>(null);
   const focusTrapProps: EuiFlyoutProps['focusTrapProps'] = useMemo(
@@ -183,6 +198,7 @@ export const EuiCollapsibleNavBeta: FunctionComponent<
   // Wait for any fixed headers to be queried before rendering (prevents position jumping)
   const flyout = fixedHeadersCount !== false && (
     <EuiFlyout
+      aria-label={defaultAriaLabel}
       {...rest} // EuiCollapsibleNav is significantly less permissive than EuiFlyout
       id={flyoutID}
       css={cssStyles}

src/components/flyout/__snapshots__/flyout.test.tsx.snap

@@ -1059,9 +1059,6 @@ Array [
   >
     <div
       class="euiFlyout emotion-euiFlyout-l-m-noMaxWidth-push-right-right"
-      data-autofocus="true"
-      role="dialog"
-      tabindex="0"
     >
       <button
         aria-label="Close this dialog"

src/components/flyout/flyout.test.tsx

@@ -71,6 +71,25 @@ describe('EuiFlyout', () => {
     ).toBeFalsy();
   });
 
+  it('allows setting custom aria-describedby attributes', () => {
+    const { getByTestSubject } = render(
+      <>
+        <EuiFlyout
+          onClose={() => {}}
+          aria-describedby="custom-test-id"
+          data-test-subj="flyout"
+        />
+        <div id="custom-test-id" hidden>
+          This flyout does X, Y, and Z
+        </div>
+      </>
+    );
+    expect(getByTestSubject('flyout')).toHaveAttribute(
+      'aria-describedby',
+      'generated-id custom-test-id'
+    );
+  });
+
   describe('props', () => {
     test('hideCloseButton', () => {
       const component = mount(<EuiFlyout onClose={() => {}} hideCloseButton />);

src/components/flyout/flyout.tsx

@@ -183,6 +183,7 @@ export const EuiFlyout = forwardRef(
       pushMinBreakpoint = 'l',
       focusTrapProps: _focusTrapProps = {},
       includeFixedHeadersInFocusTrap = true,
+      'aria-describedby': _ariaDescribedBy,
       ...rest
     }: EuiFlyoutProps<T>,
     ref:
@@ -349,6 +350,7 @@ export const EuiFlyout = forwardRef(
      */
     const hasOverlayMask = ownFocus && !isPushed;
     const descriptionId = useGeneratedHtmlId();
+    const ariaDescribedBy = classnames(descriptionId, _ariaDescribedBy);
 
     const screenReaderDescription = (
       <EuiScreenReaderOnly>
@@ -409,15 +411,15 @@ export const EuiFlyout = forwardRef(
         {...focusTrapProps}
       >
         <Element
-          css={cssStyles}
-          {...(rest as ComponentPropsWithRef<T>)}
-          role="dialog"
           className={classes}
-          tabIndex={0}
-          data-autofocus
-          aria-describedby={!isPushed ? descriptionId : undefined}
+          css={cssStyles}
           style={newStyle}
           ref={setRef}
+          {...(rest as ComponentPropsWithRef<T>)}
+          role={!isPushed ? 'dialog' : rest.role}
+          tabIndex={!isPushed ? 0 : rest.tabIndex}
+          aria-describedby={!isPushed ? ariaDescribedBy : _ariaDescribedBy}
+          data-autofocus={!isPushed || undefined}
         >
           {!isPushed && screenReaderDescription}
           {closeButton}

upcoming_changelogs/7065.md

@@ -0,0 +1,7 @@
+**Bug fixes**
+
+- Fixed `EuiFlyout`s to accept custom `aria-describedby` IDs
+
+**Accessibility**
+
+- Removed the default `dialog` role and `tabIndex` from push `EuiFlyout`s. Push flyouts, compared to overlay flyouts, require manual accessibility management.